Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,15 @@ class TermuxBridge(private val appContext: Context) {
suspend fun run(
command: String,
workdir: String? = null,
timeoutMs: Long = 12_000
timeoutMs: Long = 12_000,
background: Boolean = false
): Result {
if (!isInstalled()) return Result(
false, -1, "",
"Termux is not installed. Install it from F-Droid then enable RUN_COMMAND."
)
val first = dispatch(command, workdir, timeoutMs)
val effective = if (background) wrapBackground(command) else command
val first = dispatch(effective, workdir, timeoutMs)
if (workdir == null || !isWorkdirDenied(first)) return first

// Termux refused the WORKDIR because it lacks shared-storage access.
Expand All @@ -62,7 +64,7 @@ class TermuxBridge(private val appContext: Context) {
if (!autoSetupAttempted) {
autoSetupAttempted = true
triggerStorageBootstrap()
val fallback = dispatch(command, null, timeoutMs)
val fallback = dispatch(effective, null, timeoutMs)
val hint = "note: Termux couldn't read $workdir yet. Termux was " +
"just brought to the foreground and `termux-setup-storage` is " +
"on your clipboard — long-press in Termux, tap Paste, press " +
Expand All @@ -74,7 +76,7 @@ class TermuxBridge(private val appContext: Context) {

// Already tried auto-setup earlier — user likely skipped it. Keep
// working by falling back to $HOME and surface the manual fix.
val fallback = dispatch(command, null, timeoutMs)
val fallback = dispatch(effective, null, timeoutMs)
val hint = "note: Termux still can't read $workdir. Open Termux and " +
"run `termux-setup-storage`, then accept the Android permission " +
"dialog. Until then, prefer the workspace file tools over shell " +
Expand Down Expand Up @@ -207,6 +209,37 @@ class TermuxBridge(private val appContext: Context) {
if (isActive) resume(value)
}

// Wrap a long-running command so bash exits immediately and Termux
// broadcasts exit=0 within milliseconds, while the actual process keeps
// running in the background. stdout/stderr go to a log file under
// $HOME/.gemini-bg/ so the model can tail it later via a normal shell
// call (e.g. `tail -n 80 $LOG`). disown detaches the child so it
// survives Termux's RunCommandService tearing down.
private fun wrapBackground(command: String): String {
val id = UUID.randomUUID().toString().take(8)
val quoted = bashSingleQuote(command)
return buildString {
append("mkdir -p \"\$HOME/.gemini-bg\" && ")
append("LOG=\"\$HOME/.gemini-bg/run-$id.log\" && ")
Comment on lines +221 to +223

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (performance): Background log files under ~/.gemini-bg can grow without bound; consider a retention/cleanup strategy.

Each background run writes to a new file in ~/.gemini-bg/ with no rotation or pruning, so logs can grow without limit. Please consider adding a simple retention mechanism (e.g., keep only the last N logs, cap total directory size, or provide a cleanup command) to avoid long‑term disk usage issues.

Suggested change
return buildString {
append("mkdir -p \"\$HOME/.gemini-bg\" && ")
append("LOG=\"\$HOME/.gemini-bg/run-$id.log\" && ")
return buildString {
append("mkdir -p \"\$HOME/.gemini-bg\" && ")
// simple retention: keep only the 20 most recent background logs
append("{ ls -1t \"\$HOME/.gemini-bg\"/run-*.log 2>/dev/null | tail -n +21 | xargs -r rm -- 2>/dev/null || true ; } && ")
append("LOG=\"\$HOME/.gemini-bg/run-$id.log\" && ")

append("nohup bash -lc $quoted > \"\$LOG\" 2>&1 & ")
append("PID=\$! && ")
append("disown 2>/dev/null || true ; ")
append("sleep 0.2 ; ")
append("if kill -0 \$PID 2>/dev/null ; then ")
append(" echo \"[background] started pid=\$PID log=\$LOG\" ; ")
append("else ")
append(" echo \"[background] process died immediately; last log lines:\" ; ")
append(" tail -n 30 \"\$LOG\" 2>/dev/null || true ; ")
append(" exit 1 ; ")
append("fi")
}
}
Comment on lines +218 to +236

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Confirm bash precedence: is `A && B & C` parsed as `(A && B) & ; C` ?
# Expected: $! is the PID of the (A && B) group, not of B alone.
bash -c '
  true && sleep 3 & PID=$!
  echo "captured PID=$PID"
  ps -o pid,ppid,cmd -p $PID 2>/dev/null || echo "PID $PID not a direct sleep process"
  # Show children — if PID is a subshell wrapping nohup, sleep will be its child
  pgrep -P $PID | xargs -r ps -o pid,ppid,cmd -p
  wait
'

Repository: aciderix/gemini-android-app

Length of output: 174


🏁 Script executed:

#!/bin/bash

# Test 1: Verify bash operator precedence for `A && B & C`
# Does it parse as `(A && B) & ; C` or `(A && B & C)` ?
echo "=== Test 1: Operator Precedence ==="
bash -c '
  # If parsed as (A && B) & ; C then PID=$! captures the group
  # If parsed as (A && B & C) then the entire sequence is backgrounded
  true && (echo "B executed") & PID=$!
  echo "Captured PID=$PID"
  # Wait a moment for background job to complete
  sleep 0.5
  wait $PID 2>/dev/null && echo "Successfully waited for $PID"
'

# Test 2: More explicit test with process tree
echo -e "\n=== Test 2: What is $! when using && and & ? ==="
bash -c '
  # Create a marker file to track execution order
  true && sleep 10 & 
  PID=$!
  echo "Backgrounded PID=$PID"
  # Check if PID still exists
  if ps -p $PID > /dev/null 2>&1; then
    echo "PID $PID still running"
    ps -o pid,ppid,cmd -p $PID 2>/dev/null | head -5
  fi
  kill $PID 2>/dev/null
  wait 2>/dev/null
'

# Test 3: Verify the exact TermuxBridge pattern
echo -e "\n=== Test 3: Simulating TermuxBridge nohup pattern ==="
tmpdir=$(mktemp -d)
bash -c "
  mkdir -p \"$tmpdir\" && LOG=\"$tmpdir/test.log\" && nohup sleep 10 > \"\$LOG\" 2>&1 & PID=\$!
  echo \"Captured PID=\$PID\"
  # What process is $PID?
  ps -o pid,ppid,cmd -p \$PID 2>/dev/null | head -2
  # Cleanup
  kill \$PID 2>/dev/null || true
  wait 2>/dev/null || true
"
rm -rf "$tmpdir"

Repository: aciderix/gemini-android-app

Length of output: 296


🏁 Script executed:

#!/bin/bash
# Check if the code in the repository matches the review snippet
cat -n core-bridge/src/main/kotlin/com/gemini/bridge/termux/TermuxBridge.kt | sed -n '218,236p'

Repository: aciderix/gemini-android-app

Length of output: 1095


🏁 Script executed:

#!/bin/bash

echo "=== Detailed Test: Process Tree Analysis ==="

tmpdir=$(mktemp -d)

# Test the actual pattern with detailed process inspection
bash -c "
  # This simulates the exact TermuxBridge pattern
  mkdir -p \"$tmpdir\" && LOG=\"$tmpdir/test.log\" && nohup bash -c 'sleep 30' > \"\$LOG\" 2>&1 & PID=\$!
  echo \"Captured PID=\$PID\"
  
  # Get details about the captured PID
  echo \"Process info for captured PID \$PID:\"
  ps -o pid,ppid,sid,cmd -p \$PID 2>/dev/null || echo \"PID \$PID not found\"
  
  # Show all processes in this shell's process group
  echo \"All processes in current shell environment:\"
  ps -o pid,ppid,sid,cmd 2>/dev/null | grep -E '(bash|sleep|PID)' | head -20
  
  # Wait briefly and check again
  sleep 1
  echo \"After 1 second - checking if sleep is still under PID \$PID:\"
  pgrep -P \$PID 2>/dev/null || echo \"No children under PID \$PID\"
  
  # Cleanup
  kill -9 \$PID 2>/dev/null || true
  wait 2>/dev/null || true
" 2>&1

# Also test: what is the difference between capturing on the same line vs different lines?
echo -e "\n=== Comparison: Same line vs different line ==="
bash -c "
  mkdir -p \"$tmpdir\" && LOG=\"$tmpdir/test2.log\" && nohup bash -c 'sleep 30' > \"\$LOG\" 2>&1 &
  BASHPID_LINE=\$BASHPID
  PID_SAME=\$!
  
  # Separate capture on next line
  mkdir -p \"$tmpdir\" && LOG=\"$tmpdir/test3.log\" && nohup bash -c 'sleep 30' > \"\$LOG\" 2>&1 &
  PID_NEXT=\$!
  
  echo \"PID captured on same line (after &): \$PID_SAME\"
  echo \"PID captured on next line: \$PID_NEXT\"
  
  # Cleanup
  kill -9 \$PID_SAME \$PID_NEXT 2>/dev/null || true
  wait 2>/dev/null || true
"

rm -rf "$tmpdir"

Repository: aciderix/gemini-android-app

Length of output: 423


🏁 Script executed:

#!/bin/bash
# Search for related code and comments
rg -B5 -A10 "wrapBackground" core-bridge/src/main/kotlin/com/gemini/bridge/termux/TermuxBridge.kt

Repository: aciderix/gemini-android-app

Length of output: 1888


PID=$! captures the backgrounded subshell, not the nohup bash process.

In bash, & has lower precedence than &&. The line:

mkdir -p "$HOME/.gemini-bg" && LOG="…" && nohup bash -lc '…' > "$LOG" 2>&1 & PID=$! && disown …

parses as (mkdir && LOG=… && nohup bash … > "$LOG" 2>&1) & ; PID=$! && disown … — the entire chain runs in one backgrounded subshell. $! captures that subshell's PID, not the inner nohup bash process. Consequences:

  • nohup bash -lc '…' runs inside the subshell (not backgrounded again), so the subshell waits for it to complete before exiting.
  • $! is the temporary subshell's PID, which exits after launching nohup bash. The actual daemon process becomes a child of init.
  • The PID printed to the model ([background] started pid=$PID) is incorrect. When the model later runs kill <pid>, it targets a defunct subshell while the actual server continues running.
  • disown 2>/dev/null targets $! (the subshell), having no effect on the nohup child.

Suggested fix: don't chain with && across the &, and capture $! immediately after the nohup line:

🛠️ Proposed fix
     private fun wrapBackground(command: String): String {
         val id = UUID.randomUUID().toString().take(8)
         val quoted = bashSingleQuote(command)
         return buildString {
-            append("mkdir -p \"\$HOME/.gemini-bg\" && ")
-            append("LOG=\"\$HOME/.gemini-bg/run-$id.log\" && ")
-            append("nohup bash -lc $quoted > \"\$LOG\" 2>&1 & ")
-            append("PID=\$! && ")
-            append("disown 2>/dev/null || true ; ")
-            append("sleep 0.2 ; ")
+            append("mkdir -p \"\$HOME/.gemini-bg\" || exit 1 ; ")
+            append("LOG=\"\$HOME/.gemini-bg/run-$id.log\" ; ")
+            append("nohup bash -lc $quoted > \"\$LOG\" 2>&1 </dev/null & ")
+            append("PID=\$! ; ")
+            append("disown \$PID 2>/dev/null || true ; ")
+            append("sleep 0.2 ; ")
             append("if kill -0 \$PID 2>/dev/null ; then ")
             append("  echo \"[background] started pid=\$PID log=\$LOG\" ; ")
             append("else ")
             append("  echo \"[background] process died immediately; last log lines:\" ; ")
             append("  tail -n 30 \"\$LOG\" 2>/dev/null || true ; ")
             append("  exit 1 ; ")
             append("fi")
         }
     }

Also add </dev/null to prevent the backgrounded bash inheriting closed stdin from RunCommandService and receiving SIGTTIN on accidental reads.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@core-bridge/src/main/kotlin/com/gemini/bridge/termux/TermuxBridge.kt` around
lines 218 - 236, In TermuxBridge.wrapBackground, the current single-line chain
lets the entire chain be backgrounded so PID=$! captures the subshell, not the
actual nohup bash process; split the commands so you run mkdir and export LOG,
then run nohup bash -lc <quoted> > "$LOG" 2>&1 </dev/null & and immediately
capture PID=$! (then run disown 2>/dev/null || true and the kill-check logic
against that PID); also add the stdin redirection </dev/null to the nohup
invocation and ensure the subsequent tail/exit logic still references the same
LOG and PID.


// Escape a string for safe embedding inside a bash single-quoted word.
// Single quote → close quote, escaped quote, reopen quote.
private fun bashSingleQuote(s: String): String =
"'" + s.replace("'", "'\\''") + "'"

companion object {
private const val TERMUX_PKG = "com.termux"
private const val TERMUX_SERVICE = "com.termux.app.RunCommandService"
Expand Down
17 changes: 15 additions & 2 deletions core-bridge/src/main/kotlin/com/gemini/bridge/tools/ShellTool.kt
Original file line number Diff line number Diff line change
Expand Up @@ -25,22 +25,35 @@ class RunShellCommandTool(
"directory when it is reachable from Termux (the device path is shown in the " +
"system instruction); otherwise they run in Termux's \$HOME, so use the file " +
"tools to read/write workspace files. Requires Termux installed with the " +
"RUN_COMMAND permission granted. Returns combined stdout and stderr.",
"RUN_COMMAND permission granted. Returns combined stdout and stderr.\n\n" +
"Foreground commands must return within ~12 s or the call fails with a " +
"timeout. For anything long-running (dev/web servers, watchers, daemons), " +
"set `background: true` — the command is detached with nohup, its output " +
"is redirected to a log file under `~/.gemini-bg/`, and the result returns " +
"immediately with the PID and log path. You can then check progress with " +
"`tail -n 80 <log>` and stop the process with `kill <pid>`.",
category = ToolCategory.SHELL,
destructive = true,
parameters = objectParams(
"command" to stringProp("Command line to execute (e.g. `python script.py`)"),
"background" to booleanProp(
"Run in background. Use for servers/daemons/watchers that never return " +
"on their own. Returns once the process is confirmed alive (~200 ms), " +
"with its PID and log path."
),
required = listOf("command")
)
)

override suspend fun execute(call: ToolCall): ToolCallResult = runCatching {
val command = call.arguments["command"] as? String ?: error("command is required")
val background = call.arguments["background"] as? Boolean ?: false

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find how tool arguments are deserialized — do they land as typed JSON (Boolean/Number/String)
# or everything as String? Look at the JSON parser feeding ToolCall.arguments.
rg -nP --type=kotlin -C4 '\bToolCall\s*\(' 
rg -nP --type=kotlin -C3 'arguments\s*[:=]\s*Map'
rg -nP --type=kotlin -C5 'fun\s+\w*[Pp]arse\w*Tool|fromJson|decodeFromString.*Tool'

Repository: aciderix/gemini-android-app

Length of output: 4089


🏁 Script executed:

rg -nP --type=kotlin 'fun\s+jsonToMap|jsonToMap\s*\(' -A 10

Repository: aciderix/gemini-android-app

Length of output: 5582


🏁 Script executed:

cat -n core-bridge/src/main/kotlin/com/gemini/bridge/tools/ShellTool.kt | head -70 | tail -30

Repository: aciderix/gemini-android-app

Length of output: 1952


🏁 Script executed:

rg -nP --type=kotlin 'fun\s+booleanProp|class.*Prop' -A 5 | head -60

Repository: aciderix/gemini-android-app

Length of output: 688


background parameter silently defaults to false if the model sends a non-Boolean value (e.g., string "true").

The schema declares "type": "boolean", but booleanProp() is only a schema builder—it doesn't enforce runtime type validation. The jsonToMap() function preserves JSON types as-is, so call.arguments["background"] could be a Boolean, String, or Number depending on what the API returns. The current cast as? Boolean ?: false treats all non-Boolean values as false, creating a silent failure: a model sending "background": "true" would execute in foreground mode and hit the 12 s Termux timeout, making the failure harder to diagnose.

Consider accepting multiple types:

Suggested fix
-        val background = call.arguments["background"] as? Boolean ?: false
+        val background = when (val v = call.arguments["background"]) {
+            is Boolean -> v
+            is String -> v.equals("true", ignoreCase = true)
+            is Number -> v.toInt() != 0
+            else -> false
+        }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
val background = call.arguments["background"] as? Boolean ?: false
val background = when (val v = call.arguments["background"]) {
is Boolean -> v
is String -> v.equals("true", ignoreCase = true)
is Number -> v.toInt() != 0
else -> false
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@core-bridge/src/main/kotlin/com/gemini/bridge/tools/ShellTool.kt` at line 50,
The current silent cast of call.arguments["background"] to Boolean in
ShellTool.kt drops non-Boolean JSON values (e.g., "true") and defaults to false;
change the extraction to coerce/validate multiple types instead: read the raw
value from call.arguments["background"], then handle Boolean (use as-is), String
(parse case-insensitive "true"/"false"), and Number (treat 1 as true, 0 as
false), and for any other or unparseable value either log a warning (include the
raw value) and default to a safe choice or throw a clear validation exception;
update the binding where background is defined (the val background assignment
inside ShellTool) and include a short log/validation message referencing the raw
argument so mis-typed inputs are visible at runtime.

val workdir = workspace.absolutePath()
val reason = workspace.unreachableReason()
val r = termux.run(command, workdir)
val r = termux.run(command, workdir, background = background)
val body = buildString {
append("exit=").append(r.exitCode)
if (background) append(" mode=background")
if (workdir != null) append(" cwd=").append(workdir)
else append(" cwd=~ (workspace not reachable from Termux)")
append('\n')
Expand Down
Loading