[PR #207] fix: ACP orphaned processes, garbled TUI, and missing tool details #199

New issue

Open

opened 2026-02-27 10:22:41 +03:00 by kerem · 0 comments

kerem commented

2026-02-27 10:22:41 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/mikeyobrien/ralph-orchestrator/pull/207
Author: @jsamuel1
Created: 2/27/2026
Status: 🔄 Open

Base: main ← Head: fix/acp-orphans-and-tool-display

📝 Commits (1)

d3552e8 fix: ACP orphaned processes, garbled TUI, and missing tool details (#204)

📊 Changes

6 files changed (+644 additions, -59 deletions)

View changed files

📝 crates/ralph-adapters/src/acp_executor.rs (+97 -16)
📝 crates/ralph-adapters/src/stream_handler.rs (+62 -7)
➕ crates/ralph-adapters/tests/acp_process_cleanup.rs (+241 -0)
📝 crates/ralph-cli/src/loop_runner.rs (+16 -0)
📝 crates/ralph-cli/src/main.rs (+1 -1)
📝 crates/ralph-tui/src/rpc_source.rs (+227 -35)

📄 Description

Fixes #204

Summary

Three categories of fixes for the kiro-acp backend:

Orphaned processes

Spawn ACP child in its own process group via process_group(0)
Kill entire process group (negative PID) on cleanup instead of just the direct child, preventing grandchild leaks (e.g. MCP servers)
Add ChildKillGuard drop guard for cleanup on future cancellation
Graceful shutdown: cancel ACP session + SIGTERM before SIGKILL

Garbled TUI / subprocess output

Suppress child stderr to prevent TUI corruption
Wire ACP executor to JsonRpcStreamHandler in RPC mode

Missing tool details in output

Skip initial empty ACP ToolCall notifications (no raw_input) so only the informative follow-up with parameters and descriptive title shows
Add lowercase ACP tool names (read, shell, ls, glob, grep) to format_tool_summary in both console and TUI handlers
Add generic fallback for unknown tool names using common input keys
Use ACP locations as fallback when raw_input is missing

Tests

Add integration tests for orphan cleanup across hat transitions

Root Causes

Orphans: All kill signals targeted the individual child PID, not the process group. Grandchildren (MCP servers) never received a signal.
Bare tool names: ACP sends two ToolCall events per tool — an initial empty notification, then a follow-up with actual data. The first produced [Tool] ls with no details. Additionally, format_tool_summary only matched PascalCase Claude names, not lowercase ACP names.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/mikeyobrien/ralph-orchestrator/pull/207 **Author:** [@jsamuel1](https://github.com/jsamuel1) **Created:** 2/27/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix/acp-orphans-and-tool-display` --- ### 📝 Commits (1) - [`d3552e8`](https://github.com/mikeyobrien/ralph-orchestrator/commit/d3552e89256c0cfb206adb067b0ed0c48712467a) fix: ACP orphaned processes, garbled TUI, and missing tool details (#204) ### 📊 Changes **6 files changed** (+644 additions, -59 deletions) <details> <summary>View changed files</summary> 📝 `crates/ralph-adapters/src/acp_executor.rs` (+97 -16) 📝 `crates/ralph-adapters/src/stream_handler.rs` (+62 -7) ➕ `crates/ralph-adapters/tests/acp_process_cleanup.rs` (+241 -0) 📝 `crates/ralph-cli/src/loop_runner.rs` (+16 -0) 📝 `crates/ralph-cli/src/main.rs` (+1 -1) 📝 `crates/ralph-tui/src/rpc_source.rs` (+227 -35) </details> ### 📄 Description Fixes #204 ## Summary Three categories of fixes for the kiro-acp backend: ### Orphaned processes - Spawn ACP child in its own process group via `process_group(0)` - Kill entire process group (negative PID) on cleanup instead of just the direct child, preventing grandchild leaks (e.g. MCP servers) - Add `ChildKillGuard` drop guard for cleanup on future cancellation - Graceful shutdown: cancel ACP session + SIGTERM before SIGKILL ### Garbled TUI / subprocess output - Suppress child stderr to prevent TUI corruption - Wire ACP executor to `JsonRpcStreamHandler` in RPC mode ### Missing tool details in output - Skip initial empty ACP `ToolCall` notifications (no `raw_input`) so only the informative follow-up with parameters and descriptive title shows - Add lowercase ACP tool names (`read`, `shell`, `ls`, `glob`, `grep`) to `format_tool_summary` in both console and TUI handlers - Add generic fallback for unknown tool names using common input keys - Use ACP locations as fallback when `raw_input` is missing ### Tests - Add integration tests for orphan cleanup across hat transitions ## Root Causes 1. **Orphans**: All kill signals targeted the individual child PID, not the process group. Grandchildren (MCP servers) never received a signal. 2. **Bare tool names**: ACP sends two `ToolCall` events per tool — an initial empty notification, then a follow-up with actual data. The first produced `[Tool] ls` with no details. Additionally, `format_tool_summary` only matched PascalCase Claude names, not lowercase ACP names. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>