Skip to content

feat: Android Agent MVP — native app + reverse connection + server integration#336

Open
suyiiyii wants to merge 50 commits into
mainfrom
android-agent-unified
Open

feat: Android Agent MVP — native app + reverse connection + server integration#336
suyiiyii wants to merge 50 commits into
mainfrom
android-agent-unified

Conversation

@suyiiyii

Copy link
Copy Markdown
Owner

Summary

  • Native Android App (android-agent/): Kotlin app with HTTP Server, Screen Capture (MediaProjection), Reverse Agent Client (pairing/token persistence/reconnect), Accessibility Service, Foreground Service, Material You/3 UI with setup checklist flow, Chinese + English support
  • Server-side: ReverseAgentRegistry for agent pairing/heartbeat/WebSocket sessions, reverse_agents API endpoints for registration/pairing/status/commands
  • Frontend: "Add Android Agent" entry in DeviceSidebar with i18n (zh/en), agent pairing UI flow

This allows users to control Android devices without ADB setup — just install the APK and pair with AutoGLM-GUI.

Key Components

Android App (android-agent/)

  • MainActivity.kt — Material 3 setup checklist UI with step-by-step permissions
  • AgentHttpServer.kt — Embedded HTTP server implementing Remote Device protocol
  • ScreenCaptureController.kt — MediaProjection-based screenshot capture
  • ReverseAgentClient.kt — Reverse connection client with token persistence and auto-reconnect
  • AgentForegroundService.kt — Foreground service for long-running agent
  • DeviceAccessibilityService.kt — Accessibility service integration

Backend (AutoGLM_GUI/)

  • reverse_agent_registry.py — Agent registry with pairing, heartbeat, WebSocket sessions
  • api/reverse_agents.py — REST API endpoints for reverse agent management

Frontend (frontend/)

  • DeviceSidebar.tsx — Added "Add Android Agent" entry with pairing flow
  • api.ts — Reverse agent API client functions
  • locales/zh.ts, locales/en.ts — i18n strings

Test plan

  • Android app builds with ./gradlew assembleDebug
  • Server-side tests: uv run pytest tests/test_reverse_agents_api.py -v
  • Cancel device release regression: uv run pytest tests/test_cancel_device_release.py -v
  • Frontend builds: cd frontend && pnpm run build
  • Manual: Install APK on device, pair with AutoGLM-GUI, verify screenshot/control works

🤖 Generated with Claude Code

@vercel

vercel Bot commented Apr 23, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
autoglm-gui Ready Ready Preview, Comment Jun 21, 2026 4:28pm

@github-actions

github-actions Bot commented Apr 23, 2026

Copy link
Copy Markdown
Contributor

🚀 Lint & Format Check Results: ✅ PASSED

What was checked:

  • 🐍 Backend: Ruff linting + formatting
  • 📱 Frontend: ESLint + Prettier + TypeScript types
  • 🔧 Unified: Custom lint script validation

✅ All checks passed!

Your code follows the project's style guidelines.

Checks performed:

  • Backend Ruff lint ✅
  • Backend Ruff format ✅
  • Frontend ESLint ✅
  • Frontend Prettier format ✅
  • Frontend TypeScript types ✅
  • Unified lint script ✅

This is an automated check. For questions, ask in the PR discussion.

…bug UI

Adds a real end-to-end test that boots an Android emulator, installs the
debug APK, pairs it with a live backend and asserts that screenshot /
current_app / tap actually work through the reverse command channel — the
one path that exercises the real Kotlin app (MediaProjection +
AccessibilityService), which unit/contract tests can't cover.

- MainActivity: debug-only intent hook (test_server_url / test_pairing_code)
  so CI can claim a pairing without driving the UI. Gated on FLAG_DEBUGGABLE,
  ignored entirely in release builds.
- scripts/android_agent_e2e_check.py: deterministic setup (accessibility via
  settings, MediaProjection pre-granted via appops to skip the consent
  dialog) + device-effect assertions; dumps logcat/screenshot on failure.
- .github/workflows/android-agent-e2e.yml: emulator-runner job (api-35,
  x86_64), non-blocking by design.
- UI: move the technical/debug panel (Technical state, Start/Stop, Tap
  target, swipe sandbox) behind a "developer options" toggle so the main
  flow ends at pairing. The e2e tap assertion uses this toggle as its target.
@claude

claude Bot commented Jun 15, 2026

Copy link
Copy Markdown

Claude finished @suyiiyii's task in 28s —— View job


I'll analyze this and get back to you.

android-emulator-runner executes its `script` line-by-line, so the
backslash line-continuation split the backend launch command (autoglm-gui
got a literal `\` argument) and per-line `export`s did not persist. Move the
whole flow into scripts/run_android_agent_e2e_ci.sh and invoke it in one line.

The previous run confirmed the emulator infra works in CI (x86_64 boots under
KVM in ~40s); only the inline shell was broken.
@claude

claude Bot commented Jun 15, 2026

Copy link
Copy Markdown

Claude finished @suyiiyii's task in 6m 19s —— View job


Review complete

I re-read the server registry/API, device integration, Android native app, frontend sidebar, and contract tests at HEAD (7a3f338). Several P0/P1 issues from earlier review rounds were addressed in be2a850 (localhost-only HTTP server, non-exported accessibility service, heartbeat staleness fallback, event-loop deadlock guard, claim rate limiting, delete endpoint, command-type validation, stale-agent cleanup, updated MCP docstring). The remaining blockers are mostly security boundaries, missing DeviceProtocol actions, command-result handling, and registry/DeviceManager lifecycle consistency.

I attempted to run uv run python scripts/lint.py --backend --check-only and the reverse-agent contract tests, but both require tool-execution approval in this environment; the author’s local verification in the PR thread covers these.

Findings

P0 — Pairing endpoints remain unauthenticated

  • Files: AutoGLM_GUI/api/reverse_agents.py:32-84, AutoGLM_GUI/reverse_agent_registry.py:105-168
  • What: POST /api/reverse_agents/pairings and POST /api/reverse_agents/pairings/claim are open to any network caller. Rate-limiting on claim only slows brute-force; creation is completely unrestricted.
  • Why it matters: Anyone who can reach the server can mint pairings, claim them, obtain a valid agent_token, and connect as a managed reverse-agent device. Once connected, the server routes control commands to it.
  • Fix: Require an existing session/API key to create pairings, or restrict pairing creation to localhost/admin callers. Keep claim rate-limiting but also bind it to a real user/session context. Fix this →

P1 — Android local HTTP server has no authentication

  • File: android-agent/app/src/main/java/com/autoglm/agent/http/AgentHttpServer.kt:16-164
  • What: AgentHttpServer now binds to 127.0.0.1, but /device/$deviceId/tap, /swipe, /type_text, and /screenshot still accept requests without any token check.
  • Why it matters: Any other app on the phone can capture the screen and inject touch input without user consent. This bypasses the token-based security of the reverse WebSocket channel.
  • Fix: Require the persisted agent_token on every control request (e.g., Authorization: Bearer <token> header or a token query param validated against ReverseAgentStore), and reject with 401/403 when missing. Fix this →

P1 — Agent token stored in plaintext SharedPreferences

  • File: android-agent/app/src/main/java/com/autoglm/agent/reverse/ReverseAgentStore.kt:15
  • What: ReverseAgentStore saves agentToken directly in SharedPreferences.
  • Why it matters: Any app with storage access or a rooted device can steal the token and impersonate the agent to the AutoGLM-GUI server.
  • Fix: Use EncryptedSharedPreferences from androidx.security:security-crypto. Fix this →

P1 — ReverseAgentDevice._send ignores command success and returns empty payloads

  • File: AutoGLM_GUI/devices/reverse_agent_device.py:34-66
  • What: _send returns result.payload without checking result.success. If the agent reports failure (e.g., accessibility unavailable), callers like get_screenshot and get_current_app will raise KeyError on missing fields instead of failing with the agent’s error message.
  • Why it matters: Silent/command failures become confusing exceptions far from the actual fault.
  • Fix: Check result.success and raise a clear exception using result.error when the command failed. Fix this →

P1 — ReverseAgentDevice raises NotImplementedError for core agent actions

  • File: AutoGLM_GUI/devices/reverse_agent_device.py:129-145
  • What: back(), home(), and launch_app() raise NotImplementedError.
  • Why it matters: These are part of the DeviceProtocol contract and are commonly invoked by the phone agent. A normal task routed to a reverse agent will crash instead of completing or failing gracefully.
  • Fix: Add back, home, and launch_app command types to ReverseAgentCommandType and CommandExecutor, implement them on the Android side via performGlobalAction/startActivity, and make the device methods send those commands. Fix this →

P1 — remove_agent may fail to close the WebSocket outside an async context

  • File: AutoGLM_GUI/reverse_agent_registry.py:416-422
  • What: remove_agent calls asyncio.create_task(websocket.close(...)) from a synchronous method. asyncio.create_task requires a running event loop in the current thread.
  • Why it matters: DELETE /api/reverse_agents/registry/{agent_id} is a sync FastAPI route, so it runs in a thread-pool thread that usually has no event loop. The close coroutine will raise RuntimeError: no running event loop, be caught and logged, and the agent’s WebSocket will not be closed cleanly.
  • Fix: Guard with asyncio.get_running_loop() or schedule the close on the WebSocket handler’s loop if one exists. Fix this →

P1 — Major regression coverage removed from test_cancel_device_release.py

P2 — Claimed-but-never-connected agents are never cleaned up

  • File: AutoGLM_GUI/reverse_agent_registry.py:445-455
  • What: _cleanup_stale_agents_locked only removes agents whose connection_status is offline. An agent that is claimed but never opens a WebSocket stays in paired status forever and is never removed.
  • Why it matters: Claimed-but-never-connected pairings leak registry memory until server restart.
  • Fix: Also prune agents that have been in paired status (never connected) longer than the stale TTL, using created_at or last_seen_at as the baseline. Fix this →

P2 — WebSocket auth token is passed in the URL query string

  • Files: AutoGLM_GUI/api/reverse_agents.py:173, android-agent/app/src/main/java/com/autoglm/agent/reverse/ReverseAgentClient.kt:237
  • What: ?token=... is appended to the WebSocket URL.
  • Why it matters: URLs are routinely logged by proxies, load balancers, and server access logs, increasing the chance of token leakage.
  • Fix: Pass the token in a WebSocket subprotocol header (Sec-WebSocket-Protocol) or a custom header instead of the query string. Fix this →

P2 — ReverseAgentDevice instances are leaked on disconnect

  • Files: AutoGLM_GUI/device_manager.py:1061-1075
  • What: unregister_reverse_agent marks the managed device as disconnected but never removes the ReverseAgentDevice instance from _reverse_agent_devices. remove_reverse_agent removes it, but normal WebSocket disconnect does not.
  • Why it matters: Each disconnect-and-reconnect cycle (or permanently offline agent) leaks a ReverseAgentDevice instance until server restart.
  • Fix: Remove the reverse agent device instance in unregister_reverse_agent, or clean it up during stale-agent removal in ReverseAgentRegistry. Fix this →

P2 — Deleting a reverse agent from the registry does not remove it from DeviceManager

  • Files: AutoGLM_GUI/api/reverse_agents.py:108-114, AutoGLM_GUI/device_manager.py:1077-1100
  • What: DELETE /api/reverse_agents/registry/{agent_id} calls registry.remove_agent() but does not call device_manager.remove_reverse_agent(). The registry record is gone, but the device still appears in /api/devices as DISCONNECTED.
  • Why it matters: Users see a stale, unusable device in the device list after deleting the agent.
  • Fix: In the delete endpoint, also invoke DeviceManager.get_instance().remove_reverse_agent(agent_id). Fix this →

P2 — Accessibility actions may run on the wrong thread

  • Files: android-agent/app/src/main/java/com/autoglm/agent/reverse/CommandExecutor.kt:9-69, ReverseAgentClient.kt:435-442
  • What: Commands are executed on scheduler.execute { ... }, a background thread. DeviceAccessibilityService calls dispatchGesture and reads rootInActiveWindow.
  • Why it matters: Some AccessibilityService APIs are documented as requiring calls from the main thread; running them on a background thread can cause IllegalStateException or undefined behavior on certain OEM ROMs.
  • Fix: Post accessibility actions to a Handler(Looper.getMainLooper()) and block the scheduler thread for the result. Fix this →

P3 — ScreenCaptureController.capture has a small race window

  • File: android-agent/app/src/main/java/com/autoglm/agent/projection/ScreenCaptureController.kt:130-140
  • What: After acquireLatestImage() returns null, the listener is installed. An image produced between the null check and listener installation sits in the ImageReader buffer until the next frame.
  • Fix: Set the listener first, then call acquireLatestImage() and wait if it is still null. Also consider raising maxImages above 2. Fix this →

P3 — Command round-trip tests use busy-wait polling

  • Files: tests/test_reverse_agents_api.py:193-206, tests/test_reverse_agent_device_manager.py:155-167
  • What: Both tests poll captured["last_sent"] in a 50×0.01 s loop instead of using an asyncio.Event.
  • Why it matters: The tests are slightly racy and slower than necessary; on a slow CI runner the 0.5 s window may be insufficient.
  • Fix: Signal with an asyncio.Event when the fake WebSocket receives the command, then wait on the event. Fix this →

Test observations

  • The contract tests in tests/test_reverse_agents_api.py and tests/test_reverse_agent_device_manager.py cover pairing, claim, heartbeat, stale detection, command round-trip, rate limiting, delete, and invalid command types. Good coverage for the happy path.
  • Codecov reports 112 uncovered lines, mostly in error paths of reverse_agents.py and reverse_agent_registry.py. Adding tests for unknown command results, wrong-agent results, the WebSocket command_result exception path, and the delete/websocket-close path would close most of the gap.

Residual risk / summary

The reverse-agent pairing/registry foundation is solid and well-tested. After be2a850, the main remaining risks are:

  1. Security boundaries: unauthenticated pairing creation/claim, unauthenticated local Android HTTP server, plaintext token storage, and token-in-URL WebSocket auth.
  2. Agent task crashes: back()/home()/launch_app() will crash the phone agent when routed to a reverse agent.
  3. Command failure handling: _send returning failure payloads as success will cause KeyError crashes in screenshot/current-app callers.
  4. Registry/DeviceManager lifecycle: deleting an agent does not remove it from DeviceManager, disconnect leaks ReverseAgentDevice instances, and claimed-but-never-connected agents are never pruned.
  5. Lost regression coverage: the cancelled-device-release tests should be restored.
  6. Runtime reliability: command failures are not propagated, accessibility actions may run off the main thread, and remove_agent may fail to close the WebSocket cleanly.
    · Branch android-agent-unified

In CI the websocket reaches "connected" ~1.4s before the appops-approved
MediaProjection consent activity returns RESULT_OK, so the first screenshot
command raced ahead and got "Screen capture permission has not been granted".
Poll the screenshot command until it succeeds, same as the current_app gate.

CI run 27553475670 confirmed appops auto-grant works on the x86_64 emulator
(logcat: capturePermissionLauncher resultCode=-1); the only issue was timing.
@claude

claude Bot commented Jun 15, 2026

Copy link
Copy Markdown

Claude finished @suyiiyii's task in 2m 49s —— View job


Review complete

I re-read the server registry/API, device integration, Android native app, frontend sidebar, and contract tests at HEAD (7a3f338). Several earlier P0/P1 issues were addressed in be2a850 (localhost-only HTTP server, non-exported accessibility service, heartbeat staleness fallback, event-loop deadlock guard, claim rate limiting, delete endpoint, command-type validation, stale-agent cleanup, updated MCP docstring). The remaining blockers are mostly security boundaries, missing DeviceProtocol actions, command-result handling, and registry/DeviceManager lifecycle consistency.

Findings

P0 — Pairing endpoints remain unauthenticated

  • Files: AutoGLM_GUI/api/reverse_agents.py:32-84, AutoGLM_GUI/reverse_agent_registry.py:105-168
  • What: POST /api/reverse_agents/pairings and POST /api/reverse_agents/pairings/claim are open to any network caller. Rate-limiting on claim only slows brute-force; creation is completely unrestricted.
  • Why it matters: Anyone who can reach the server can mint pairings, claim them, obtain a valid agent_token, and connect as a managed reverse-agent device. Once connected, the server routes control commands to it.
  • Fix: Require an existing session/API key to create pairings, or restrict pairing creation to localhost/admin callers. Keep claim rate-limiting but also bind it to a real user/session context. Fix this →

P1 — Android local HTTP server has no authentication

  • File: android-agent/app/src/main/java/com/autoglm/agent/http/AgentHttpServer.kt:16-164
  • What: AgentHttpServer now binds to 127.0.0.1, but /device/$deviceId/tap, /swipe, /type_text, and /screenshot still accept requests without any token check.
  • Why it matters: Any other app on the phone can capture the screen and inject touch input without user consent. This bypasses the token-based security of the reverse WebSocket channel.
  • Fix: Require the persisted agent_token on every control request (e.g., Authorization: Bearer <token> header or a token query param validated against ReverseAgentStore), and reject with 401/403 when missing. Fix this →

P1 — Agent token stored in plaintext SharedPreferences

  • File: android-agent/app/src/main/java/com/autoglm/agent/reverse/ReverseAgentStore.kt:15
  • What: ReverseAgentStore saves agentToken directly in SharedPreferences.
  • Why it matters: Any app with storage access or a rooted device can steal the token and impersonate the agent to the AutoGLM-GUI server.
  • Fix: Use EncryptedSharedPreferences from androidx.security:security-crypto. Fix this →

P1 — ReverseAgentDevice._send ignores command success and returns empty payloads

  • File: AutoGLM_GUI/devices/reverse_agent_device.py:34-66
  • What: _send returns result.payload without checking result.success. If the agent reports failure (e.g., accessibility unavailable), callers like get_screenshot and get_current_app will raise KeyError on missing fields instead of failing with the agent's error message.
  • Why it matters: Silent/command failures become confusing exceptions far from the actual fault.
  • Fix: Check result.success and raise a clear exception using result.error when the command failed. Fix this →

P1 — ReverseAgentDevice raises NotImplementedError for core agent actions

  • File: AutoGLM_GUI/devices/reverse_agent_device.py:129-145
  • What: back(), home(), and launch_app() raise NotImplementedError.
  • Why it matters: These are part of the DeviceProtocol contract and are commonly invoked by the phone agent. A normal task routed to a reverse agent will crash instead of completing or failing gracefully.
  • Fix: Add back, home, and launch_app command types to ReverseAgentCommandType and CommandExecutor, implement them on the Android side via performGlobalAction/startActivity, and make the device methods send those commands. Fix this →

P1 — remove_agent may fail to close the WebSocket outside an async context

  • File: AutoGLM_GUI/reverse_agent_registry.py:416-422
  • What: remove_agent calls asyncio.create_task(websocket.close(...)) from a synchronous method. asyncio.create_task requires a running event loop in the current thread.
  • Why it matters: DELETE /api/reverse_agents/registry/{agent_id} is a sync FastAPI route, so it runs in a thread-pool thread that usually has no event loop. The close coroutine will raise RuntimeError: no running event loop, be caught and logged, and the agent's WebSocket will not be closed cleanly.
  • Fix: Guard with asyncio.get_running_loop() or schedule the close on the WebSocket handler's loop if one exists. Fix this →

P1 — Major regression coverage removed from test_cancel_device_release.py

P2 — Claimed-but-never-connected agents are never cleaned up

  • File: AutoGLM_GUI/reverse_agent_registry.py:445-455
  • What: _cleanup_stale_agents_locked only removes agents whose connection_status is offline. An agent that is claimed but never opens a WebSocket stays in paired status forever and is never removed.
  • Why it matters: Claimed-but-never-connected pairings leak registry memory until server restart.
  • Fix: Also prune agents that have been in paired status (never connected) longer than the stale TTL, using created_at or last_seen_at as the baseline. Fix this →

P2 — WebSocket auth token is passed in the URL query string

  • Files: AutoGLM_GUI/api/reverse_agents.py:173, android-agent/app/src/main/java/com/autoglm/agent/reverse/ReverseAgentClient.kt:237
  • What: ?token=... is appended to the WebSocket URL.
  • Why it matters: URLs are routinely logged by proxies, load balancers, and server access logs, increasing the chance of token leakage.
  • Fix: Pass the token in a WebSocket subprotocol header (Sec-WebSocket-Protocol) or a custom header instead of the query string. Fix this →

P2 — ReverseAgentDevice instances are leaked on disconnect

  • Files: AutoGLM_GUI/device_manager.py:1061-1075
  • What: unregister_reverse_agent marks the managed device as disconnected but never removes the ReverseAgentDevice instance from _reverse_agent_devices. remove_reverse_agent removes it, but normal WebSocket disconnect does not.
  • Why it matters: Each disconnect-and-reconnect cycle (or permanently offline agent) leaks a ReverseAgentDevice instance until server restart.
  • Fix: Remove the reverse agent device instance in unregister_reverse_agent, or clean it up during stale-agent removal in ReverseAgentRegistry. Fix this →

P2 — Deleting a reverse agent from the registry does not remove it from DeviceManager

  • Files: AutoGLM_GUI/api/reverse_agents.py:108-114, AutoGLM_GUI/device_manager.py:1077-1100
  • What: DELETE /api/reverse_agents/registry/{agent_id} calls registry.remove_agent() but does not call device_manager.remove_reverse_agent(). The registry record is gone, but the device still appears in /api/devices as DISCONNECTED.
  • Why it matters: Users see a stale, unusable device in the device list after deleting the agent.
  • Fix: In the delete endpoint, also invoke DeviceManager.get_instance().remove_reverse_agent(agent_id). Fix this →

P2 — Accessibility actions may run on the wrong thread

  • Files: android-agent/app/src/main/java/com/autoglm/agent/reverse/CommandExecutor.kt:9-69, ReverseAgentClient.kt:435-442
  • What: Commands are executed on scheduler.execute { ... }, a background thread. DeviceAccessibilityService calls dispatchGesture and reads rootInActiveWindow.
  • Why it matters: Some AccessibilityService APIs are documented as requiring calls from the main thread; running them on a background thread can cause IllegalStateException or undefined behavior on certain OEM ROMs.
  • Fix: Post accessibility actions to a Handler(Looper.getMainLooper()) and block the scheduler thread for the result. Fix this →

P3 — ScreenCaptureController.capture has a small race window

  • File: android-agent/app/src/main/java/com/autoglm/agent/projection/ScreenCaptureController.kt:130-140
  • What: After acquireLatestImage() returns null, the listener is installed. An image produced between the null check and listener installation sits in the ImageReader buffer until the next frame.
  • Fix: Set the listener first, then call acquireLatestImage() and wait if it is still null. Also consider raising maxImages above 2. Fix this →

P3 — Command round-trip tests use busy-wait polling

  • Files: tests/test_reverse_agents_api.py:193-206, tests/test_reverse_agent_device_manager.py:155-167
  • What: Both tests poll captured["last_sent"] in a 50×0.01 s loop instead of using an asyncio.Event.
  • Why it matters: The tests are slightly racy and slower than necessary; on a slow CI runner the 0.5 s window may be insufficient.
  • Fix: Signal with an asyncio.Event when the fake WebSocket receives the command, then wait on the event. Fix this →

Test observations

  • The contract tests in tests/test_reverse_agents_api.py and tests/test_reverse_agent_device_manager.py cover pairing, claim, heartbeat, stale detection, command round-trip, rate limiting, delete, and invalid command types. Good coverage for the happy path.
  • Codecov reports 112 uncovered lines, mostly in error paths of reverse_agents.py and reverse_agent_registry.py. Adding tests for unknown command results, wrong-agent results, the WebSocket command_result exception path, and the delete/websocket-close path would close most of the gap.

Residual risk / summary

The reverse-agent pairing/registry foundation is solid and well-tested. After be2a850, the main remaining risks are:

  1. Security boundaries: unauthenticated pairing creation/claim, unauthenticated local Android HTTP server, plaintext token storage, and token-in-URL WebSocket auth.
  2. Agent task crashes: back()/home()/launch_app() will crash the phone agent when routed to a reverse agent.
  3. Command failure handling: _send returning failure payloads as success will cause KeyError crashes in screenshot/current-app callers.
  4. Registry/DeviceManager lifecycle: deleting an agent does not remove it from DeviceManager, disconnect leaks ReverseAgentDevice instances, and claimed-but-never-connected agents are never pruned.
  5. Lost regression coverage: the cancelled-device-release tests should be restored.
  6. Runtime reliability: command failures are not propagated, accessibility actions may run off the main thread, and remove_agent may fail to close the WebSocket cleanly.

· Branch android-agent-unified

current_app failed in CI with accessibility_service_not_connected: setting
enabled_accessibility_services before the freshly-installed app had ever run
does not bind the service, and re-writing the same value is a no-op. Now:
- enable accessibility, but force a rebind (toggle accessibility_enabled 0->1)
  only after the app process is alive (post am start);
- re-kick on every current_app retry until the service connects.

Verified locally from a cleared-accessibility state: ALL CHECKS PASSED.
@claude

claude Bot commented Jun 15, 2026

Copy link
Copy Markdown

Claude finished @suyiiyii's task in 2m 59s —— View job


Review plan

  • Read CLAUDE.md for project context and commands
  • Read backend registry/API files (reverse_agent_registry.py, reverse_agents.py)
  • Read device integration files (reverse_agent_device.py, device_manager.py)
  • Read Android native app files (AgentHttpServer.kt, ReverseAgentClient.kt, CommandExecutor.kt, etc.)
  • Read frontend sidebar integration
  • Read and assess test changes, especially test_cancel_device_release.py
  • Post review feedback with severity-ordered findings

Review complete

I re-read the server registry/API, device integration, Android native app, frontend sidebar, and contract tests at HEAD (4ca0964). Several earlier P0/P1 issues were addressed in previous commits (localhost-only HTTP server, non-exported accessibility service, heartbeat staleness fallback, event-loop deadlock guard, claim rate limiting, delete endpoint, command-type validation, stale-agent cleanup, updated MCP docstring). The remaining blockers are mostly security boundaries, missing DeviceProtocol actions, command-result handling, and registry/DeviceManager lifecycle consistency.

Findings

P0 — Pairing endpoints remain unauthenticated

  • Files: AutoGLM_GUI/api/reverse_agents.py:32-84, AutoGLM_GUI/reverse_agent_registry.py:105-168
  • What: POST /api/reverse_agents/pairings and POST /api/reverse_agents/pairings/claim are open to any network caller. Rate-limiting on claim only slows brute-force; creation is completely unrestricted.
  • Why it matters: Anyone who can reach the server can mint pairings, claim them, obtain a valid agent_token, and connect as a managed reverse-agent device. Once connected, the server routes control commands to it.
  • Fix: Require an existing session/API key to create pairings, or restrict pairing creation to localhost/admin callers. Keep claim rate-limiting but also bind it to a real user/session context. Fix this →

P1 — Android local HTTP server has no authentication

  • File: android-agent/app/src/main/java/com/autoglm/agent/http/AgentHttpServer.kt:16-164
  • What: AgentHttpServer now binds to 127.0.0.1, but /device/$deviceId/tap, /swipe, /type_text, and /screenshot still accept requests without any token check.
  • Why it matters: Any other app on the phone can capture the screen and inject touch input without user consent. This bypasses the token-based security of the reverse WebSocket channel.
  • Fix: Require the persisted agent_token on every control request (e.g., Authorization: Bearer <token> header or a token query param validated against ReverseAgentStore), and reject with 401/403 when missing. Fix this →

P1 — Agent token stored in plaintext SharedPreferences

  • File: android-agent/app/src/main/java/com/autoglm/agent/reverse/ReverseAgentStore.kt:15
  • What: ReverseAgentStore saves agentToken directly in SharedPreferences.
  • Why it matters: Any app with storage access or a rooted device can steal the token and impersonate the agent to the AutoGLM-GUI server.
  • Fix: Use EncryptedSharedPreferences from androidx.security:security-crypto. Fix this →

P1 — ReverseAgentDevice._send ignores command success and returns empty payloads

  • File: AutoGLM_GUI/devices/reverse_agent_device.py:34-66
  • What: _send returns result.payload without checking result.success. If the agent reports failure (e.g., accessibility unavailable), callers like get_screenshot and get_current_app will raise KeyError on missing fields instead of failing with the agent's error message.
  • Why it matters: Silent/command failures become confusing exceptions far from the actual fault.
  • Fix: Check result.success and raise a clear exception using result.error when the command failed. Fix this →

P1 — ReverseAgentDevice raises NotImplementedError for core agent actions

  • File: AutoGLM_GUI/devices/reverse_agent_device.py:129-145
  • What: back(), home(), and launch_app() raise NotImplementedError.
  • Why it matters: These are part of the DeviceProtocol contract and are commonly invoked by the phone agent. A normal task routed to a reverse agent will crash instead of completing or failing gracefully.
  • Fix: Add back, home, and launch_app command types to ReverseAgentCommandType and CommandExecutor, implement them on the Android side via performGlobalAction/startActivity, and make the device methods send those commands. Fix this →

P1 — remove_agent may fail to close the WebSocket outside an async context

  • File: AutoGLM_GUI/reverse_agent_registry.py:416-422
  • What: remove_agent calls asyncio.create_task(websocket.close(...)) from a synchronous method. asyncio.create_task requires a running event loop in the current thread.
  • Why it matters: DELETE /api/reverse_agents/registry/{agent_id} is a sync FastAPI route, so it runs in a thread-pool thread that usually has no event loop. The close coroutine will raise RuntimeError: no running event loop, be caught and logged, and the agent's WebSocket will not be closed cleanly.
  • Fix: Guard with asyncio.get_running_loop() or schedule the close on the WebSocket handler's loop if one exists. Fix this →

P1 — Major regression coverage removed from test_cancel_device_release.py

P2 — Claimed-but-never-connected agents are never cleaned up

  • File: AutoGLM_GUI/reverse_agent_registry.py:445-455
  • What: _cleanup_stale_agents_locked only removes agents whose connection_status is offline. An agent that is claimed but never opens a WebSocket stays in paired status forever and is never removed.
  • Why it matters: Claimed-but-never-connected pairings leak registry memory until server restart.
  • Fix: Also prune agents that have been in paired status (never connected) longer than the stale TTL, using created_at or last_seen_at as the baseline. Fix this →

P2 — WebSocket auth token is passed in the URL query string

  • Files: AutoGLM_GUI/api/reverse_agents.py:173, android-agent/app/src/main/java/com/autoglm/agent/reverse/ReverseAgentClient.kt:237
  • What: ?token=... is appended to the WebSocket URL.
  • Why it matters: URLs are routinely logged by proxies, load balancers, and server access logs, increasing the chance of token leakage.
  • Fix: Pass the token in a WebSocket subprotocol header (Sec-WebSocket-Protocol) or a custom header instead of the query string. Fix this →

P2 — ReverseAgentDevice instances are leaked on disconnect

  • Files: AutoGLM_GUI/device_manager.py:1061-1075
  • What: unregister_reverse_agent marks the managed device as disconnected but never removes the ReverseAgentDevice instance from _reverse_agent_devices. remove_reverse_agent removes it, but normal WebSocket disconnect does not.
  • Why it matters: Each disconnect-and-reconnect cycle (or permanently offline agent) leaks a ReverseAgentDevice instance until server restart.
  • Fix: Remove the reverse agent device instance in unregister_reverse_agent, or clean it up during stale-agent removal in ReverseAgentRegistry. Fix this →

P2 — Deleting a reverse agent from the registry does not remove it from DeviceManager

  • Files: AutoGLM_GUI/api/reverse_agents.py:108-114, AutoGLM_GUI/device_manager.py:1077-1100
  • What: DELETE /api/reverse_agents/registry/{agent_id} calls registry.remove_agent() but does not call device_manager.remove_reverse_agent(). The registry record is gone, but the device still appears in /api/devices as DISCONNECTED.
  • Why it matters: Users see a stale, unusable device in the device list after deleting the agent.
  • Fix: In the delete endpoint, also invoke DeviceManager.get_instance().remove_reverse_agent(agent_id). Fix this →

P2 — Accessibility actions may run on the wrong thread

  • Files: android-agent/app/src/main/java/com/autoglm/agent/reverse/CommandExecutor.kt:9-69, ReverseAgentClient.kt:435-442
  • What: Commands are executed on scheduler.execute { ... }, a background thread. DeviceAccessibilityService calls dispatchGesture and reads rootInActiveWindow.
  • Why it matters: Some AccessibilityService APIs are documented as requiring calls from the main thread; running them on a background thread can cause IllegalStateException or undefined behavior on certain OEM ROMs.
  • Fix: Post accessibility actions to a Handler(Looper.getMainLooper()) and block the scheduler thread for the result. Fix this →

P3 — ScreenCaptureController.capture has a small race window

  • File: android-agent/app/src/main/java/com/autoglm/agent/projection/ScreenCaptureController.kt:130-140
  • What: After acquireLatestImage() returns null, the listener is installed. An image produced between the null check and listener installation sits in the ImageReader buffer until the next frame.
  • Fix: Set the listener first, then call acquireLatestImage() and wait if it is still null. Also consider raising maxImages above 2. Fix this →

P3 — Command round-trip tests use busy-wait polling

  • Files: tests/test_reverse_agents_api.py:193-206, tests/test_reverse_agent_device_manager.py:155-167
  • What: Both tests poll captured["last_sent"] in a 50×0.01 s loop instead of using an asyncio.Event.
  • Why it matters: The tests are slightly racy and slower than necessary; on a slow CI runner the 0.5 s window may be insufficient.
  • Fix: Signal with an asyncio.Event when the fake WebSocket receives the command, then wait on the event. Fix this →

Test observations

  • The contract tests in tests/test_reverse_agents_api.py and tests/test_reverse_agent_device_manager.py cover pairing, claim, heartbeat, stale detection, command round-trip, rate limiting, delete, and invalid command types. Good coverage for the happy path.
  • Codecov reports 112 uncovered lines, mostly in error paths of reverse_agents.py and reverse_agent_registry.py. Adding tests for unknown command results, wrong-agent results, the WebSocket command_result exception path, and the delete/websocket-close path would close most of the gap.

Residual risk / summary

The reverse-agent pairing/registry foundation is solid and well-tested. At 4ca0964, the main remaining risks are:

  1. Security boundaries: unauthenticated pairing creation/claim, unauthenticated local Android HTTP server, plaintext token storage, and token-in-URL WebSocket auth.
  2. Agent task crashes: back()/home()/launch_app() will crash the phone agent when routed to a reverse agent.
  3. Command failure handling: _send returning failure payloads as success will cause KeyError crashes in screenshot/current-app callers.
  4. Registry/DeviceManager lifecycle: deleting an agent does not remove it from DeviceManager, disconnect leaks ReverseAgentDevice instances, and claimed-but-never-connected agents are never pruned.
  5. Lost regression coverage: the cancelled-device-release tests should be restored.
  6. Runtime reliability: command failures are not propagated, accessibility actions may run off the main thread, and remove_agent may fail to close the WebSocket cleanly.

· Branch android-agent-unified

The tap assertion scrolls to developerToggleButton, but the swipe used
hard-coded large-screen coordinates (540,1800). CI emulators run at 320x640,
so those land off-screen and the toggle never scrolls into view
("could not find on-screen node developerToggleButton"). Compute swipe
coordinates from `wm size` instead.

CI progress: screenshot + current_app now pass on x86_64; this was the last
hard-coded assumption. Verified locally (1080x2400): ALL CHECKS PASSED.
@claude

claude Bot commented Jun 15, 2026

Copy link
Copy Markdown

Claude finished @suyiiyii's task in 3m 45s —— View job


Review complete

I re-read the server registry/API, device integration, Android native app, frontend sidebar, and contract tests at HEAD (4ca0964). Several earlier P0/P1 issues were addressed in previous commits (localhost-only HTTP server, non-exported accessibility service, heartbeat staleness fallback, event-loop deadlock guard, claim rate limiting, delete endpoint, command-type validation, stale-agent cleanup, updated MCP docstring). The remaining blockers are mostly security boundaries, missing DeviceProtocol actions, command-result handling, and registry/DeviceManager lifecycle consistency.

I attempted to run uv run python scripts/lint.py --backend --check-only and the reverse-agent contract tests, but both require tool-execution approval in this environment; the author’s local verification in the PR thread covers these.

Findings

P0 — Pairing endpoints remain unauthenticated

  • Files: AutoGLM_GUI/api/reverse_agents.py:32-84, AutoGLM_GUI/reverse_agent_registry.py:105-168
  • What: POST /api/reverse_agents/pairings and POST /api/reverse_agents/pairings/claim are open to any network caller. Rate-limiting on claim only slows brute-force; creation is completely unrestricted.
  • Why it matters: Anyone who can reach the server can mint pairings, claim them, obtain a valid agent_token, and connect as a managed reverse-agent device. Once connected, the server routes control commands to it.
  • Fix: Require an existing session/API key to create pairings, or restrict pairing creation to localhost/admin callers. Keep claim rate-limiting but also bind it to a real user/session context. Fix this →

P1 — Android local HTTP server has no authentication

  • File: android-agent/app/src/main/java/com/autoglm/agent/http/AgentHttpServer.kt:16-164
  • What: AgentHttpServer now binds to 127.0.0.1, but /device/$deviceId/tap, /swipe, /type_text, and /screenshot still accept requests without any token check.
  • Why it matters: Any other app on the phone can capture the screen and inject touch input without user consent. This bypasses the token-based security of the reverse WebSocket channel.
  • Fix: Require the persisted agent_token on every control request (e.g., Authorization: Bearer <token> header or a token query param validated against ReverseAgentStore), and reject with 401/403 when missing. Fix this →

P1 — Agent token stored in plaintext SharedPreferences

  • File: android-agent/app/src/main/java/com/autoglm/agent/reverse/ReverseAgentStore.kt:15
  • What: ReverseAgentStore saves agentToken directly in SharedPreferences.
  • Why it matters: Any app with storage access or a rooted device can steal the token and impersonate the agent to the AutoGLM-GUI server.
  • Fix: Use EncryptedSharedPreferences from androidx.security:security-crypto. Fix this →

P1 — ReverseAgentDevice._send ignores command success and returns empty payloads

  • File: AutoGLM_GUI/devices/reverse_agent_device.py:34-66
  • What: _send returns result.payload without checking result.success. If the agent reports failure (e.g., accessibility unavailable), callers like get_screenshot and get_current_app will raise KeyError on missing fields instead of failing with the agent's error message.
  • Why it matters: Silent/command failures become confusing exceptions far from the actual fault.
  • Fix: Check result.success and raise a clear exception using result.error when the command failed. Fix this →

P1 — ReverseAgentDevice raises NotImplementedError for core agent actions

  • File: AutoGLM_GUI/devices/reverse_agent_device.py:129-145
  • What: back(), home(), and launch_app() raise NotImplementedError.
  • Why it matters: These are part of the DeviceProtocol contract and are commonly invoked by the phone agent. A normal task routed to a reverse agent will crash instead of completing or failing gracefully.
  • Fix: Add back, home, and launch_app command types to ReverseAgentCommandType and CommandExecutor, implement them on the Android side via performGlobalAction/startActivity, and make the device methods send those commands. Fix this →

P1 — remove_agent may fail to close the WebSocket outside an async context

  • File: AutoGLM_GUI/reverse_agent_registry.py:416-422
  • What: remove_agent calls asyncio.create_task(websocket.close(...)) from a synchronous method. asyncio.create_task requires a running event loop in the current thread.
  • Why it matters: DELETE /api/reverse_agents/registry/{agent_id} is a sync FastAPI route, so it runs in a thread-pool thread that usually has no event loop. The close coroutine will raise RuntimeError: no running event loop, be caught and logged, and the agent's WebSocket will not be closed cleanly.
  • Fix: Guard with asyncio.get_running_loop() or schedule the close on the WebSocket handler’s loop if one exists. Fix this →

P1 — Major regression coverage removed from test_cancel_device_release.py

P2 — Claimed-but-never-connected agents are never cleaned up

  • File: AutoGLM_GUI/reverse_agent_registry.py:445-455
  • What: _cleanup_stale_agents_locked only removes agents whose connection_status is offline. An agent that is claimed but never opens a WebSocket stays in paired status forever and is never removed.
  • Why it matters: Claimed-but-never-connected pairings leak registry memory until server restart.
  • Fix: Also prune agents that have been in paired status (never connected) longer than the stale TTL, using created_at or last_seen_at as the baseline. Fix this →

P2 — WebSocket auth token is passed in the URL query string

  • Files: AutoGLM_GUI/api/reverse_agents.py:173, android-agent/app/src/main/java/com/autoglm/agent/reverse/ReverseAgentClient.kt:237
  • What: ?token=... is appended to the WebSocket URL.
  • Why it matters: URLs are routinely logged by proxies, load balancers, and server access logs, increasing the chance of token leakage.
  • Fix: Pass the token in a WebSocket subprotocol header (Sec-WebSocket-Protocol) or a custom header instead of the query string. Fix this →

P2 — ReverseAgentDevice instances are leaked on disconnect

  • Files: AutoGLM_GUI/device_manager.py:1061-1075
  • What: unregister_reverse_agent marks the managed device as disconnected but never removes the ReverseAgentDevice instance from _reverse_agent_devices. remove_reverse_agent removes it, but normal WebSocket disconnect does not.
  • Why it matters: Each disconnect-and-reconnect cycle (or permanently offline agent) leaks a ReverseAgentDevice instance until server restart.
  • Fix: Remove the reverse agent device instance in unregister_reverse_agent, or clean it up during stale-agent removal in ReverseAgentRegistry. Fix this →

P2 — Deleting a reverse agent from the registry does not remove it from DeviceManager

  • Files: AutoGLM_GUI/api/reverse_agents.py:108-114, AutoGLM_GUI/device_manager.py:1077-1100
  • What: DELETE /api/reverse_agents/registry/{agent_id} calls registry.remove_agent() but does not call device_manager.remove_reverse_agent(). The registry record is gone, but the device still appears in /api/devices as DISCONNECTED.
  • Why it matters: Users see a stale, unusable device in the device list after deleting the agent.
  • Fix: In the delete endpoint, also invoke DeviceManager.get_instance().remove_reverse_agent(agent_id). Fix this →

P2 — Accessibility actions may run on the wrong thread

  • Files: android-agent/app/src/main/java/com/autoglm/agent/reverse/CommandExecutor.kt:9-69, ReverseAgentClient.kt:435-442
  • What: Commands are executed on scheduler.execute { ... }, a background thread. DeviceAccessibilityService calls dispatchGesture and reads rootInActiveWindow.
  • Why it matters: Some AccessibilityService APIs are documented as requiring calls from the main thread; running them on a background thread can cause IllegalStateException or undefined behavior on certain OEM ROMs.
  • Fix: Post accessibility actions to a Handler(Looper.getMainLooper()) and block the scheduler thread for the result. Fix this →

P3 — ScreenCaptureController.capture has a small race window

  • File: android-agent/app/src/main/java/com/autoglm/agent/projection/ScreenCaptureController.kt:130-140
  • What: After acquireLatestImage() returns null, the listener is installed. An image produced between the null check and listener installation sits in the ImageReader buffer until the next frame.
  • Fix: Set the listener first, then call acquireLatestImage() and wait if it is still null. Also consider raising maxImages above 2. Fix this →

P3 — Command round-trip tests use busy-wait polling

  • Files: tests/test_reverse_agents_api.py:193-206, tests/test_reverse_agent_device_manager.py:155-167
  • What: Both tests poll captured["last_sent"] in a 50×0.01 s loop instead of using an asyncio.Event.
  • Why it matters: The tests are slightly racy and slower than necessary; on a slow CI runner the 0.5 s window may be insufficient.
  • Fix: Signal with an asyncio.Event when the fake WebSocket receives the command, then wait on the event. Fix this →

Test observations

  • The contract tests in tests/test_reverse_agents_api.py and tests/test_reverse_agent_device_manager.py cover pairing, claim, heartbeat, stale detection, command round-trip, rate limiting, delete, and invalid command types. Good coverage for the happy path.
  • Codecov reports 112 uncovered lines, mostly in error paths of reverse_agents.py and reverse_agent_registry.py. Adding tests for unknown command results, wrong-agent results, the WebSocket command_result exception path, and the delete/websocket-close path would close most of the gap.

Residual risk / summary

The reverse-agent pairing/registry foundation is solid and well-tested. At 4ca0964, the main remaining risks are:

  1. Security boundaries: unauthenticated pairing creation/claim, unauthenticated local Android HTTP server, plaintext token storage, and token-in-URL WebSocket auth.
  2. Agent task crashes: back()/home()/launch_app() will crash the phone agent when routed to a reverse agent.
  3. Command failure handling: _send returning failure payloads as success will cause KeyError crashes in screenshot/current-app callers.
  4. Registry/DeviceManager lifecycle: deleting an agent does not remove it from DeviceManager, disconnect leaks ReverseAgentDevice instances, and claimed-but-never-connected agents are never pruned.
  5. Lost regression coverage: the cancelled-device-release tests should be restored.
  6. Runtime reliability: command failures are not propagated, accessibility actions may run off the main thread, and remove_agent may fail to close the WebSocket cleanly.

· Branch android-agent-unified

The tap assertion kept failing on the CI emulator (320x640). Three fixes:
- scroll_to now accepts the node only when its center is in the on-screen
  tappable band and nudges up/down, instead of tapping whatever the dump
  reports first (uiautomator lists off-screen scroll children too).
- find_node matches negative bounds, so an overshoot (node scrolled above the
  fold) is detectable and recoverable.
- screen_size prefers "Override size" over "Physical size".

Reproduced CI's 320x640 locally (adb wm size 320x640) and passed twice.
@claude

claude Bot commented Jun 15, 2026

Copy link
Copy Markdown

Claude finished @suyiiyii's task in 5m 18s —— View job


Review plan

  • Read CLAUDE.md for project context and commands
  • Read backend registry/API files (reverse_agent_registry.py, reverse_agents.py)
  • Read device integration files (reverse_agent_device.py, device_manager.py)
  • Read Android native app files (AgentHttpServer.kt, ReverseAgentClient.kt, CommandExecutor.kt, etc.)
  • Read frontend sidebar integration and contract tests
  • Compare changes since last review (4ca0964..HEAD)
  • Post review feedback with severity-ordered findings

Review complete

I re-read the server registry/API, device integration, Android native app, frontend sidebar, and contract tests at HEAD (24c3f49). The commits since the last review (4ca0964) only touch scripts/android_agent_e2e_check.py (scroll-robustness fixes for small emulators), so the code-level findings below are unchanged from the previous round.

Several earlier P0/P1 issues were already addressed in be2a850 (localhost-only HTTP server, non-exported accessibility service, heartbeat staleness fallback, event-loop deadlock guard, claim rate limiting, delete endpoint, command-type validation, stale-agent cleanup, updated MCP docstring). The remaining blockers are mostly security boundaries, missing DeviceProtocol actions, command-result handling, and registry/DeviceManager lifecycle consistency.

I did not run uv run python scripts/lint.py or the test suite because those commands require tool-execution approval in this environment; the author’s local verification in the PR thread covers them.

Findings

P0 — Pairing endpoints remain unauthenticated

  • Files: AutoGLM_GUI/api/reverse_agents.py:32-84, AutoGLM_GUI/reverse_agent_registry.py:105-168
  • What: POST /api/reverse_agents/pairings and POST /api/reverse_agents/pairings/claim are open to any network caller. Rate-limiting on claim only slows brute-force; creation is completely unrestricted.
  • Why it matters: Anyone who can reach the server can mint pairings, claim them, obtain a valid agent_token, and connect as a managed reverse-agent device. Once connected, the server routes control commands to it.
  • Fix: Require an existing session/API key to create pairings, or restrict pairing creation to localhost/admin callers. Keep claim rate-limiting but also bind it to a real user/session context. Fix this →

P1 — Android local HTTP server has no authentication

  • File: android-agent/app/src/main/java/com/autoglm/agent/http/AgentHttpServer.kt:16-164
  • What: AgentHttpServer binds to 127.0.0.1, but /device/$deviceId/tap, /swipe, /type_text, and /screenshot accept requests without any token check.
  • Why it matters: Any other app on the phone can capture the screen and inject touch input without user consent. This bypasses the token-based security of the reverse WebSocket channel.
  • Fix: Require the persisted agent_token on every control request (e.g., Authorization: Bearer <token> header or a token query param validated against ReverseAgentStore), and reject with 401/403 when missing. Fix this →

P1 — Agent token stored in plaintext SharedPreferences

  • File: android-agent/app/src/main/java/com/autoglm/agent/reverse/ReverseAgentStore.kt:15
  • What: ReverseAgentStore saves agentToken directly in SharedPreferences.
  • Why it matters: Any app with storage access or a rooted device can steal the token and impersonate the agent to the AutoGLM-GUI server.
  • Fix: Use EncryptedSharedPreferences from androidx.security:security-crypto. Fix this →

P1 — ReverseAgentDevice._send ignores command success and returns empty payloads

  • File: AutoGLM_GUI/devices/reverse_agent_device.py:34-66
  • What: _send returns result.payload without checking result.success. If the agent reports failure (e.g., accessibility unavailable), callers like get_screenshot and get_current_app will raise KeyError on missing fields instead of failing with the agent’s error message.
  • Why it matters: Silent/command failures become confusing exceptions far from the actual fault.
  • Fix: Check result.success and raise a clear exception using result.error when the command failed. Fix this →

P1 — ReverseAgentDevice raises NotImplementedError for core agent actions

  • File: AutoGLM_GUI/devices/reverse_agent_device.py:129-145
  • What: back(), home(), and launch_app() raise NotImplementedError.
  • Why it matters: These are part of the DeviceProtocol contract and are commonly invoked by the phone agent. A normal task routed to a reverse agent will crash instead of completing or failing gracefully.
  • Fix: Add back, home, and launch_app command types to ReverseAgentCommandType and CommandExecutor, implement them on the Android side via performGlobalAction/startActivity, and make the device methods send those commands. Fix this →

P1 — remove_agent may fail to close the WebSocket outside an async context

  • File: AutoGLM_GUI/reverse_agent_registry.py:416-422
  • What: remove_agent calls asyncio.create_task(websocket.close(...)) from a synchronous method. asyncio.create_task requires a running event loop in the current thread.
  • Why it matters: DELETE /api/reverse_agents/registry/{agent_id} is a sync FastAPI route, so it runs in a thread-pool thread that usually has no event loop. The close coroutine will raise RuntimeError: no running event loop, be caught and logged, and the agent’s WebSocket will not be closed cleanly.
  • Fix: Guard with asyncio.get_running_loop() or schedule the close on the WebSocket handler’s loop if one exists. Fix this →

P1 — Major regression coverage removed from test_cancel_device_release.py

P2 — Claimed-but-never-connected agents are never cleaned up

  • File: AutoGLM_GUI/reverse_agent_registry.py:445-455
  • What: _cleanup_stale_agents_locked only removes agents whose connection_status is offline. An agent that is claimed but never opens a WebSocket stays in paired status forever and is never removed.
  • Why it matters: Claimed-but-never-connected pairings leak registry memory until server restart.
  • Fix: Also prune agents that have been in paired status (never connected) longer than the stale TTL, using created_at or last_seen_at as the baseline. Fix this →

P2 — WebSocket auth token is passed in the URL query string

  • Files: AutoGLM_GUI/api/reverse_agents.py:173, android-agent/app/src/main/java/com/autoglm/agent/reverse/ReverseAgentClient.kt:237
  • What: ?token=... is appended to the WebSocket URL.
  • Why it matters: URLs are routinely logged by proxies, load balancers, and server access logs, increasing the chance of token leakage.
  • Fix: Pass the token in a WebSocket subprotocol header (Sec-WebSocket-Protocol) or a custom header instead of the query string. Fix this →

P2 — ReverseAgentDevice instances are leaked on disconnect

  • Files: AutoGLM_GUI/device_manager.py:1061-1075
  • What: unregister_reverse_agent marks the managed device as disconnected but never removes the ReverseAgentDevice instance from _reverse_agent_devices. remove_reverse_agent removes it, but normal WebSocket disconnect does not.
  • Why it matters: Each disconnect-and-reconnect cycle (or permanently offline agent) leaks a ReverseAgentDevice instance until server restart.
  • Fix: Remove the reverse agent device instance in unregister_reverse_agent, or clean it up during stale-agent removal in ReverseAgentRegistry. Fix this →

P2 — Deleting a reverse agent from the registry does not remove it from DeviceManager

  • Files: AutoGLM_GUI/api/reverse_agents.py:108-114, AutoGLM_GUI/device_manager.py:1077-1100
  • What: DELETE /api/reverse_agents/registry/{agent_id} calls registry.remove_agent() but does not call device_manager.remove_reverse_agent(). The registry record is gone, but the device still appears in /api/devices as DISCONNECTED.
  • Why it matters: Users see a stale, unusable device in the device list after deleting the agent.
  • Fix: In the delete endpoint, also invoke DeviceManager.get_instance().remove_reverse_agent(agent_id). Fix this →

P2 — Accessibility actions may run on the wrong thread

  • Files: android-agent/app/src/main/java/com/autoglm/agent/reverse/CommandExecutor.kt:9-69, ReverseAgentClient.kt:435-442
  • What: Commands are executed on scheduler.execute { ... }, a background thread. DeviceAccessibilityService calls dispatchGesture and reads rootInActiveWindow.
  • Why it matters: Some AccessibilityService APIs are documented as requiring calls from the main thread; running them on a background thread can cause IllegalStateException or undefined behavior on certain OEM ROMs.
  • Fix: Post accessibility actions to a Handler(Looper.getMainLooper()) and block the scheduler thread for the result. Fix this →

P2 — android:usesCleartextTraffic="true" is overly broad

  • File: android-agent/app/src/main/AndroidManifest.xml:14
  • What: The entire app opts into cleartext traffic.
  • Why it matters: Paired with the unauthenticated local HTTP server, this widens the window for network-based attacks. The WebSocket path can already be wss:// when the server uses HTTPS; the local HTTP server only serves 127.0.0.1, so a per-domain network_security_config.xml would be safer.
  • Fix: Replace the global flag with a networkSecurityConfig that permits cleartext only for 127.0.0.1 and the configured server base URL if needed.

P3 — ScreenCaptureController.capture has a small race window

  • File: android-agent/app/src/main/java/com/autoglm/agent/projection/ScreenCaptureController.kt:130-140
  • What: After acquireLatestImage() returns null, the listener is installed. An image produced between the null check and listener installation sits in the ImageReader buffer until the next frame.
  • Fix: Set the listener first, then call acquireLatestImage() and wait if it is still null. Also consider raising maxImages above 2. Fix this →

P3 — Command round-trip tests use busy-wait polling

  • Files: tests/test_reverse_agents_api.py:193-206, tests/test_reverse_agent_device_manager.py:155-167
  • What: Both tests poll captured["last_sent"] in a 50×0.01 s loop instead of using an asyncio.Event.
  • Why it matters: The tests are slightly racy and slower than necessary; on a slow CI runner the 0.5 s window may be insufficient.
  • Fix: Signal with an asyncio.Event when the fake WebSocket receives the command, then wait on the event. Fix this →

Test observations

  • The contract tests in tests/test_reverse_agents_api.py and tests/test_reverse_agent_device_manager.py cover pairing, claim, heartbeat, stale detection, command round-trip, rate limiting, delete, and invalid command types. Good coverage for the happy path.
  • Codecov reports 112 uncovered lines, mostly in error paths of reverse_agents.py and reverse_agent_registry.py. Adding tests for unknown command results, wrong-agent results, the WebSocket command_result exception path, and the delete/websocket-close path would close most of the gap.
  • The new scroll-robustness changes in scripts/android_agent_e2e_check.py correctly handle small (320×640) emulators and off-screen nodes with negative bounds.

Residual risk / summary

The reverse-agent pairing/registry foundation is solid and well-tested. At 24c3f49, the main remaining risks are:

  1. Security boundaries: unauthenticated pairing creation/claim, unauthenticated local Android HTTP server, plaintext token storage, token-in-URL WebSocket auth, and broad cleartext traffic.
  2. Agent task crashes: back()/home()/launch_app() will crash the phone agent when routed to a reverse agent.
  3. Command failure handling: _send returning failure payloads as success will cause KeyError crashes in screenshot/current-app callers.
  4. Registry/DeviceManager lifecycle: deleting an agent does not remove it from DeviceManager, disconnect leaks ReverseAgentDevice instances, and claimed-but-never-connected agents are never pruned.
  5. Lost regression coverage: the cancelled-device-release tests should be restored.
  6. Runtime reliability: command failures are not propagated, accessibility actions may run off the main thread, and remove_agent may fail to close the WebSocket cleanly.

· Branch android-agent-unified

The e2e test step itself passed on the x86_64 CI emulator (run 27556963480,
step "Run Android Agent E2E" = success). The job was only marked failed by the
setup-uv post-job step, where `uv cache prune` hung for 5 minutes and was
killed (exit 2). Disable uv caching for this job; uv sync is fast enough.
@claude

claude Bot commented Jun 15, 2026

Copy link
Copy Markdown

Claude finished @suyiiyii's task in 3m 12s —— View job


Review plan

  • Read CLAUDE.md for project context and commands
  • Read backend registry/API files (reverse_agent_registry.py, reverse_agents.py)
  • Read device integration files (reverse_agent_device.py, device_manager.py)
  • Read Android native app files (AgentHttpServer.kt, ReverseAgentClient.kt, CommandExecutor.kt, etc.)
  • Read frontend sidebar integration and contract tests
  • Compare changes since last review and assess current HEAD
  • Post review feedback with severity-ordered findings

Review complete

I read the server registry/API, device integration, Android native app, frontend sidebar, and contract tests at HEAD (24c3f49). The commits since the previous review round only touch scripts/android_agent_e2e_check.py (scroll-robustness fixes for small emulators), so the code-level findings below remain applicable to the current branch.

Several earlier P0/P1 issues were already addressed in previous commits: the Android HTTP server now binds to 127.0.0.1, the accessibility service is no longer exported, heartbeat staleness falls back to connected_at, ReverseAgentDevice._send guards against event-loop-thread deadlock, claim attempts are rate-limited, a DELETE registry endpoint exists, command types are validated, stale offline agents are cleaned up, and the MCP list_devices docstring was updated.

The remaining blockers are mostly security boundaries, missing DeviceProtocol actions, command-result handling, and registry/DeviceManager lifecycle consistency.

Findings

P0 — Pairing endpoints remain unauthenticated

  • Files: AutoGLM_GUI/api/reverse_agents.py:32-84, AutoGLM_GUI/reverse_agent_registry.py:105-168
  • What: POST /api/reverse_agents/pairings and POST /api/reverse_agents/pairings/claim are open to any network caller. Rate-limiting on claim only slows brute-force; creation is completely unrestricted.
  • Why it matters: Anyone who can reach the server can mint pairings, claim them, obtain a valid agent_token, and connect as a managed reverse-agent device. Once connected, the server routes control commands to it.
  • Fix: Require an existing session/API key to create pairings, or restrict pairing creation to localhost/admin callers. Keep claim rate-limiting but also bind it to a real user/session context. Fix this →

P1 — Android local HTTP server has no authentication

  • File: android-agent/app/src/main/java/com/autoglm/agent/http/AgentHttpServer.kt:16-164
  • What: AgentHttpServer binds to 127.0.0.1, but /device/$deviceId/tap, /swipe, /type_text, and /screenshot accept requests without any token check.
  • Why it matters: Any other app on the phone can capture the screen and inject touch input without user consent. This bypasses the token-based security of the reverse WebSocket channel.
  • Fix: Require the persisted agent_token on every control request (e.g., Authorization: Bearer <token> header or a token query param validated against ReverseAgentStore), and reject with 401/403 when missing. Fix this →

P1 — Agent token stored in plaintext SharedPreferences

  • File: android-agent/app/src/main/java/com/autoglm/agent/reverse/ReverseAgentStore.kt:15
  • What: ReverseAgentStore saves agentToken directly in SharedPreferences.
  • Why it matters: Any app with storage access or a rooted device can steal the token and impersonate the agent to the AutoGLM-GUI server.
  • Fix: Use EncryptedSharedPreferences from androidx.security:security-crypto. Fix this →

P1 — ReverseAgentDevice._send ignores command success and returns empty payloads

  • File: AutoGLM_GUI/devices/reverse_agent_device.py:34-66
  • What: _send returns result.payload without checking result.success. If the agent reports failure (e.g., accessibility unavailable), callers like get_screenshot and get_current_app will raise KeyError on missing fields instead of failing with the agent’s error message.
  • Why it matters: Silent/command failures become confusing exceptions far from the actual fault.
  • Fix: Check result.success and raise a clear exception using result.error when the command failed. Fix this →

P1 — ReverseAgentDevice raises NotImplementedError for core agent actions

  • File: AutoGLM_GUI/devices/reverse_agent_device.py:129-145
  • What: back(), home(), and launch_app() raise NotImplementedError.
  • Why it matters: These are part of the DeviceProtocol contract and are commonly invoked by the phone agent. A normal task routed to a reverse agent will crash instead of completing or failing gracefully.
  • Fix: Add back, home, and launch_app command types to ReverseAgentCommandType and CommandExecutor, implement them on the Android side via performGlobalAction/startActivity, and make the device methods send those commands. Fix this →

P1 — remove_agent may fail to close the WebSocket outside an async context

  • File: AutoGLM_GUI/reverse_agent_registry.py:416-422
  • What: remove_agent calls asyncio.create_task(websocket.close(...)) from a synchronous method. asyncio.create_task requires a running event loop in the current thread.
  • Why it matters: DELETE /api/reverse_agents/registry/{agent_id} is a sync FastAPI route, so it runs in a thread-pool thread that usually has no event loop. The close coroutine will raise RuntimeError: no running event loop, be caught and logged, and the agent’s WebSocket will not be closed cleanly.
  • Fix: Guard with asyncio.get_running_loop() or schedule the close on the WebSocket handler’s loop if one exists. Fix this →

P1 — Major regression coverage removed from test_cancel_device_release.py

P2 — Claimed-but-never-connected agents are never cleaned up

  • File: AutoGLM_GUI/reverse_agent_registry.py:445-455
  • What: _cleanup_stale_agents_locked only removes agents whose connection_status is offline. An agent that is claimed but never opens a WebSocket stays in paired status forever and is never removed.
  • Why it matters: Claimed-but-never-connected pairings leak registry memory until server restart.
  • Fix: Also prune agents that have been in paired status (never connected) longer than the stale TTL, using created_at or last_seen_at as the baseline. Fix this →

P2 — WebSocket auth token is passed in the URL query string

  • Files: AutoGLM_GUI/api/reverse_agents.py:173, android-agent/app/src/main/java/com/autoglm/agent/reverse/ReverseAgentClient.kt:237
  • What: ?token=... is appended to the WebSocket URL.
  • Why it matters: URLs are routinely logged by proxies, load balancers, and server access logs, increasing the chance of token leakage.
  • Fix: Pass the token in a WebSocket subprotocol header (Sec-WebSocket-Protocol) or a custom header instead of the query string. Fix this →

P2 — ReverseAgentDevice instances are leaked on disconnect

  • Files: AutoGLM_GUI/device_manager.py:1061-1075
  • What: unregister_reverse_agent marks the managed device as disconnected but never removes the ReverseAgentDevice instance from _reverse_agent_devices. remove_reverse_agent removes it, but normal WebSocket disconnect does not.
  • Why it matters: Each disconnect-and-reconnect cycle (or permanently offline agent) leaks a ReverseAgentDevice instance until server restart.
  • Fix: Remove the reverse agent device instance in unregister_reverse_agent, or clean it up during stale-agent removal in ReverseAgentRegistry. Fix this →

P2 — Deleting a reverse agent from the registry does not remove it from DeviceManager

  • Files: AutoGLM_GUI/api/reverse_agents.py:108-114, AutoGLM_GUI/device_manager.py:1077-1100
  • What: DELETE /api/reverse_agents/registry/{agent_id} calls registry.remove_agent() but does not call device_manager.remove_reverse_agent(). The registry record is gone, but the device still appears in /api/devices as DISCONNECTED.
  • Why it matters: Users see a stale, unusable device in the device list after deleting the agent.
  • Fix: In the delete endpoint, also invoke DeviceManager.get_instance().remove_reverse_agent(agent_id). Fix this →

P2 — Accessibility actions may run on the wrong thread

  • Files: android-agent/app/src/main/java/com/autoglm/agent/reverse/CommandExecutor.kt:9-69, ReverseAgentClient.kt:435-442
  • What: Commands are executed on scheduler.execute { ... }, a background thread. DeviceAccessibilityService calls dispatchGesture and reads rootInActiveWindow.
  • Why it matters: Some AccessibilityService APIs are documented as requiring calls from the main thread; running them on a background thread can cause IllegalStateException or undefined behavior on certain OEM ROMs.
  • Fix: Post accessibility actions to a Handler(Looper.getMainLooper()) and block the scheduler thread for the result. Fix this →

P2 — android:usesCleartextTraffic="true" is overly broad

  • File: android-agent/app/src/main/AndroidManifest.xml:14
  • What: The entire app opts into cleartext traffic.
  • Why it matters: Paired with the unauthenticated local HTTP server, this widens the window for network-based attacks. The WebSocket path can already be wss:// when the server uses HTTPS; the local HTTP server only serves 127.0.0.1, so a per-domain network_security_config.xml would be safer.
  • Fix: Replace the global flag with a networkSecurityConfig that permits cleartext only for 127.0.0.1 and the configured server base URL if needed.

P3 — ScreenCaptureController.capture has a small race window

  • File: android-agent/app/src/main/java/com/autoglm/agent/projection/ScreenCaptureController.kt:130-140
  • What: After acquireLatestImage() returns null, the listener is installed. An image produced between the null check and listener installation sits in the ImageReader buffer until the next frame.
  • Fix: Set the listener first, then call acquireLatestImage() and wait if it is still null. Also consider raising maxImages above 2. Fix this →

P3 — Command round-trip tests use busy-wait polling

  • Files: tests/test_reverse_agents_api.py:193-206, tests/test_reverse_agent_device_manager.py:155-167
  • What: Both tests poll captured["last_sent"] in a 50×0.01 s loop instead of using an asyncio.Event.
  • Why it matters: The tests are slightly racy and slower than necessary; on a slow CI runner the 0.5 s window may be insufficient.
  • Fix: Signal with an asyncio.Event when the fake WebSocket receives the command, then wait on the event. Fix this →

Test observations

  • The contract tests in tests/test_reverse_agents_api.py and tests/test_reverse_agent_device_manager.py cover pairing, claim, heartbeat, stale detection, command round-trip, rate limiting, delete, and invalid command types. Good coverage for the happy path.
  • Codecov reports 112 uncovered lines, mostly in error paths of reverse_agents.py and reverse_agent_registry.py. Adding tests for unknown command results, wrong-agent results, the WebSocket command_result exception path, and the delete/websocket-close path would close most of the gap.
  • The new scroll-robustness changes in scripts/android_agent_e2e_check.py correctly handle small (320×640) emulators and off-screen nodes with negative bounds.

Residual risk / summary

The reverse-agent pairing/registry foundation is solid and well-tested. At 24c3f49, the main remaining risks are:

  1. Security boundaries: unauthenticated pairing creation/claim, unauthenticated local Android HTTP server, plaintext token storage, token-in-URL WebSocket auth, and broad cleartext traffic.
  2. Agent task crashes: back()/home()/launch_app() will crash the phone agent when routed to a reverse agent.
  3. Command failure handling: _send returning failure payloads as success will cause KeyError crashes in screenshot/current-app callers.
  4. Registry/DeviceManager lifecycle: deleting an agent does not remove it from DeviceManager, disconnect leaks ReverseAgentDevice instances, and claimed-but-never-connected agents are never pruned.
  5. Lost regression coverage: the cancelled-device-release tests should be restored.
  6. Runtime reliability: command failures are not propagated, accessibility actions may run off the main thread, and remove_agent may fail to close the WebSocket cleanly.

· Branch android-agent-unified
·

@claude

claude Bot commented Jun 15, 2026

Copy link
Copy Markdown

Claude finished @suyiiyii's task in 3m 28s —— View job


Review complete

I read the server registry/API, device integration, Android native app, frontend sidebar, and tests at HEAD (9887d21). Several P0/P1 issues from earlier rounds were already addressed in be2a850 (localhost-only HTTP server, non-exported accessibility service, heartbeat staleness fallback, event-loop deadlock guard, claim rate limiting, delete endpoint, command-type validation, stale-agent cleanup, updated MCP docstring). The remaining blockers are mostly security boundaries, missing DeviceProtocol actions, command-result handling, and registry/DeviceManager lifecycle consistency.

I was unable to run git diff origin/main...HEAD because the base branch has no merge base in this checkout, so the review is based on reading the current files directly.

Findings

P0 — Pairing endpoints remain unauthenticated

  • Files: AutoGLM_GUI/api/reverse_agents.py:32-84, AutoGLM_GUI/reverse_agent_registry.py:105-168
  • What: POST /api/reverse_agents/pairings and POST /api/reverse_agents/pairings/claim are open to any network caller. Rate-limiting on claim only slows brute-force; creation is completely unrestricted.
  • Why it matters: Anyone who can reach the server can mint pairings, claim them, obtain a valid agent_token, and connect as a managed reverse-agent device. Once connected, the server routes control commands to it.
  • Fix: Require an existing session/API key to create pairings, or restrict pairing creation to localhost/admin callers. Keep claim rate-limiting but also bind it to a real user/session context. Fix this →

P1 — Android local HTTP server has no authentication

  • File: android-agent/app/src/main/java/com/autoglm/agent/http/AgentHttpServer.kt:16-164
  • What: AgentHttpServer binds to 127.0.0.1, but /device/$deviceId/tap, /swipe, /type_text, and /screenshot accept requests without any token check.
  • Why it matters: Any other app on the phone can capture the screen and inject touch input without user consent. This bypasses the token-based security of the reverse WebSocket channel.
  • Fix: Require the persisted agent_token on every control request (e.g., Authorization: Bearer <token> header or a token query param validated against ReverseAgentStore), and reject with 401/403 when missing. Fix this →

P1 — Agent token stored in plaintext SharedPreferences

  • File: android-agent/app/src/main/java/com/autoglm/agent/reverse/ReverseAgentStore.kt:15
  • What: ReverseAgentStore saves agentToken directly in SharedPreferences.
  • Why it matters: Any app with storage access or a rooted device can steal the token and impersonate the agent to the AutoGLM-GUI server.
  • Fix: Use EncryptedSharedPreferences from androidx.security:security-crypto. Fix this →

P1 — ReverseAgentDevice._send ignores command success and returns empty payloads

  • File: AutoGLM_GUI/devices/reverse_agent_device.py:34-66
  • What: _send returns result.payload without checking result.success. If the agent reports failure (e.g., accessibility unavailable), callers like get_screenshot and get_current_app will raise KeyError on missing fields instead of failing with the agent’s error message.
  • Why it matters: Silent/command failures become confusing exceptions far from the actual fault.
  • Fix: Check result.success and raise a clear exception using result.error when the command failed. Fix this →

P1 — ReverseAgentDevice raises NotImplementedError for core agent actions

  • File: AutoGLM_GUI/devices/reverse_agent_device.py:129-145
  • What: back(), home(), and launch_app() raise NotImplementedError.
  • Why it matters: These are part of the DeviceProtocol contract and are commonly invoked by the phone agent. A normal task routed to a reverse agent will crash instead of completing or failing gracefully.
  • Fix: Add back, home, and launch_app command types to ReverseAgentCommandType and CommandExecutor, implement them on the Android side via performGlobalAction/startActivity, and make the device methods send those commands. Fix this →

P1 — remove_agent may fail to close the WebSocket outside an async context

  • File: AutoGLM_GUI/reverse_agent_registry.py:416-422
  • What: remove_agent calls asyncio.create_task(websocket.close(...)) from a synchronous method. asyncio.create_task requires a running event loop in the current thread.
  • Why it matters: DELETE /api/reverse_agents/registry/{agent_id} is a sync FastAPI route, so it runs in a thread-pool thread that usually has no event loop. The close coroutine will raise RuntimeError: no running event loop, be caught and logged, and the agent’s WebSocket will not be closed cleanly.
  • Fix: Guard with asyncio.get_running_loop() or schedule the close on the WebSocket handler’s loop if one exists. Fix this →

P1 — Major regression coverage removed from test_cancel_device_release.py

P2 — Claimed-but-never-connected agents are never cleaned up

  • File: AutoGLM_GUI/reverse_agent_registry.py:445-455
  • What: _cleanup_stale_agents_locked only removes agents whose connection_status is offline. An agent that is claimed but never opens a WebSocket stays in paired status forever and is never removed.
  • Why it matters: Claimed-but-never-connected pairings leak registry memory until server restart.
  • Fix: Also prune agents that have been in paired status (never connected) longer than the stale TTL, using created_at or last_seen_at as the baseline. Fix this →

P2 — WebSocket auth token is passed in the URL query string

  • Files: AutoGLM_GUI/api/reverse_agents.py:173, android-agent/app/src/main/java/com/autoglm/agent/reverse/ReverseAgentClient.kt:237
  • What: ?token=... is appended to the WebSocket URL.
  • Why it matters: URLs are routinely logged by proxies, load balancers, and server access logs, increasing the chance of token leakage.
  • Fix: Pass the token in a WebSocket subprotocol header (Sec-WebSocket-Protocol) or a custom header instead of the query string. Fix this →

P2 — ReverseAgentDevice instances are leaked on disconnect

  • Files: AutoGLM_GUI/device_manager.py:1061-1075
  • What: unregister_reverse_agent marks the managed device as disconnected but never removes the ReverseAgentDevice instance from _reverse_agent_devices. remove_reverse_agent removes it, but normal WebSocket disconnect does not.
  • Why it matters: Each disconnect-and-reconnect cycle (or permanently offline agent) leaks a ReverseAgentDevice instance until server restart.
  • Fix: Remove the reverse agent device instance in unregister_reverse_agent, or clean it up during stale-agent removal in ReverseAgentRegistry. Fix this →

P2 — Deleting a reverse agent from the registry does not remove it from DeviceManager

  • Files: AutoGLM_GUI/api/reverse_agents.py:108-114, AutoGLM_GUI/device_manager.py:1077-1100
  • What: DELETE /api/reverse_agents/registry/{agent_id} calls registry.remove_agent() but does not call device_manager.remove_reverse_agent(). The registry record is gone, but the device still appears in /api/devices as DISCONNECTED.
  • Why it matters: Users see a stale, unusable device in the device list after deleting the agent.
  • Fix: In the delete endpoint, also invoke DeviceManager.get_instance().remove_reverse_agent(agent_id). Fix this →

P2 — Accessibility actions may run on the wrong thread

  • Files: android-agent/app/src/main/java/com/autoglm/agent/reverse/CommandExecutor.kt:9-69, ReverseAgentClient.kt:435-442
  • What: Commands are executed on scheduler.execute { ... }, a background thread. DeviceAccessibilityService calls dispatchGesture and reads rootInActiveWindow.
  • Why it matters: Some AccessibilityService APIs are documented as requiring calls from the main thread; running them on a background thread can cause IllegalStateException or undefined behavior on certain OEM ROMs.
  • Fix: Post accessibility actions to a Handler(Looper.getMainLooper()) and block the scheduler thread for the result. Fix this →

P2 — android:usesCleartextTraffic="true" is overly broad

  • File: android-agent/app/src/main/AndroidManifest.xml:14
  • What: The entire app opts into cleartext traffic.
  • Why it matters: Paired with the unauthenticated local HTTP server, this widens the window for network-based attacks. The WebSocket path can already be wss:// when the server uses HTTPS; the local HTTP server only serves 127.0.0.1, so a per-domain network_security_config.xml would be safer.
  • Fix: Replace the global flag with a networkSecurityConfig that permits cleartext only for 127.0.0.1 and the configured server base URL if needed.

P2 — Double-tap and clear-text are silently degraded

  • Files: AutoGLM_GUI/devices/reverse_agent_device.py:80-82, 124-127
  • What: double_tap() sends a single tap, and clear_text() sends type_text with an empty string. Neither matches the DeviceProtocol contract.
  • Why it matters: Callers expecting a real double-tap or text-field clear will get subtly wrong behavior, which is hard to debug.
  • Fix: Add a real double_tap command to ReverseAgentCommandType/CommandExecutor, and implement clear_text via the accessibility service (e.g., select-all + delete).

P3 — ScreenCaptureController.capture has a small race window

  • File: android-agent/app/src/main/java/com/autoglm/agent/projection/ScreenCaptureController.kt:130-140
  • What: After acquireLatestImage() returns null, the listener is installed. An image produced between the null check and listener installation sits in the ImageReader buffer until the next frame.
  • Fix: Set the listener first, then call acquireLatestImage() and wait if it is still null. Also consider raising maxImages above 2. Fix this →

P3 — Command round-trip tests use busy-wait polling

  • Files: tests/test_reverse_agents_api.py:193-206, tests/test_reverse_agent_device_manager.py:155-167
  • What: Both tests poll captured["last_sent"] in a 50×0.01 s loop instead of using an asyncio.Event.
  • Why it matters: The tests are slightly racy and slower than necessary; on a slow CI runner the 0.5 s window may be insufficient.
  • Fix: Signal with an asyncio.Event when the fake WebSocket receives the command, then wait on the event. Fix this →

Test observations

  • The contract tests in tests/test_reverse_agents_api.py and tests/test_reverse_agent_device_manager.py cover pairing, claim, heartbeat, stale detection, command round-trip, rate limiting, delete, and invalid command types. Good coverage for the happy path.
  • Codecov reports 112 uncovered lines, mostly in error paths of reverse_agents.py and reverse_agent_registry.py. Adding tests for unknown command results, wrong-agent results, the WebSocket command_result exception path, and the delete/websocket-close path would close most of the gap.
  • The latest scroll-robustness changes in scripts/android_agent_e2e_check.py correctly handle small (320×640) emulators and off-screen nodes with negative bounds.

Residual risk / summary

The reverse-agent pairing/registry foundation is solid and well-tested. At 9887d21, the main remaining risks are:

  1. Security boundaries: unauthenticated pairing creation/claim, unauthenticated local Android HTTP server, plaintext token storage, token-in-URL WebSocket auth, and broad cleartext traffic.
  2. Agent task crashes: back()/home()/launch_app() will crash the phone agent when routed to a reverse agent; _send returning failure payloads will cause KeyError crashes in screenshot/current-app callers.
  3. Registry/DeviceManager lifecycle: deleting an agent does not remove it from DeviceManager, disconnect leaks ReverseAgentDevice instances, and claimed-but-never-connected agents are never pruned.
  4. Lost regression coverage: the cancelled-device-release tests should be restored.
  5. Runtime reliability: command failures are not propagated, accessibility actions may run off the main thread, and remove_agent may fail to close the WebSocket cleanly.

· Branch android-agent-unified
·

…tests

- ReverseAgentDevice._send now checks result.success and raises a clear error
  (with result.error) instead of returning an empty payload that crashes
  callers like get_screenshot/get_current_app with KeyError. (P1)
- ReverseAgentRegistry.remove_agent closes the WebSocket safely when called
  from a sync route: it records each session's event loop and uses
  run_coroutine_threadsafe when no loop is running in the caller thread,
  instead of asyncio.create_task (which raises off the event loop). (P1)
- DELETE /api/reverse_agents/registry/{id} now also removes the device from
  DeviceManager, so it no longer lingers in /api/devices as DISCONNECTED. (P2)
- DeviceManager.unregister_reverse_agent drops the ReverseAgentDevice instance
  so disconnect/reconnect cycles don't leak. (P2)
- _cleanup_stale_agents_locked now also prunes claimed-but-never-connected
  agents stuck in "paired" past the stale TTL. (P2)
- Restore the cancel-device-release regression tests (issue #172 / PR #317)
  that were dropped, and keep the new chat-cancel test (8 tests total). (P1)
@claude

claude Bot commented Jun 15, 2026

Copy link
Copy Markdown

Claude finished @suyiiyii's task in 2m 39s —— View job


Review complete

I read the server registry/API, device integration, Android native app, frontend sidebar, and contract tests at HEAD (9887d21). Several earlier P0/P1 issues from prior review rounds have been addressed: the Android HTTP server binds to 127.0.0.1, the accessibility service is not exported, heartbeat staleness falls back to connected_at, ReverseAgentDevice._send guards against event-loop-thread deadlock and checks result.success, claim attempts are rate-limited, a DELETE registry endpoint exists and synchronizes with DeviceManager, stale offline/paired agents are cleaned up, command types are validated, and the regression tests in test_cancel_device_release.py are restored.

The remaining blockers are mostly security boundaries, missing DeviceProtocol actions, and Android runtime reliability.

Findings

P0 — Pairing endpoints remain unauthenticated

  • Files: AutoGLM_GUI/api/reverse_agents.py:32-84, AutoGLM_GUI/reverse_agent_registry.py:108-171
  • What: POST /api/reverse_agents/pairings and POST /api/reverse_agents/pairings/claim are open to any network caller. Rate-limiting on claim only slows brute-force; creation is completely unrestricted.
  • Why it matters: Anyone who can reach the server can mint pairings, claim them, obtain a valid agent_token, and connect as a managed reverse-agent device. Once connected, the server routes control commands to it.
  • Fix: Require an existing session/API key to create pairings, or restrict pairing creation to localhost/admin callers. Keep claim rate-limiting but also bind it to a real user/session context. Fix this →

P1 — Android local HTTP server has no authentication

  • File: android-agent/app/src/main/java/com/autoglm/agent/http/AgentHttpServer.kt:16-164
  • What: AgentHttpServer binds to 127.0.0.1, but /device/$deviceId/tap, /swipe, /type_text, and /screenshot accept requests without any token check.
  • Why it matters: Any other app on the phone can capture the screen and inject touch input without user consent. This bypasses the token-based security of the reverse WebSocket channel.
  • Fix: Require the persisted agent_token on every control request (e.g., Authorization: Bearer <token> header or a token query param validated against ReverseAgentStore), and reject with 401/403 when missing. Fix this →

P1 — Agent token stored in plaintext SharedPreferences

  • File: android-agent/app/src/main/java/com/autoglm/agent/reverse/ReverseAgentStore.kt:15
  • What: ReverseAgentStore saves agentToken directly in SharedPreferences.
  • Why it matters: Any app with storage access or a rooted device can steal the token and impersonate the agent to the AutoGLM-GUI server.
  • Fix: Use EncryptedSharedPreferences from androidx.security:security-crypto. Fix this →

P1 — ReverseAgentDevice raises NotImplementedError for core agent actions

  • File: AutoGLM_GUI/devices/reverse_agent_device.py:135-151
  • What: back(), home(), and launch_app() raise NotImplementedError.
  • Why it matters: These are part of the DeviceProtocol contract and are commonly invoked by the phone agent. A normal task routed to a reverse agent will crash instead of completing or failing gracefully.
  • Fix: Add back, home, and launch_app command types to ReverseAgentCommandType and CommandExecutor, implement them on the Android side via performGlobalAction/startActivity, and make the device methods send those commands. Fix this →

P2 — WebSocket auth token is passed in the URL query string

  • Files: AutoGLM_GUI/api/reverse_agents.py:181, android-agent/app/src/main/java/com/autoglm/agent/reverse/ReverseAgentClient.kt:237
  • What: ?token=... is appended to the WebSocket URL.
  • Why it matters: URLs are routinely logged by proxies, load balancers, and server access logs, increasing the chance of token leakage.
  • Fix: Pass the token in a WebSocket subprotocol header (Sec-WebSocket-Protocol) or a custom header instead of the query string. Fix this →

P2 — Accessibility actions may run on the wrong thread

  • Files: android-agent/app/src/main/java/com/autoglm/agent/reverse/CommandExecutor.kt:9-69, ReverseAgentClient.kt:435-442
  • What: Commands are executed on scheduler.execute { ... }, a background thread. DeviceAccessibilityService calls dispatchGesture and reads rootInActiveWindow.
  • Why it matters: Some AccessibilityService APIs are documented as requiring calls from the main thread; running them on a background thread can cause IllegalStateException or undefined behavior on certain OEM ROMs.
  • Fix: Post accessibility actions to a Handler(Looper.getMainLooper()) and block the scheduler thread for the result. Fix this →

P2 — android:usesCleartextTraffic="true" is overly broad

  • File: android-agent/app/src/main/AndroidManifest.xml:14
  • What: The entire app opts into cleartext traffic.
  • Why it matters: Paired with the unauthenticated local HTTP server, this widens the window for network-based attacks. The WebSocket path can already be wss:// when the server uses HTTPS; the local HTTP server only serves 127.0.0.1, so a per-domain network_security_config.xml would be safer.
  • Fix: Replace the global flag with a networkSecurityConfig that permits cleartext only for 127.0.0.1 and the configured server base URL if needed.

P2 — Double-tap and clear-text are silently degraded

  • Files: AutoGLM_GUI/devices/reverse_agent_device.py:86-88, 130-133
  • What: double_tap() sends a single tap, and clear_text() sends type_text with an empty string. Neither matches the DeviceProtocol contract.
  • Why it matters: Callers expecting a real double-tap or text-field clear will get subtly wrong behavior, which is hard to debug.
  • Fix: Add a real double_tap command to ReverseAgentCommandType/CommandExecutor, and implement clear_text via the accessibility service (e.g., select-all + delete).

P2 — ScreenCaptureController.capture has a small race window

  • File: android-agent/app/src/main/java/com/autoglm/agent/projection/ScreenCaptureController.kt:130-140
  • What: After acquireLatestImage() returns null, the listener is installed. An image produced between the null check and listener installation sits in the ImageReader buffer until the next frame.
  • Fix: Set the listener first, then call acquireLatestImage() and wait if it is still null. Also consider raising maxImages above 2. Fix this →

P3 — Command round-trip tests use busy-wait polling

  • Files: tests/test_reverse_agents_api.py:193-206, tests/test_reverse_agent_device_manager.py:155-167
  • What: Both tests poll captured["last_sent"] in a 50×0.01 s loop instead of using an asyncio.Event.
  • Why it matters: The tests are slightly racy and slower than necessary; on a slow CI runner the 0.5 s window may be insufficient.
  • Fix: Signal with an asyncio.Event when the fake WebSocket receives the command, then wait on the event. Fix this →

Previously reported issues now resolved

  • test_cancel_device_release.py regression tests restored.
  • DeviceManager.unregister_reverse_agent now drops ReverseAgentDevice instances.
  • DELETE /api/reverse_agents/registry/{agent_id} removes the device from DeviceManager.
  • ReverseAgentDevice._send checks result.success and raises on failure.
  • _cleanup_stale_agents_locked prunes claimed-but-never-connected agents.
  • ✅ Heartbeat staleness falls back to connected_at.
  • remove_agent closes the WebSocket without requiring a running loop in the caller's thread.
  • ✅ Command type validation rejects unknown types.

Test observations

  • The contract tests in tests/test_reverse_agents_api.py and tests/test_reverse_agent_device_manager.py cover pairing, claim, heartbeat, stale detection, command round-trip, rate limiting, delete, and invalid command types. Good coverage for the happy path.
  • Codecov reports 112 uncovered lines, mostly in error paths of reverse_agents.py and reverse_agent_registry.py. Adding tests for unknown command results, wrong-agent results, and the delete/websocket-close exception path would close most of the gap.

Residual risk / summary

The reverse-agent pairing/registry foundation is solid and well-tested. At 9887d21, the main remaining risks are:

  1. Security boundaries: unauthenticated pairing creation/claim, unauthenticated local Android HTTP server, plaintext token storage, and token-in-URL WebSocket auth.
  2. Agent task crashes: back()/home()/launch_app() will crash the phone agent when routed to a reverse agent.
  3. Runtime reliability: accessibility actions may run off the main thread, screenshot capture has a small race window, and double-tap/clear-text do not match the DeviceProtocol contract.

· Branch android-agent-unified
·

# Conflicts:
#	docs/docs/configuration.md
#	docs/docs/intro.md
@claude

claude Bot commented Jun 21, 2026

Copy link
Copy Markdown

Claude finished @suyiiyii's task in 3m 57s —— View job


Review complete

I read the server registry/API, device integration, Android native app, frontend sidebar, and contract tests at the current HEAD. Several earlier P0/P1 issues have already been fixed in prior commits: the Android HTTP server binds to 127.0.0.1, the accessibility service is not exported, heartbeat staleness falls back to connected_at, ReverseAgentDevice._send guards against event-loop-thread deadlock and checks result.success, claim attempts are rate-limited, a DELETE registry endpoint exists and synchronizes with DeviceManager, stale offline/claimed-but-never-connected agents are cleaned up, command types are validated, and the regression tests in test_cancel_device_release.py are restored.

I was unable to run git diff origin/main...HEAD or the lint/test suite in this environment because those Bash commands require tool-execution approval; the review below is based on direct file reads.

Findings

P0 — Pairing endpoints remain unauthenticated

  • Files: AutoGLM_GUI/api/reverse_agents.py:32-84, AutoGLM_GUI/reverse_agent_registry.py:108-171
  • What: POST /api/reverse_agents/pairings and POST /api/reverse_agents/pairings/claim are open to any network caller. Rate-limiting on claim only slows brute-force; creation is completely unrestricted.
  • Why it matters: Anyone who can reach the server can mint pairings, claim them, obtain a valid agent_token, and connect as a managed reverse-agent device. Once connected, the server routes control commands to it.
  • Fix: Require an existing session/API key to create pairings, or restrict pairing creation to localhost/admin callers. Keep claim rate-limiting but also bind it to a real user/session context. Fix this →

P1 — Android local HTTP server has no authentication

  • File: android-agent/app/src/main/java/com/autoglm/agent/http/AgentHttpServer.kt:16-164
  • What: AgentHttpServer binds to 127.0.0.1, but /device/$deviceId/tap, /swipe, /type_text, and /screenshot accept requests without any token check.
  • Why it matters: Any other app on the phone can capture the screen and inject touch input without user consent. This bypasses the token-based security of the reverse WebSocket channel.
  • Fix: Require the persisted agent_token on every control request (e.g., Authorization: Bearer <token> header or a token query param validated against ReverseAgentStore), and reject with 401/403 when missing. Fix this →

P1 — Agent token stored in plaintext SharedPreferences

  • File: android-agent/app/src/main/java/com/autoglm/agent/reverse/ReverseAgentStore.kt:14-15
  • What: ReverseAgentStore saves agentToken directly in SharedPreferences.
  • Why it matters: Any app with storage access or a rooted device can steal the token and impersonate the agent to the AutoGLM-GUI server.
  • Fix: Use EncryptedSharedPreferences from androidx.security:security-crypto. Fix this →

P1 — ReverseAgentDevice raises NotImplementedError for core agent actions

  • File: AutoGLM_GUI/devices/reverse_agent_device.py:135-151
  • What: back(), home(), and launch_app() raise NotImplementedError.
  • Why it matters: These are part of the DeviceProtocol contract and are commonly invoked by the phone agent. A normal task routed to a reverse agent will crash instead of completing or failing gracefully.
  • Fix: Add back, home, and launch_app command types to ReverseAgentCommandType and CommandExecutor, implement them on the Android side via performGlobalAction/startActivity, and make the device methods send those commands. Fix this →

P2 — WebSocket auth token is passed in the URL query string

  • Files: AutoGLM_GUI/api/reverse_agents.py:181, android-agent/app/src/main/java/com/autoglm/agent/reverse/ReverseAgentClient.kt:237
  • What: ?token=... is appended to the WebSocket URL.
  • Why it matters: URLs are routinely logged by proxies, load balancers, and server access logs, increasing the chance of token leakage.
  • Fix: Pass the token in a WebSocket subprotocol header (Sec-WebSocket-Protocol) or a custom header instead of the query string. Fix this →

P2 — Accessibility actions may run on the wrong thread

  • Files: android-agent/app/src/main/java/com/autoglm/agent/reverse/CommandExecutor.kt:9-69, android-agent/app/src/main/java/com/autoglm/agent/service/DeviceAccessibilityService.kt:102-122
  • What: Commands are executed on scheduler.execute { ... }, a background thread with no Looper. DeviceAccessibilityService.dispatchGesture is called with a null handler, and rootInActiveWindow is read from that same background thread.
  • Why it matters: Some AccessibilityService APIs are documented as requiring calls from the main thread; running them on a background thread can cause IllegalStateException or undefined behavior on certain OEM ROMs.
  • Fix: Post accessibility actions to a Handler(Looper.getMainLooper()) and block the scheduler thread for the result. Fix this →

P2 — android:usesCleartextTraffic="true" is overly broad

  • File: android-agent/app/src/main/AndroidManifest.xml:14
  • What: The entire app opts into cleartext traffic.
  • Why it matters: Paired with the unauthenticated local HTTP server, this widens the window for network-based attacks. The WebSocket path can already be wss:// when the server uses HTTPS; the local HTTP server only serves 127.0.0.1, so a per-domain network_security_config.xml would be safer.
  • Fix: Replace the global flag with a networkSecurityConfig that permits cleartext only for 127.0.0.1 and the configured server base URL if needed.

P2 — Double-tap and clear-text are silently degraded

  • Files: AutoGLM_GUI/devices/reverse_agent_device.py:86-88, 130-133
  • What: double_tap() sends a single tap, and clear_text() sends type_text with an empty string. Neither matches the DeviceProtocol contract.
  • Why it matters: Callers expecting a real double-tap or text-field clear will get subtly wrong behavior, which is hard to debug.
  • Fix: Add a real double_tap command to ReverseAgentCommandType/CommandExecutor, and implement clear_text via the accessibility service (e.g., select-all + delete). Fix this →

P2 — ScreenCaptureController.capture has a small race window

  • File: android-agent/app/src/main/java/com/autoglm/agent/projection/ScreenCaptureController.kt:130-140
  • What: After acquireLatestImage() returns null, the listener is installed. An image produced between the null check and listener installation sits in the ImageReader buffer until the next frame.
  • Why it matters: With a 2-image buffer this is usually harmless, but under heavy load it can cause intermittent first-frame capture delays or timeouts.
  • Fix: Set the listener first, then call acquireLatestImage() and wait if it is still null. Also consider raising maxImages above 2. Fix this →

P3 — Command round-trip tests use busy-wait polling

  • Files: tests/test_reverse_agents_api.py:193-206, tests/test_reverse_agent_device_manager.py:155-167
  • What: Both tests poll captured["last_sent"] in a 50×0.01 s loop instead of using an asyncio.Event.
  • Why it matters: The tests are slightly racy and slower than necessary; on a slow CI runner the 0.5 s window may be insufficient.
  • Fix: Signal with an asyncio.Event when the fake WebSocket receives the command, then wait on the event. Fix this →

Previously reported issues now resolved

  • test_cancel_device_release.py regression tests restored.
  • DeviceManager.unregister_reverse_agent now drops ReverseAgentDevice instances.
  • DELETE /api/reverse_agents/registry/{agent_id} removes the device from DeviceManager.
  • ReverseAgentDevice._send checks result.success and raises on failure.
  • _cleanup_stale_agents_locked prunes claimed-but-never-connected agents.
  • ✅ Heartbeat staleness falls back to connected_at.
  • remove_agent closes the WebSocket without requiring a running loop in the caller's thread.
  • ✅ Command type validation rejects unknown types.

Test observations

  • The contract tests in tests/test_reverse_agents_api.py and tests/test_reverse_agent_device_manager.py cover pairing, claim, heartbeat, stale detection, command round-trip, rate limiting, delete, and invalid command types. Good coverage for the happy path.
  • Codecov reports ~112 uncovered lines, mostly in error paths of reverse_agents.py and reverse_agent_registry.py. Adding tests for unknown command results, wrong-agent results, and the delete/websocket-close exception path would close most of the gap.

Residual risk / summary

The reverse-agent pairing/registry foundation is solid and well-tested. At HEAD, the main remaining risks are:

  1. Security boundaries: unauthenticated pairing creation/claim, unauthenticated local Android HTTP server, plaintext token storage, and token-in-URL WebSocket auth.
  2. Agent task crashes: back()/home()/launch_app() will crash the phone agent when routed to a reverse agent.
  3. Runtime reliability: accessibility actions may run off the main thread, screenshot capture has a small race window, and double-tap/clear-text do not match the DeviceProtocol contract.

· Branch android-agent-unified
·

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant