Files
oh-my-openagent/src/tools/interactive-bash/tools.ts
Nguyen Khac Trung Kien b03e463bde fix: prevent zombie processes with proper process lifecycle management (#1306)
* fix: prevent zombie processes with proper process lifecycle management

- Await proc.exited for fire-and-forget spawns in tmux-utils.ts
- Remove competing process.exit() calls from LSP client and skill-mcp-manager
  signal handlers to let background-agent manager coordinate final exit
- Await process exit after kill() in interactive-bash timeout handler
- Await process exit after kill() in LSP client stop() method

These changes ensure spawned processes are properly reaped and prevent
orphan/zombie processes when running with tmux integration.

* fix: address Copilot review comments on process cleanup

- LSP cleanup: use async/sync split with Promise.allSettled for proper subprocess cleanup
- LSP stop(): make idempotent by nulling proc before await to prevent race conditions
- Interactive-bash timeout: use .then()/.catch() pattern instead of async callback to avoid unhandled rejections
- Skill-mcp-manager: use void+catch pattern for fire-and-forget signal handlers

* fix: address remaining Copilot review comments

- interactive-bash: reject timeout immediately, fire-and-forget zombie cleanup
- skill-mcp-manager: update comments to accurately describe signal handling strategy

* fix: address additional Copilot review comments

- LSP stop(): add 5s timeout to prevent indefinite hang on stuck processes
- tmux-utils: log warnings when pane title setting fails (both spawn/replace)
- BackgroundManager: delay process.exit() to next tick via setImmediate to allow other signal handlers to complete cleanup

* fix: address code review findings

- Increase exit delay from setImmediate to 100ms setTimeout to allow async cleanup
- Use asyncCleanup for SIGBREAK on Windows for consistency with SIGINT/SIGTERM
- Add try/catch around stderr read in spawnTmuxPane for consistency with replaceTmuxPane

* fix: address latest Copilot review comments

- LSP stop(): properly clear timeout when proc.exited wins the race
- BackgroundManager: use process.exitCode before delayed exit for cleaner shutdown
- spawnTmuxPane: remove redundant log import, reuse existing one

* fix: address latest Copilot review comments

- LSP stop(): escalate to SIGKILL on timeout, add logging
- tmux spawnTmuxPane/replaceTmuxPane: drain stderr immediately to avoid backpressure

* fix: address latest Copilot review comments

- Add .catch() to asyncCleanup() signal handlers to prevent unhandled rejections
- Await proc.exited after SIGKILL with 1s timeout to confirm termination

* fix: increase exit delay to 6s to accommodate LSP cleanup

LSP cleanup can take up to 5s (timeout) + 1s (SIGKILL wait), so the exit
delay must be at least 6s to ensure child processes are properly reaped.
2026-01-31 16:01:19 +09:00

136 lines
3.8 KiB
TypeScript

import { tool, type ToolDefinition } from "@opencode-ai/plugin/tool"
import { BLOCKED_TMUX_SUBCOMMANDS, DEFAULT_TIMEOUT_MS, INTERACTIVE_BASH_DESCRIPTION } from "./constants"
import { getCachedTmuxPath } from "./utils"
/**
* Quote-aware command tokenizer with escape handling
* Handles single/double quotes and backslash escapes without external dependencies
*/
export function tokenizeCommand(cmd: string): string[] {
const tokens: string[] = []
let current = ""
let inQuote = false
let quoteChar = ""
let escaped = false
for (let i = 0; i < cmd.length; i++) {
const char = cmd[i]
if (escaped) {
current += char
escaped = false
continue
}
if (char === "\\") {
escaped = true
continue
}
if ((char === "'" || char === '"') && !inQuote) {
inQuote = true
quoteChar = char
} else if (char === quoteChar && inQuote) {
inQuote = false
quoteChar = ""
} else if (char === " " && !inQuote) {
if (current) {
tokens.push(current)
current = ""
}
} else {
current += char
}
}
if (current) tokens.push(current)
return tokens
}
export const interactive_bash: ToolDefinition = tool({
description: INTERACTIVE_BASH_DESCRIPTION,
args: {
tmux_command: tool.schema.string().describe("The tmux command to execute (without 'tmux' prefix)"),
},
execute: async (args) => {
try {
const tmuxPath = getCachedTmuxPath() ?? "tmux"
const parts = tokenizeCommand(args.tmux_command)
if (parts.length === 0) {
return "Error: Empty tmux command"
}
const subcommand = parts[0].toLowerCase()
if (BLOCKED_TMUX_SUBCOMMANDS.includes(subcommand)) {
const sessionIdx = parts.findIndex(p => p === "-t" || p.startsWith("-t"))
let sessionName = "omo-session"
if (sessionIdx !== -1) {
if (parts[sessionIdx] === "-t" && parts[sessionIdx + 1]) {
sessionName = parts[sessionIdx + 1]
} else if (parts[sessionIdx].startsWith("-t")) {
sessionName = parts[sessionIdx].slice(2)
}
}
return `Error: '${parts[0]}' is blocked in interactive_bash.
**USE BASH TOOL INSTEAD:**
\`\`\`bash
# Capture terminal output
tmux capture-pane -p -t ${sessionName}
# Or capture with history (last 1000 lines)
tmux capture-pane -p -t ${sessionName} -S -1000
\`\`\`
The Bash tool can execute these commands directly. Do NOT retry with interactive_bash.`
}
const proc = Bun.spawn([tmuxPath, ...parts], {
stdout: "pipe",
stderr: "pipe",
})
const timeoutPromise = new Promise<never>((_, reject) => {
const id = setTimeout(() => {
const timeoutError = new Error(`Timeout after ${DEFAULT_TIMEOUT_MS}ms`)
try {
proc.kill()
// Fire-and-forget: wait for process exit in background to avoid zombies
void proc.exited.catch(() => {})
} catch {
// Ignore kill errors; we'll still reject with timeoutError below
}
reject(timeoutError)
}, DEFAULT_TIMEOUT_MS)
proc.exited
.then(() => clearTimeout(id))
.catch(() => clearTimeout(id))
})
// Read stdout and stderr in parallel to avoid race conditions
const [stdout, stderr, exitCode] = await Promise.race([
Promise.all([
new Response(proc.stdout).text(),
new Response(proc.stderr).text(),
proc.exited,
]),
timeoutPromise,
])
// Check exitCode properly - return error even if stderr is empty
if (exitCode !== 0) {
const errorMsg = stderr.trim() || `Command failed with exit code ${exitCode}`
return `Error: ${errorMsg}`
}
return stdout || "(no output)"
} catch (e) {
return `Error: ${e instanceof Error ? e.message : String(e)}`
}
},
})