Skip to content

Commit 7403792

Browse files
author
Yogthos
committed
fix(ui): thread question_tx/plan_tx through 3 missed sites + passive close on tool-turnover
Two related UI/threading bugs surfaced during code review of 9475586. ## 1. Three missed build_agent sites in ui/mod.rs (companion to 9475586) Last commit fixed the 9 build_agent calls in slash.rs but missed three in ui/mod.rs that also passed `None, None` for question_tx + plan_tx: - L1770: `/cd` worktree-return to main repo - L2507: `/model` switch rebuild - L2764: auto worktree-cleanup return Same C8-LSP shape: after any of these paths rebuilt the agent the `question` + `plan_enter` + `plan_exit` tools silently disappeared from the LLM's tool list for the rest of the session, because `builder.rs:621` / `agent/builder.rs:252` install those tools only when their channels are `Some`. The threading is now mechanical across all 13 call sites (3 in main.rs, 9 in slash.rs, 4 in ui/mod.rs — all `&Option<T>` refs auto-clone to `Option<T>` via method resolution). ## 2. Chamber-abort false-positive on fresh ToolCall `close_tool_chamber_if_open` aliases `close_tool_chamber_abort` for back-compat (658a63e), which paints "⚠ tool denied · aborted · no result" inside any chamber where `last_tool_name.is_some() || tool_chamber_open` at the time of close. The call at line 2123 fires when a NEW ToolCall arrives over a chamber whose flags haven't flipped yet — under parallel tool execution the next `ToolExecutionStart` lands before the prior `ToolExecutionEnd` processes, so EVERY in-flight chamber gets falsely branded as "denied" while the tool is actually executing normally. Switched 2123 to `close_tool_chamber_passive` — a fresh ToolCall is chamber turnover, not a denial event. The other three abort callers (`AgentEvent::Error` / `Interjected` / `ContextOverflow`) genuinely are denial-shaped and stay on the abort variant. This unmasks a separate underlying bug: the UI's single-chamber state model (`tool_chamber_open: bool`, `last_tool_name: Option`) can't represent multiple in-flight tool calls. Empty Read chambers + out-of-place `↳ trailers` for tools whose results arrive after a newer call's chamber opens are the new visible symptom. A per-id chamber state rework is filed separately. Tests: existing `close_tool_chamber_*` unit tests still pass (passive variant has equivalent gating behavior, just different inner-row content).
1 parent 9475586 commit 7403792

1 file changed

Lines changed: 21 additions & 7 deletions

File tree

src/ui/mod.rs

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1774,8 +1774,8 @@ pub async fn run_interactive(
17741774
context,
17751775
permission.clone(),
17761776
ask_tx.clone(),
1777-
None,
1778-
None,
1777+
question_tx.clone(),
1778+
plan_tx.clone(),
17791779
bg_store.clone(),
17801780
#[cfg(feature = "lsp")]
17811781
lsp_manager.clone(),
@@ -2120,7 +2120,21 @@ pub async fn run_interactive(
21202120
// it before opening the new one. Without this
21212121
// the new `╭─ NAME ─ args` lands inside the
21222122
// stale chamber.
2123-
close_tool_chamber_if_open(&mut renderer, &mut last_tool_name, &mut tool_chamber_open)?;
2123+
//
2124+
// Use PASSIVE close, not abort. A new ToolCall
2125+
// arriving over a stale chamber is chamber
2126+
// turnover, not a denial event — the prior
2127+
// tool may have finished cleanly and just
2128+
// not flipped the flags yet (race, or a code
2129+
// path that emitted ToolCall before the
2130+
// previous ToolResult landed). Painting
2131+
// "⚠ tool denied · aborted · no result" on it
2132+
// would falsely brand a healthy tool call as
2133+
// refused. The other three abort callers
2134+
// (Error / Interjected / ContextOverflow) are
2135+
// genuine denial-shaped events and stay on
2136+
// `close_tool_chamber_if_open`.
2137+
close_tool_chamber_passive(&mut renderer, &mut last_tool_name, &mut tool_chamber_open)?;
21242138
last_tool_name = Some(name.to_string());
21252139
if agent_line_started {
21262140
renderer.write_line("", Color::White)?;
@@ -2511,8 +2525,8 @@ pub async fn run_interactive(
25112525
context,
25122526
permission.clone(),
25132527
ask_tx.clone(),
2514-
None,
2515-
None,
2528+
question_tx.clone(),
2529+
plan_tx.clone(),
25162530
bg_store.clone(),
25172531
#[cfg(feature = "lsp")]
25182532
lsp_manager.clone(),
@@ -2768,8 +2782,8 @@ pub async fn run_interactive(
27682782
context,
27692783
permission.clone(),
27702784
ask_tx.clone(),
2771-
None,
2772-
None,
2785+
question_tx.clone(),
2786+
plan_tx.clone(),
27732787
bg_store.clone(),
27742788
#[cfg(feature = "lsp")]
27752789
lsp_manager.clone(),

0 commit comments

Comments
 (0)