mirror of
https://github.com/hanzoai/dev.git
synced 2026-10-25 03:32:32 +00:00
fix(core/exec): kill child on task abort to avoid orphaned commands\n\nWrap spawned child in KillOnDrop guard so user interrupts (which abort the agent\ntask) terminate the process immediately. This ensures the UI no longer shows\ncommands as still running after cancel, and the OS process is not left alive.
This commit is contained in:
@@ -33,7 +33,11 @@ async fn discover_ws_via_port(port: u16) -> Result<String> {
|
||||
debug!("Requesting Chrome version info from: {}", url);
|
||||
|
||||
let client_start = tokio::time::Instant::now();
|
||||
// Do not respect system proxy env vars for local CDP discovery.
|
||||
// Corporate proxies frequently intercept 127.0.0.1 and break discovery with
|
||||
// opaque "error sending request" failures. We always talk to localhost here.
|
||||
let client = Client::builder()
|
||||
.no_proxy()
|
||||
.timeout(Duration::from_secs(5)) // Allow Chrome time to bring up /json/version on fresh launch
|
||||
.build()
|
||||
.map_err(|e| BrowserError::CdpError(format!("Failed to build HTTP client: {}", e)))?;
|
||||
@@ -120,7 +124,9 @@ async fn scan_for_chrome_debug_port() -> Option<u16> {
|
||||
for port in found_ports {
|
||||
let test_future = async move {
|
||||
let url = format!("http://127.0.0.1:{}/json/version", port);
|
||||
// Skip proxy env for localhost checks; see note above.
|
||||
let client = Client::builder()
|
||||
.no_proxy()
|
||||
.timeout(Duration::from_millis(200)) // Shorter timeout for parallel tests
|
||||
.build()
|
||||
.ok()?;
|
||||
|
||||
@@ -6,8 +6,14 @@ Code is a fast, community-driven fork focused on key developer ergonomics: Brows
|
||||
|
||||
This version has a few key changes and additions.
|
||||
|
||||
## Preamble & Focus
|
||||
You should provide fewer preamble messages and focus on completing the task as quickly as possible (once you fully understand it). You should attempt to work autonomously as much as possible and ask for input only when you can not proceed further, or the task is complete.
|
||||
## Autonomy
|
||||
You should be highly autonomous. Avoid asking the user to perform commands, perform research or run tests. You should implement these things yourself. Use your `agent_run` command to delegate tasks to sub-agents, particularly for tasks which can be performed in parallel such as setting up a testing script to validate changes. Work until the task is complete. This may take many hours.
|
||||
|
||||
## Preamble
|
||||
You should provide fewer preamble messages and focus on completing the task as quickly as possible (once you fully understand it). Use parallel calls and agents to speed up work.
|
||||
|
||||
## Exploration
|
||||
Understand every task you are given. Don't make assumptions and use your search tools to explore the problem space. Assigning agents to research is also a fantastic use for them!
|
||||
|
||||
## Testing
|
||||
With your additional browser tools you can validate web UI easily. For code that generates a web interface, always test with browser tools after changes and use your visual judgment to improve UX. You should always generate aesthetically pleasing interfaces with great UX.
|
||||
|
||||
@@ -268,7 +268,7 @@ async fn exec(
|
||||
/// Consumes the output of a child process, truncating it so it is suitable for
|
||||
/// use as the output of a `shell` tool call. Also enforces specified timeout.
|
||||
async fn consume_truncated_output(
|
||||
mut child: Child,
|
||||
child: Child,
|
||||
timeout: Duration,
|
||||
stdout_stream: Option<StdoutStream>,
|
||||
) -> Result<RawExecToolCallOutput> {
|
||||
@@ -276,12 +276,14 @@ async fn consume_truncated_output(
|
||||
// above, therefore `take()` should normally return `Some`. If it doesn't
|
||||
// we treat it as an exceptional I/O error
|
||||
|
||||
let stdout_reader = child.stdout.take().ok_or_else(|| {
|
||||
let mut killer = KillOnDrop::new(child);
|
||||
|
||||
let stdout_reader = killer.as_mut().stdout.take().ok_or_else(|| {
|
||||
CodexErr::Io(io::Error::other(
|
||||
"stdout pipe was unexpectedly not available",
|
||||
))
|
||||
})?;
|
||||
let stderr_reader = child.stderr.take().ok_or_else(|| {
|
||||
let stderr_reader = killer.as_mut().stderr.take().ok_or_else(|| {
|
||||
CodexErr::Io(io::Error::other(
|
||||
"stderr pipe was unexpectedly not available",
|
||||
))
|
||||
@@ -303,24 +305,28 @@ async fn consume_truncated_output(
|
||||
));
|
||||
|
||||
let exit_status = tokio::select! {
|
||||
result = tokio::time::timeout(timeout, child.wait()) => {
|
||||
result = tokio::time::timeout(timeout, killer.as_mut().wait()) => {
|
||||
match result {
|
||||
Ok(Ok(exit_status)) => exit_status,
|
||||
Ok(e) => e?,
|
||||
Err(_) => {
|
||||
// timeout
|
||||
child.start_kill()?;
|
||||
killer.as_mut().start_kill()?;
|
||||
// Debatable whether `child.wait().await` should be called here.
|
||||
synthetic_exit_status(EXIT_CODE_SIGNAL_BASE + TIMEOUT_CODE)
|
||||
}
|
||||
}
|
||||
}
|
||||
_ = tokio::signal::ctrl_c() => {
|
||||
child.start_kill()?;
|
||||
killer.as_mut().start_kill()?;
|
||||
synthetic_exit_status(EXIT_CODE_SIGNAL_BASE + SIGKILL_CODE)
|
||||
}
|
||||
};
|
||||
|
||||
// Disarm killer now that we've observed process termination status to
|
||||
// avoid re-sending a kill signal during Drop.
|
||||
killer.disarm();
|
||||
|
||||
let stdout = stdout_handle.await??;
|
||||
let stderr = stderr_handle.await??;
|
||||
|
||||
@@ -403,6 +409,27 @@ fn synthetic_exit_status(code: i32) -> ExitStatus {
|
||||
std::process::ExitStatus::from_raw(code)
|
||||
}
|
||||
|
||||
/// Guard that ensures a spawned child process is terminated if the owning
|
||||
/// future is dropped before the child has exited. This prevents orphaned
|
||||
/// processes when a running turn is interrupted (e.g., user presses Esc or
|
||||
/// Ctrl+C) and the task executing the command is aborted.
|
||||
struct KillOnDrop {
|
||||
child: Option<Child>,
|
||||
}
|
||||
|
||||
impl KillOnDrop {
|
||||
fn new(child: Child) -> Self { Self { child: Some(child) } }
|
||||
fn as_mut(&mut self) -> &mut Child { self.child.as_mut().expect("child present") }
|
||||
fn disarm(&mut self) { self.child = None; }
|
||||
}
|
||||
|
||||
impl Drop for KillOnDrop {
|
||||
fn drop(&mut self) {
|
||||
if let Some(child) = self.child.as_mut() {
|
||||
let _ = child.start_kill();
|
||||
}
|
||||
}
|
||||
}
|
||||
#[cfg(windows)]
|
||||
fn synthetic_exit_status(code: i32) -> ExitStatus {
|
||||
use std::os::windows::process::ExitStatusExt;
|
||||
|
||||
+101
-11
@@ -147,6 +147,8 @@ pub(crate) struct ChatWidget<'a> {
|
||||
running_custom_tools: HashMap<String, usize>,
|
||||
// Track running web searches by call_id -> (history index, query)
|
||||
running_web_search: HashMap<String, (usize, Option<String>)>,
|
||||
// Single live aggregation block for consecutive Read actions
|
||||
running_read_agg_index: Option<usize>,
|
||||
live_builder: RowBuilder,
|
||||
// Store pending image paths keyed by their placeholder text
|
||||
pending_images: HashMap<String, PathBuf>,
|
||||
@@ -579,6 +581,19 @@ impl ChatWidget<'_> {
|
||||
if !any_tasks_active {
|
||||
self.bottom_pane.set_task_running(false);
|
||||
}
|
||||
// Finalize a running Read aggregation if present to avoid dangling "Read" running block
|
||||
if let Some(idx) = self.running_read_agg_index.take() {
|
||||
if idx < self.history_cells.len() {
|
||||
if let Some(agg) = self.history_cells[idx]
|
||||
.as_any_mut()
|
||||
.downcast_mut::<history_cell::ReadAggregationCell>()
|
||||
{
|
||||
agg.finalize();
|
||||
self.invalidate_height_cache();
|
||||
self.request_redraw();
|
||||
}
|
||||
}
|
||||
}
|
||||
// Re-check global idle state
|
||||
self.maybe_hide_spinner();
|
||||
}
|
||||
@@ -655,6 +670,19 @@ impl ChatWidget<'_> {
|
||||
self.running_web_search.remove(&call_id);
|
||||
}
|
||||
}
|
||||
// Also finalize a running Read aggregation block, if any
|
||||
if let Some(idx) = self.running_read_agg_index.take() {
|
||||
if idx < self.history_cells.len() {
|
||||
if let Some(agg) = self.history_cells[idx]
|
||||
.as_any_mut()
|
||||
.downcast_mut::<history_cell::ReadAggregationCell>()
|
||||
{
|
||||
agg.finalize();
|
||||
self.invalidate_height_cache();
|
||||
self.request_redraw();
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
fn perf_label_for_item(&self, item: &dyn HistoryCell) -> String {
|
||||
use crate::history_cell::{ExecKind, ExecStatus, HistoryCellType, PatchKind, ToolStatus};
|
||||
@@ -1041,21 +1069,49 @@ impl ChatWidget<'_> {
|
||||
// Still track run lifecycle for layout/metrics
|
||||
self.height_manager.borrow_mut().record_event(HeightEvent::RunBegin);
|
||||
|
||||
// For read/search/list actions: don't insert a running cell in history.
|
||||
// Track the command so End can append the finalized result (and merge).
|
||||
if matches!(action, "read" | "search" | "list") {
|
||||
// For read actions: maintain a single live aggregation cell that gets
|
||||
// updated as individual read commands begin, then flip it to a finalized
|
||||
// state when the last read completes.
|
||||
if action == "read" {
|
||||
// Track this call without placing an individual running cell
|
||||
self.running_commands.insert(
|
||||
ev.call_id.clone(),
|
||||
RunningCommand { command: ev.command.clone(), parsed: parsed_command, history_index: None },
|
||||
RunningCommand { command: ev.command.clone(), parsed: parsed_command.clone(), history_index: None },
|
||||
);
|
||||
// Update concise status text without creating a history section
|
||||
let status = match action {
|
||||
"read" => "reading files…",
|
||||
"search" => "searching…",
|
||||
"list" => "listing files…",
|
||||
_ => "working…",
|
||||
|
||||
// Ensure aggregation cell exists
|
||||
let agg_index = match self.running_read_agg_index {
|
||||
Some(idx) if idx < self.history_cells.len()
|
||||
&& self.history_cells[idx]
|
||||
.as_any()
|
||||
.downcast_ref::<history_cell::ReadAggregationCell>()
|
||||
.is_some() => Some(idx),
|
||||
_ => None,
|
||||
};
|
||||
self.bottom_pane.update_status_text(status.to_string());
|
||||
let idx = if let Some(i) = agg_index {
|
||||
i
|
||||
} else {
|
||||
let before_len = self.history_cells.len();
|
||||
self.add_to_history(history_cell::ReadAggregationCell::new());
|
||||
let i = if self.history_cells.len() > 0 { self.history_cells.len() - 1 } else { before_len };
|
||||
self.running_read_agg_index = Some(i);
|
||||
i
|
||||
};
|
||||
|
||||
// Derive preamble lines for this read from a temp running ExecCell and drop header
|
||||
let tmp = history_cell::new_active_exec_command(ev.command.clone(), parsed_command.clone());
|
||||
let mut lines = tmp.display_lines();
|
||||
if !lines.is_empty() { lines.remove(0); }
|
||||
if let Some(agg) = self.history_cells[idx]
|
||||
.as_any_mut()
|
||||
.downcast_mut::<history_cell::ReadAggregationCell>()
|
||||
{
|
||||
agg.push_lines(lines);
|
||||
self.invalidate_height_cache();
|
||||
self.request_redraw();
|
||||
}
|
||||
|
||||
self.bottom_pane.update_status_text("reading files…".to_string());
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -1117,6 +1173,38 @@ impl ChatWidget<'_> {
|
||||
.map(|cmd| (cmd.command, cmd.parsed, cmd.history_index))
|
||||
.unwrap_or_else(|| (vec![call_id.clone()], vec![], None));
|
||||
|
||||
// Handle completed Read commands by finalizing the aggregation cell
|
||||
let action = history_cell::action_from_parsed(&parsed);
|
||||
if action == "read" {
|
||||
// Flip aggregation to finalized if this was the last active read
|
||||
let any_read_running = self
|
||||
.running_commands
|
||||
.values()
|
||||
.any(|rc| history_cell::action_from_parsed(&rc.parsed) == "read");
|
||||
if !any_read_running {
|
||||
if let Some(idx) = self.running_read_agg_index.take() {
|
||||
if idx < self.history_cells.len() {
|
||||
if let Some(agg) = self.history_cells[idx]
|
||||
.as_any_mut()
|
||||
.downcast_mut::<history_cell::ReadAggregationCell>()
|
||||
{
|
||||
agg.finalize();
|
||||
self.invalidate_height_cache();
|
||||
self.request_redraw();
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
// Update status and drop spinner if idle
|
||||
if exit_code == 0 {
|
||||
self.bottom_pane.update_status_text("files read".to_string());
|
||||
} else {
|
||||
self.bottom_pane.update_status_text(format!("read failed (exit {})", exit_code));
|
||||
}
|
||||
self.maybe_hide_spinner();
|
||||
return;
|
||||
}
|
||||
|
||||
// Build the completed cell
|
||||
let mut completed_opt = Some(history_cell::new_completed_exec_command(
|
||||
command,
|
||||
@@ -1781,6 +1869,7 @@ impl ChatWidget<'_> {
|
||||
running_commands: HashMap::new(),
|
||||
running_custom_tools: HashMap::new(),
|
||||
running_web_search: HashMap::new(),
|
||||
running_read_agg_index: None,
|
||||
// Use max width to disable wrapping during streaming
|
||||
// Text will be properly wrapped when displayed based on terminal width
|
||||
live_builder: RowBuilder::new(usize::MAX),
|
||||
@@ -1904,6 +1993,7 @@ impl ChatWidget<'_> {
|
||||
running_commands: HashMap::new(),
|
||||
running_custom_tools: HashMap::new(),
|
||||
running_web_search: HashMap::new(),
|
||||
running_read_agg_index: None,
|
||||
live_builder: RowBuilder::new(usize::MAX),
|
||||
pending_images: HashMap::new(),
|
||||
pending_files: HashMap::new(),
|
||||
|
||||
@@ -318,6 +318,91 @@ impl HistoryCell for Box<dyn HistoryCell> {
|
||||
}
|
||||
}
|
||||
|
||||
// ==================== ReadAggregationCell ====================
|
||||
// Aggregates multiple Read preamble entries into a single history cell while
|
||||
// commands are still running, then flips to a finalized state without changing
|
||||
// position in the history. This prevents flicker where multiple transient
|
||||
// "Read" sections appear and later merge.
|
||||
|
||||
pub(crate) struct ReadAggregationCell {
|
||||
// Aggregated preamble lines (each line typically starts with "└ " or two spaces)
|
||||
lines: Vec<Line<'static>>,
|
||||
// When true, render as a finalized "Read" section (ExecStatus::Success);
|
||||
// otherwise render with a running header style (ExecStatus::Running).
|
||||
finalized: bool,
|
||||
}
|
||||
|
||||
impl ReadAggregationCell {
|
||||
pub(crate) fn new() -> Self {
|
||||
Self { lines: Vec::new(), finalized: false }
|
||||
}
|
||||
|
||||
pub(crate) fn push_lines(&mut self, mut more: Vec<Line<'static>>) {
|
||||
// Trim completely empty prefix/suffix lines from the chunk to keep the block compact
|
||||
more = trim_empty_lines(more);
|
||||
if more.is_empty() { return; }
|
||||
self.lines.extend(more);
|
||||
}
|
||||
|
||||
pub(crate) fn finalize(&mut self) {
|
||||
self.finalized = true;
|
||||
}
|
||||
|
||||
// Build a normalized copy of aggregated lines where only the very first
|
||||
// visible line uses the corner connector "└ "; subsequent lines use two
|
||||
// spaces. Also coalesce adjacent read ranges for the same file.
|
||||
fn normalized_lines(&self) -> Vec<Line<'static>> {
|
||||
let mut v = self.lines.clone();
|
||||
// Ensure only the first content line shows the corner
|
||||
let mut seen_first = false;
|
||||
for line in v.iter_mut() {
|
||||
if let Some(sp0) = line.spans.get_mut(0) {
|
||||
let s = sp0.content.as_ref();
|
||||
if s == "└ " || s == " └ " {
|
||||
if seen_first {
|
||||
sp0.content = " ".into();
|
||||
sp0.style = sp0.style.add_modifier(Modifier::DIM);
|
||||
} else {
|
||||
// First occurrence keeps the corner but dim it consistently
|
||||
sp0.style = sp0.style.add_modifier(Modifier::DIM);
|
||||
seen_first = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
// Merge overlapping/touching ranges per file to keep the list succinct
|
||||
coalesce_read_ranges_in_lines_local(&mut v);
|
||||
v
|
||||
}
|
||||
}
|
||||
|
||||
impl HistoryCell for ReadAggregationCell {
|
||||
fn as_any(&self) -> &dyn std::any::Any { self }
|
||||
fn as_any_mut(&mut self) -> &mut dyn std::any::Any { self }
|
||||
fn kind(&self) -> HistoryCellType {
|
||||
HistoryCellType::Exec { kind: ExecKind::Read, status: if self.finalized { ExecStatus::Success } else { ExecStatus::Running } }
|
||||
}
|
||||
fn display_lines(&self) -> Vec<Line<'static>> {
|
||||
let mut out: Vec<Line<'static>> = Vec::new();
|
||||
// Header: match Exec running/success styling for informational actions
|
||||
let header = if self.finalized {
|
||||
Line::styled("Read", Style::default().fg(crate::colors::text()))
|
||||
} else {
|
||||
Line::styled("Read", Style::default().fg(crate::colors::info()))
|
||||
};
|
||||
out.push(header);
|
||||
out.extend(self.normalized_lines());
|
||||
out
|
||||
}
|
||||
fn desired_height(&self, width: u16) -> u16 {
|
||||
Paragraph::new(Text::from(self.display_lines_trimmed()))
|
||||
.wrap(Wrap { trim: false })
|
||||
.line_count(width)
|
||||
.try_into()
|
||||
.unwrap_or(0)
|
||||
}
|
||||
}
|
||||
|
||||
// ==================== PlainHistoryCell ====================
|
||||
// For simple cells that just store lines
|
||||
|
||||
@@ -2649,64 +2734,12 @@ impl CollapsibleReasoningCell {
|
||||
self.in_progress.set(in_progress);
|
||||
}
|
||||
|
||||
/// Normalize reasoning content lines by splitting any line that begins
|
||||
/// with a bold "section title" followed immediately by regular text.
|
||||
/// This produces a separate title line and keeps following text on a new line,
|
||||
/// improving section detection and spacing.
|
||||
fn normalized_lines(&self) -> Vec<Line<'static>> {
|
||||
let mut out: Vec<Line<'static>> = Vec::new();
|
||||
for line in &self.lines {
|
||||
// Skip unchanged if empty or single span
|
||||
if line.spans.len() <= 1 {
|
||||
out.push(line.clone());
|
||||
continue;
|
||||
}
|
||||
|
||||
// Determine length of the leading bold run
|
||||
let mut idx = 0usize;
|
||||
while idx < line.spans.len() {
|
||||
let s = &line.spans[idx];
|
||||
// Treat heading-style titles (often bold) as bold too
|
||||
let is_bold = s.style.add_modifier.contains(Modifier::BOLD);
|
||||
if idx == 0 && s.content.trim().is_empty() {
|
||||
// allow leading spaces in the bold run
|
||||
idx += 1;
|
||||
continue;
|
||||
}
|
||||
if is_bold {
|
||||
idx += 1;
|
||||
continue;
|
||||
}
|
||||
break;
|
||||
}
|
||||
|
||||
// If no leading bold run or the entire line is bold, keep as-is
|
||||
if idx == 0 || idx >= line.spans.len() {
|
||||
out.push(line.clone());
|
||||
continue;
|
||||
}
|
||||
|
||||
// Create a separate title line from the leading bold spans
|
||||
let mut title_spans = Vec::new();
|
||||
let mut rest_spans = Vec::new();
|
||||
for (i, s) in line.spans.iter().enumerate() {
|
||||
if i < idx {
|
||||
title_spans.push(s.clone());
|
||||
} else {
|
||||
rest_spans.push(s.clone());
|
||||
}
|
||||
}
|
||||
|
||||
// Push title line
|
||||
out.push(Line::from(title_spans));
|
||||
// Insert a spacer if the rest is non-empty and not already a blank line
|
||||
let rest_is_blank = rest_spans.iter().all(|s| s.content.trim().is_empty());
|
||||
if !rest_is_blank {
|
||||
out.push(Line::from(rest_spans));
|
||||
}
|
||||
}
|
||||
out
|
||||
}
|
||||
/// Normalized lines for reasoning.
|
||||
///
|
||||
/// Robust mode: do not split lines. We keep provider output verbatim to
|
||||
/// avoid mid‑word truncation and accidental title/body separation. Collapsed
|
||||
/// view still extracts titles using the entire-line-bold rule below.
|
||||
fn normalized_lines(&self) -> Vec<Line<'static>> { self.lines.clone() }
|
||||
|
||||
/// Extracts section titles for collapsed display: any line that appears to
|
||||
/// be a heading or starts with a leading bold run. Returns at least the
|
||||
|
||||
+14
-11
@@ -53,19 +53,22 @@ pub fn init(config: &Config) -> Result<(Tui, TerminalInfo)> {
|
||||
// Initialize syntax highlighting preference from config
|
||||
crate::syntax_highlight::init_highlight_from_config(&config.tui.highlight);
|
||||
|
||||
execute!(stdout(), EnableBracketedPaste)?;
|
||||
// Enable focus change events so we can detect when the terminal window/tab
|
||||
// regains focus and proactively repaint the UI (helps terminals that clear
|
||||
// their alt‑screen buffer while unfocused).
|
||||
let _ = execute!(stdout(), EnableFocusChange);
|
||||
|
||||
// Enter alternate screen mode for full screen TUI
|
||||
execute!(stdout(), crossterm::terminal::EnterAlternateScreen)?;
|
||||
|
||||
// Query terminal capabilities and font size after entering alternate screen
|
||||
// but before enabling raw mode
|
||||
// Query terminal capabilities BEFORE entering the alternate screen.
|
||||
// On Windows Terminal (ConPTY), sending/reading terminal queries from the
|
||||
// alt‑screen can be flaky and cause state corruption. Doing this first
|
||||
// avoids timing issues and ensures we know capabilities up front.
|
||||
let terminal_info = query_terminal_info();
|
||||
|
||||
// Now set up the terminal for full‑screen TUI.
|
||||
execute!(stdout(), crossterm::terminal::EnterAlternateScreen)?;
|
||||
execute!(stdout(), EnableBracketedPaste)?;
|
||||
// Enable focus change notifications where they are known to behave.
|
||||
// Skip on Windows to avoid potential event stream issues under ConPTY.
|
||||
#[cfg(not(windows))]
|
||||
{
|
||||
let _ = execute!(stdout(), EnableFocusChange);
|
||||
}
|
||||
|
||||
enable_raw_mode()?;
|
||||
// Enable keyboard enhancement flags so modifiers for keys like Enter are disambiguated.
|
||||
// chat_composer.rs is using a keyboard event listener to enter for any modified keys
|
||||
|
||||
Reference in New Issue
Block a user