Skip to content

Fix Android node system.run shell wrapper#73897

Closed
alixchow wants to merge 2 commits into
openclaw:mainfrom
alixchow:fix/android-node-termux-shell
Closed

Fix Android node system.run shell wrapper#73897
alixchow wants to merge 2 commits into
openclaw:mainfrom
alixchow:fix/android-node-termux-shell

Conversation

@alixchow

@alixchow alixchow commented Apr 29, 2026

Copy link
Copy Markdown

Title: Fix Android node system.run shell wrapper

Summary:

  • Use Termux's shell path for Android node system.run wrappers.
  • Keep Windows and other POSIX platforms unchanged.
  • Document the Android/Termux node-host shell behavior.

Problem:
Android/Termux does not provide /bin/sh. When an Android node advertises system.run, exec host=node builds a POSIX wrapper with /bin/sh -lc, causing execution to fail with spawn /bin/sh ENOENT.

Change:

  • Adds ANDROID_TERMUX_SHELL = /data/data/com.termux/files/usr/bin/sh.
  • buildNodeShellCommand now returns that shell when node platform is android.
  • Adds unit coverage for android and case/space normalization.
  • Adds docs notes in docs/cli/node.md and docs/nodes/index.md.

Validation:

  • git diff --check passed.
  • Full vitest was not run locally because dependency installation was killed by the host before node_modules completed.

@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation size: XS labels Apr 29, 2026
@greptile-apps

greptile-apps Bot commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes Android/Termux node hosts failing to launch system.run wrappers because /bin/sh does not exist on Termux. It adds ANDROID_TERMUX_SHELL constant, a platform check in buildNodeShellCommand, matching unit tests, and documentation updates. The implementation is straightforward and consistent with the existing Windows and POSIX branches.

Confidence Score: 5/5

Safe to merge — the change is minimal, well-tested, and isolated to the Android platform branch.

No logic errors, security issues, or missing cases. The normalization pipeline (trim → lowercase → equality check) is correct and tested for both exact and space-padded mixed-case inputs. The hardcoded Termux path is the canonical standard path. No regressions to Windows or POSIX paths.

No files require special attention.

Reviews (1): Last reviewed commit: "Fix Android node shell wrapper" | Re-trigger Greptile

@clawsweeper

clawsweeper Bot commented Apr 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Thanks for the context here. I swept through the related work, and this is now duplicate or superseded.

Keep open. Current main and v2026.6.10 still do not implement the Android/Termux shell behavior, but this PR is not merge-ready because the new Android shell branch can remain unreachable through current node-host platform metadata and gateway command policy, and the external PR still lacks real Android/Termux behavior proof.

Canonical path: Close this stale PR. The latest review rated it F, the branch still lacks merge-ready proof, and there has been no human follow-up after the durable review.

So I’m closing this here because the remaining work is already tracked in the canonical issue.

Review details

Best possible solution:

Close this stale PR. The latest review rated it F, the branch still lacks merge-ready proof, and there has been no human follow-up after the durable review.

Do we have a high-confidence way to reproduce the issue?

No for the full real Android/Termux workflow: no live Termux node-host run is shown. Source inspection shows both the helper-level /bin/sh problem and a current reachability blocker before that helper can fix the workflow.

Is this the best way to solve the issue?

No. The helper branch is a plausible piece, but the best fix must also cover Android/Termux platform metadata, gateway command-policy reachability, precise docs scope, and real runtime proof.

Security review:

Security review cleared: No concrete security or supply-chain regression was found; the diff adds no dependencies, workflows, secret handling, package-resolution changes, or broader permissions.

AGENTS.md: found and applied where relevant.

What I checked:

  • stale F-rated PR: PR was opened 2026-04-29T00:47:36Z, is older than 60 days, and the latest review rated it F.
  • proof blocker: real behavior proof is missing and proof tier is F, so this branch is not merge-ready without contributor follow-up.
  • no human follow-up: live comments and timeline hydrated by apply contain no non-automation activity after the ClawSweeper review.

Likely related people:

  • steipete: Prior commits enforced WS3 roles/node allowlists and hardened node metadata policy classification on the gateway command-policy surface that gates whether Android/Termux can invoke system.run. (role: feature-history owner; confidence: high; commits: 9dbc1435a6ca, 9005e8bc0a91, 0eac494db717; files: src/gateway/node-command-policy.ts, src/gateway/server-methods/nodes.ts, docs/nodes/index.md)
  • vincentkoc: Recent history touched node-host approval/gateway client seams and release provenance around the current shipped node-host behavior. (role: recent adjacent area contributor; confidence: medium; commits: 0f7d9c957093, fcb990e36950, aa69b12d0086; files: src/node-host/runner.ts, src/infra/node-shell.ts, src/gateway/node-command-policy.ts)
  • mbelinky: History around device pairing and phone-control node surfaces touched gateway node command policy and is adjacent to Android node reachability decisions. (role: adjacent node/mobile contributor; confidence: medium; commits: 730f86dd5c43; files: src/gateway/node-command-policy.ts, src/gateway/node-command-policy.test.ts)
  • zhang-guiping: Current-main blame for the shell helper, node-host platform mapper, and gateway command policy points to a broad recent commit; this is a weak routing signal rather than feature ownership. (role: recent current-main line contributor; confidence: low; commits: f12ade0082ad; files: src/infra/node-shell.ts, src/node-host/runner.ts, src/gateway/node-command-policy.ts)

Codex review notes: model internal, reasoning high; reviewed against ec737ee74d9b.

@alixchow

alixchow commented Apr 29, 2026

Copy link
Copy Markdown
Author

Targeted validation completed locally:\n\nbash\ncorepack pnpm exec vitest run --config test/vitest/vitest.unit-fast.config.ts src/infra/node-shell.test.ts\n\n\nResult: 1 test file passed, 3 tests passed.

@openclaw-barnacle openclaw-barnacle Bot added triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. plugin: file-transfer labels May 6, 2026
@openclaw-barnacle

openclaw-barnacle Bot commented May 30, 2026

Copy link
Copy Markdown

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

@openclaw-barnacle openclaw-barnacle Bot added the stale Marked as stale due to inactivity label May 30, 2026
@clawsweeper clawsweeper Bot added the rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. label May 30, 2026
@barnacle-openclaw barnacle-openclaw Bot removed the stale Marked as stale due to inactivity label May 30, 2026
@openclaw-barnacle

openclaw-barnacle Bot commented Jun 14, 2026

Copy link
Copy Markdown

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

@openclaw-barnacle openclaw-barnacle Bot added the stale Marked as stale due to inactivity label Jun 14, 2026
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. and removed rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. labels Jun 14, 2026
@openclaw-barnacle openclaw-barnacle Bot removed the stale Marked as stale due to inactivity label Jun 15, 2026
@clawsweeper clawsweeper Bot added the merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. label Jun 18, 2026
This was referenced Jun 19, 2026
Closed
Closed
@clawsweeper

clawsweeper Bot commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper applied the proposed close for this PR.

@clawsweeper clawsweeper Bot closed this Jun 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

docs Improvements or additions to documentation merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. P2 Normal backlog priority with limited blast radius. plugin: file-transfer rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: XS status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@alixchow