feat(android): add http.request node invoke command#61483
feat(android): add http.request node invoke command#61483fredguile wants to merge 14 commits intoopenclaw:mainfrom
Conversation
- Add OpenClawCapability.Http and OpenClawHttpCommand to protocol constants - Add HttpHandler for executing HTTP requests from Android nodes - Wire httpHandler into InvokeDispatcher - Exposes 'http.request' command on Android nodes for gateway-initiated HTTP This enables Android nodes to act as HTTP proxies for the gateway, allowing web requests from networks that block datacenter IPs.
- Export NODE_HTTP_REQUEST_COMMAND constant from infra/node-commands.ts - Add http.request to Android platform defaults in node-command-policy.ts
Adds a Switch toggle in Settings > DATA ACCESS to enable/disable http.request capability. When disabled, the Android node will not advertise the http command to the gateway. Changes: - SecurePrefs: add httpAccessEnabled StateFlow + setter - InvokeCommandRegistry: add HttpEnabled availability + apply to Http capability/command - ConnectionManager: pass httpEnabled flag from prefs to NodeRuntimeFlags - MainViewModel: expose httpAccessEnabled - SettingsSheet: add HTTP Access toggle in DATA ACCESS section
- Remove garbage lines from failed sed insertion in InvokeCommandRegistry.kt - Add HttpEnabled to InvokeCommandAvailability enum - Add NodeCapabilityAvailability.HttpEnabled branch to advertisedCapabilities when - Add setHttpAccessEnabled to NodeRuntime.kt - Add setHttpAccessEnabled to MainViewModel.kt
openclaw-barnacle
bot
added
app: android
Greptile SummaryThis PR adds Key issues found during review:
Confidence Score: 2/5Not safe to merge as-is — unbounded heap buffering in HttpHandler is a real memory-safety risk on mobile, and the dispatcher availability check for HttpEnabled is a no-op stub. Score of 2 reflects two genuine logic issues (P0 memory risk + P1 missing runtime availability check) plus six test cases that don't test what they claim due to a missing URL factory injection seam. The feature's overall architecture is sound — opt-in default, Android-only allowlist, correct capability advertising — but these issues need fixing before landing. apps/android/app/src/main/java/ai/openclaw/app/node/HttpHandler.kt (memory buffering), apps/android/app/src/main/java/ai/openclaw/app/node/InvokeDispatcher.kt (HttpEnabled no-op), apps/android/app/src/test/java/ai/openclaw/app/HttpHandlerTest.kt (mock injection) Prompt To Fix All With AIThis is a comment left during a code review.
Path: apps/android/app/src/main/java/ai/openclaw/app/node/HttpHandler.kt
Line: 103-108
Comment:
**Full response body buffered in memory before truncation**
`reader.readText()` reads the entire HTTP response into memory before `.take(MAX_BODY_SIZE_BYTES)` is applied. A server returning a large body (e.g. hundreds of MB) will fill the device heap before any truncation occurs — the cap needs to be enforced during reading, not after.
A lightweight fix is to stop reading once the byte limit is reached:
```suggestion
val responseBody = try {
val inputStream = if (responseCode >= 400) connection.errorStream else connection.inputStream
if (inputStream != null) {
inputStream.use { stream ->
val buffer = ByteArray(MAX_BODY_SIZE_BYTES)
val bytesRead = stream.read(buffer, 0, MAX_BODY_SIZE_BYTES)
if (bytesRead > 0) String(buffer, 0, bytesRead, Charsets.UTF_8) else null
}
} else {
null
}
} catch (_: Throwable) {
null
}
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: apps/android/app/src/main/java/ai/openclaw/app/node/InvokeDispatcher.kt
Line: 332-333
Comment:
**`HttpEnabled` availability check is a no-op**
Unlike every other availability case (`CameraEnabled`, `LocationEnabled`, `CallLogAvailable`, etc.), `InvokeCommandAvailability.HttpEnabled` always returns `null` — it never checks whether HTTP is actually enabled at dispatch time. The `InvokeDispatcher` also has no `httpEnabled: () -> Boolean` lambda in its constructor, so there is no way to verify the runtime flag here.
If `http.request` arrives at the dispatcher while the user has just toggled HTTP access off (e.g. a request already in-flight when the preference changes), it will execute without restriction. Adding a `httpEnabled: () -> Boolean` lambda to the dispatcher constructor and wiring it through `NodeRuntime` (where `httpHandler` is already constructed) would close this gap and make the pattern consistent with every other capability check.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: apps/android/app/src/test/java/ai/openclaw/app/HttpHandlerTest.kt
Line: 29-50
Comment:
**Mock `URL`/`HttpURLConnection` objects are created but never injected**
In `handles_GET_request_successfully` and the five other tests that create `mockUrl = mock<URL>()` and `mockConnection = mock<HttpURLConnection>()`, the mock objects are set up but never passed to the handler. `HttpHandler.handleHttpRequest()` accepts only a JSON string, constructs its own `URL(url)` object internally via `validateUrl()`, and calls `.openConnection()` on that real instance — the mocked connection is entirely bypassed.
As a result those tests exercise real network I/O (or Robolectric's default HTTP shadow) rather than the intended mocked responses. Tests that verify status codes, headers, body content, or body truncation do not actually verify what they claim to verify.
The same problem exists in `respects_timeout_parameter`, `parses_headers_correctly`, `truncates_body_to_MAX_BODY_SIZE_BYTES`, `supports_POST_with_body`, and `returns_correct_status_and_statusText`.
A minimal fix is to add a `urlFactory: (String) -> URL = ::URL` parameter to `HttpHandler` so tests can inject `{ mockUrl }`.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: apps/android/app/src/main/java/ai/openclaw/app/SecurePrefs.kt
Line: 40
Comment:
**Indentation mismatch in companion object**
`httpAccessEnabledKey` uses 2-space indentation while every other `private const val` in the same companion object block uses 4 spaces.
```suggestion
private const val httpAccessEnabledKey = "http.access.enabled"
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix(android): add HttpEnabled to InvokeD..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| val responseBody = try { | ||
| val inputStream = if (responseCode >= 400) connection.errorStream else connection.inputStream | ||
| if (inputStream != null) { | ||
| inputStream.bufferedReader(Charsets.UTF_8).use { reader -> | ||
| reader.readText().take(MAX_BODY_SIZE_BYTES) | ||
| } |
There was a problem hiding this comment.
reader.readText() reads the entire HTTP response into memory before .take(MAX_BODY_SIZE_BYTES) is applied. A server returning a large body (e.g. hundreds of MB) will fill the device heap before any truncation occurs — the cap needs to be enforced during reading, not after.
A lightweight fix is to stop reading once the byte limit is reached:
| val responseBody = try { | |
| val inputStream = if (responseCode >= 400) connection.errorStream else connection.inputStream | |
| if (inputStream != null) { | |
| inputStream.bufferedReader(Charsets.UTF_8).use { reader -> | |
| reader.readText().take(MAX_BODY_SIZE_BYTES) | |
| } | |
| val responseBody = try { | |
| val inputStream = if (responseCode >= 400) connection.errorStream else connection.inputStream | |
| if (inputStream != null) { | |
| inputStream.use { stream -> | |
| val buffer = ByteArray(MAX_BODY_SIZE_BYTES) | |
| val bytesRead = stream.read(buffer, 0, MAX_BODY_SIZE_BYTES) | |
| if (bytesRead > 0) String(buffer, 0, bytesRead, Charsets.UTF_8) else null | |
| } | |
| } else { | |
| null | |
| } | |
| } catch (_: Throwable) { | |
| null | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/android/app/src/main/java/ai/openclaw/app/node/HttpHandler.kt
Line: 103-108
Comment:
**Full response body buffered in memory before truncation**
`reader.readText()` reads the entire HTTP response into memory before `.take(MAX_BODY_SIZE_BYTES)` is applied. A server returning a large body (e.g. hundreds of MB) will fill the device heap before any truncation occurs — the cap needs to be enforced during reading, not after.
A lightweight fix is to stop reading once the byte limit is reached:
```suggestion
val responseBody = try {
val inputStream = if (responseCode >= 400) connection.errorStream else connection.inputStream
if (inputStream != null) {
inputStream.use { stream ->
val buffer = ByteArray(MAX_BODY_SIZE_BYTES)
val bytesRead = stream.read(buffer, 0, MAX_BODY_SIZE_BYTES)
if (bytesRead > 0) String(buffer, 0, bytesRead, Charsets.UTF_8) else null
}
} else {
null
}
} catch (_: Throwable) {
null
}
```
How can I resolve this? If you propose a fix, please make it concise.| InvokeCommandAvailability.HttpEnabled -> | ||
| null |
There was a problem hiding this comment.
HttpEnabled availability check is a no-op
Unlike every other availability case (CameraEnabled, LocationEnabled, CallLogAvailable, etc.), InvokeCommandAvailability.HttpEnabled always returns null — it never checks whether HTTP is actually enabled at dispatch time. The InvokeDispatcher also has no httpEnabled: () -> Boolean lambda in its constructor, so there is no way to verify the runtime flag here.
If http.request arrives at the dispatcher while the user has just toggled HTTP access off (e.g. a request already in-flight when the preference changes), it will execute without restriction. Adding a httpEnabled: () -> Boolean lambda to the dispatcher constructor and wiring it through NodeRuntime (where httpHandler is already constructed) would close this gap and make the pattern consistent with every other capability check.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/android/app/src/main/java/ai/openclaw/app/node/InvokeDispatcher.kt
Line: 332-333
Comment:
**`HttpEnabled` availability check is a no-op**
Unlike every other availability case (`CameraEnabled`, `LocationEnabled`, `CallLogAvailable`, etc.), `InvokeCommandAvailability.HttpEnabled` always returns `null` — it never checks whether HTTP is actually enabled at dispatch time. The `InvokeDispatcher` also has no `httpEnabled: () -> Boolean` lambda in its constructor, so there is no way to verify the runtime flag here.
If `http.request` arrives at the dispatcher while the user has just toggled HTTP access off (e.g. a request already in-flight when the preference changes), it will execute without restriction. Adding a `httpEnabled: () -> Boolean` lambda to the dispatcher constructor and wiring it through `NodeRuntime` (where `httpHandler` is already constructed) would close this gap and make the pattern consistent with every other capability check.
How can I resolve this? If you propose a fix, please make it concise.| fun handles_GET_request_successfully() { | ||
| val mockConnection = mock<HttpURLConnection>() | ||
| whenever(mockConnection.responseCode).thenReturn(200) | ||
| whenever(mockConnection.responseMessage).thenReturn("OK") | ||
| whenever(mockConnection.headerFields).thenReturn( | ||
| mapOf( | ||
| "Content-Type" to listOf("application/json"), | ||
| "X-Custom" to listOf("value") | ||
| ) | ||
| ) | ||
| whenever(mockConnection.inputStream).thenReturn(ByteArrayInputStream("""{"success":true}""".toByteArray())) | ||
|
|
||
| val mockUrl = mock<URL>() | ||
| whenever(mockUrl.openConnection()).thenReturn(mockConnection) | ||
|
|
||
| val result = handler.handleHttpRequest("""{"url":"http://example.com/api","method":"GET"}""") | ||
|
|
||
| assertNotNull(result) | ||
| assertTrue(result.ok) | ||
| assertTrue(result.payload?.contains("\"status\":200") ?: false) | ||
| assertTrue(result.payload?.contains("\"statusText\":\"OK\"") ?: false) | ||
| } |
There was a problem hiding this comment.
URL/HttpURLConnection objects are created but never injected
In handles_GET_request_successfully and the five other tests that create mockUrl = mock<URL>() and mockConnection = mock<HttpURLConnection>(), the mock objects are set up but never passed to the handler. HttpHandler.handleHttpRequest() accepts only a JSON string, constructs its own URL(url) object internally via validateUrl(), and calls .openConnection() on that real instance — the mocked connection is entirely bypassed.
As a result those tests exercise real network I/O (or Robolectric's default HTTP shadow) rather than the intended mocked responses. Tests that verify status codes, headers, body content, or body truncation do not actually verify what they claim to verify.
The same problem exists in respects_timeout_parameter, parses_headers_correctly, truncates_body_to_MAX_BODY_SIZE_BYTES, supports_POST_with_body, and returns_correct_status_and_statusText.
A minimal fix is to add a urlFactory: (String) -> URL = ::URL parameter to HttpHandler so tests can inject { mockUrl }.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/android/app/src/test/java/ai/openclaw/app/HttpHandlerTest.kt
Line: 29-50
Comment:
**Mock `URL`/`HttpURLConnection` objects are created but never injected**
In `handles_GET_request_successfully` and the five other tests that create `mockUrl = mock<URL>()` and `mockConnection = mock<HttpURLConnection>()`, the mock objects are set up but never passed to the handler. `HttpHandler.handleHttpRequest()` accepts only a JSON string, constructs its own `URL(url)` object internally via `validateUrl()`, and calls `.openConnection()` on that real instance — the mocked connection is entirely bypassed.
As a result those tests exercise real network I/O (or Robolectric's default HTTP shadow) rather than the intended mocked responses. Tests that verify status codes, headers, body content, or body truncation do not actually verify what they claim to verify.
The same problem exists in `respects_timeout_parameter`, `parses_headers_correctly`, `truncates_body_to_MAX_BODY_SIZE_BYTES`, `supports_POST_with_body`, and `returns_correct_status_and_statusText`.
A minimal fix is to add a `urlFactory: (String) -> URL = ::URL` parameter to `HttpHandler` so tests can inject `{ mockUrl }`.
How can I resolve this? If you propose a fix, please make it concise.| private const val notificationsForwardingMaxEventsPerMinuteKey = | ||
| "notifications.forwarding.maxEventsPerMinute" | ||
| private const val notificationsForwardingSessionKeyKey = "notifications.forwarding.sessionKey" | ||
| private const val httpAccessEnabledKey = "http.access.enabled" |
There was a problem hiding this comment.
httpAccessEnabledKey uses 2-space indentation while every other private const val in the same companion object block uses 4 spaces.
| private const val httpAccessEnabledKey = "http.access.enabled" | |
| private const val httpAccessEnabledKey = "http.access.enabled" |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/android/app/src/main/java/ai/openclaw/app/SecurePrefs.kt
Line: 40
Comment:
**Indentation mismatch in companion object**
`httpAccessEnabledKey` uses 2-space indentation while every other `private const val` in the same companion object block uses 4 spaces.
```suggestion
private const val httpAccessEnabledKey = "http.access.enabled"
```
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2a54f496f4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| InvokeCommandAvailability.HttpEnabled -> | ||
| null |
There was a problem hiding this comment.
InvokeCommandAvailability.HttpEnabled currently returns null unconditionally, so http.request is still executable even after the user turns HTTP Access off. This is reachable because setHttpAccessEnabled only updates prefs and does not force a reconnect, so an already-connected node can keep receiving http.request invokes that were previously declared. Unlike camera/location checks, the runtime toggle is not actually enforced at dispatch time.
Useful? React with 👍 / 👎.
| inputStream.bufferedReader(Charsets.UTF_8).use { reader -> | ||
| reader.readText().take(MAX_BODY_SIZE_BYTES) | ||
| } |
There was a problem hiding this comment.
The code claims to cap response bodies at 5 MB, but reader.readText().take(MAX_BODY_SIZE_BYTES) reads the entire response into memory first and only truncates afterward. When a target URL returns a large payload, this can cause high memory usage or OOM on the device despite the advertised cap, which is especially risky for a remotely-invoked command.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: deefba59c9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| val mockUrl = mock<URL>() | ||
| whenever(mockUrl.openConnection()).thenReturn(mockConnection) | ||
|
|
||
| val result = handler.handleHttpRequest("""{"url":"http://example.com/api","method":"GET"}""") |
There was a problem hiding this comment.
This test sets up mockUrl and mockConnection, but handleHttpRequest builds its own URL from the JSON string, so the mock is never used and the call at runtime goes to the real network (example.com). That makes assertions like status: 200 non-deterministic (and often incorrect), leading to flaky/failing unit tests and weak coverage of the handler logic.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b73d0a710b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
apps/android/app/src/main/java/ai/openclaw/app/node/HttpHandler.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6a9f3cab76
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| fun setHttpAccessEnabled(value: Boolean) { | ||
| prefs.setHttpAccessEnabled(value) | ||
| } |
There was a problem hiding this comment.
This setter only writes the preference, so an already-connected node keeps its original advertised command list for the current session. Because http.request is advertised from ConnectionManager.buildNodeConnectOptions() at connect time, enabling HTTP Access after the node is connected does not make the command invokable until a manual reconnect occurs, which makes the new toggle appear broken on the default off→on path.
Useful? React with 👍 / 👎.
1 participant
Summary
Adds
http.requestas a new node invoke command for Android nodes. This enables an Android device connected as an OpenClaw node to act as an HTTP proxy for the gateway — useful for fetching URLs from networks that block datacenter IPs (e.g., Cloudflare-protected sites).Loom QA demo: https://www.loom.com/share/caafc3e3786e49b7a3e607aa00b204cd
Changes
protocol/OpenClawProtocolConstants.ktHttp(http)capability +OpenClawHttpCommandenumnode/HttpHandler.ktnode/InvokeCommandRegistry.kthttpin capabilities +http.requestin commandsnode/InvokeDispatcher.kthttp.request→HttpHandlerNodeRuntime.ktHttpHandlerand pass to dispatcherAPI
{ "url": "https://example.com", "method": "GET", // GET|POST|PUT|PATCH|DELETE|HEAD|OPTIONS "headers": { "Auth": "Bearer ..." }, "body": "{ ... }", // optional "timeout": 30000 // optional, ms, default 30000, max 120000 }Response:
{ "ok": true, "status": 200, "statusText": "OK", "headers": { "Content-Type": "text/html" }, "body": "<!DOCTYPE html>..." }Test plan
http.requestfrom gateway with a known-blocked URL (testing with https://www.zonebourse.com/)Notes
AI-generated using OpenClaw + OpenCode ACP. Fully tested.