From ef2e54c07fa5a7c43a8618d8d084426a20f5251a Mon Sep 17 00:00:00 2001 From: DDKinger Date: Fri, 29 May 2026 15:46:42 +0800 Subject: [PATCH] feat(wta): split tool boundary from turn boundary so F2 status stops flickering A model turn typically calls many tools sequentially. Under the old wire mapping, every `agent.tool.finished` was routed to `SessionEvent::ToolCompleted` and the reducer demoted Working->Idle; the next `agent.tool.starting` re-promoted to Working. The F2 status badge flickered green<->grey at every tool boundary inside a single turn -- visible in hook-trace.log as ToolStarting/ToolCompleted pairs every few hundred ms. This split makes the wire vocabulary match the actual semantics: * `SessionEvent::ToolCompleted { key }` is now ONLY a single-tool boundary. The reducer clears `current_tool` (for transparency on what's running right now) but does NOT demote Working->Idle. Special case: Attention->Working models `ask_user` (or a permission_prompt) -- the user just answered, the agent will resume the turn. * `SessionEvent::TurnCompleted { key }` (NEW) is the turn-ending event. Demotes Working/Attention->Idle and clears `current_tool` and `attention_reason`. Honors the same Ended/Historical resurrection guard as the other lifecycle events. `Error` is intentionally sticky -- a `ConnectionFailed` from mid-turn should keep surfacing until the next prompt cycle. Wire mapping changes in `route_agent_event_to_registry`: agent.tool.completed agent.tool.finished agent.tool.failed agent.subagent.stop -> ToolCompleted (no demote) agent.stop -> TurnCompleted (the turn boundary) `agent.subagent.stop` moves from the `agent.stop` arm to the `ToolCompleted` arm: Claude's SubagentStop hook fires when a Task-tool sub-agent finishes; the main agent continues the turn. Folding it in with `agent.stop` was part of the original flicker problem. Touched: * agent_sessions.rs -- new `SessionEvent::TurnCompleted`; helper reducer split (ToolCompleted no longer demotes; TurnCompleted does). * session_registry.rs -- new `SessionHookParams::TurnCompleted` (wire serialization with `#[serde(tag = "kind")]`); master reducer split mirrors the helper. * app.rs -- wire mapping updated. * master/mod.rs -- `session_event_key` recognizes TurnCompleted so title-refresh and other key-keyed paths see it. Tests (16 new, all passing; 560 total `cargo test --bin wta`): * agent_sessions: - tool_completed_keeps_working_so_status_does_not_flicker_between_tools - turn_completed_returns_working_to_idle - multi_tool_turn_stays_working_until_turn_completed - tool_completed_demotes_attention_to_working_so_turn_continues - turn_completed_demotes_attention_to_idle_and_clears_reason - turn_completed_does_not_clear_error_state - tool_completed_does_not_clear_error_state - turn_completed_on_terminal_row_is_noop_resurrection_guard - tool_completed_after_ask_user_resumes_working_not_idle * session_registry (master reducer mirror): - master_reducer_multi_tool_turn_stays_working_until_turn_completed - master_reducer_turn_completed_does_not_resurrect_terminal_row - master_reducer_turn_completed_does_not_clear_error - Updated master_reducer_tool_lifecycle_and_notification_update_activity_fields to assert Attention->Working on ToolCompleted, Working->Idle on TurnCompleted. * app (end-to-end via route_agent_event_to_registry): - route_multi_tool_turn_stays_working_until_agent_stop - route_subagent_stop_does_not_end_the_turn - route_ask_user_answer_resumes_working_not_idle Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- tools/wta/src/agent_sessions.rs | 252 ++++++++++++++++++++++++++---- tools/wta/src/app.rs | 183 +++++++++++++++++++++- tools/wta/src/master/mod.rs | 2 + tools/wta/src/session_registry.rs | 113 +++++++++++++- 4 files changed, 520 insertions(+), 30 deletions(-) diff --git a/tools/wta/src/agent_sessions.rs b/tools/wta/src/agent_sessions.rs index 65ebdf842..972b808b5 100644 --- a/tools/wta/src/agent_sessions.rs +++ b/tools/wta/src/agent_sessions.rs @@ -199,7 +199,24 @@ impl AgentSession { pub enum SessionEvent { SessionStarted { key: AgentKey, cli_source: CliSource, pane_session_id: String, cwd: PathBuf, title: String }, ToolStarting { key: AgentKey, tool_name: String }, + /// A single tool invocation completed (`agent.tool.completed`, + /// `agent.tool.finished`, `agent.tool.failed`, or + /// `agent.subagent.stop`). **Does not end the turn** — the + /// reducer keeps `Working`/`Working` as `Working` (so the F2 + /// status badge stays stable across the many tool boundaries in + /// one model turn) and demotes `Attention -> Working` (a + /// user-input tool like `ask_user` just got an answer, so the + /// agent will resume work). The turn-ending demotion to `Idle` + /// is [`SessionEvent::TurnCompleted`]. ToolCompleted { key: AgentKey }, + /// The agent's current turn ended (`agent.stop`). Demotes + /// `Working`/`Attention -> Idle`, clears `current_tool` and + /// `attention_reason`. Honors the same Ended/Historical + /// resurrection guard as the other lifecycle events. `Error` + /// is sticky — `TurnCompleted` does not touch it, so a + /// `ConnectionFailed` from mid-turn keeps surfacing until the + /// next prompt cycle resets it. + TurnCompleted { key: AgentKey }, Notification { key: AgentKey, message: String }, SessionStopped { key: AgentKey, reason: String }, ConnectionFailed { pane_session_id: String, reason: String }, @@ -421,17 +438,60 @@ impl AgentSessionRegistry { SessionEvent::ToolCompleted { key } => { if let Some(entry) = self.sessions.get_mut(&key) { - // Demote to Idle when this completion resolves an active - // wait. Two cases produce Attention: - // 1. A user-input tool started (e.g. Copilot's `ask_user`) - // — its matching ToolCompleted means the user replied. - // 2. A `Notification` event from a permission_prompt hook - // escalated us mid-tool — the matching tool.completed - // / tool.failed (approve / reject) resolves it. - // Both cases mean "next event clears Attention", so we - // demote on any ToolCompleted while in Attention. The Error - // state is separate (set by ConnectionFailed) and is NOT - // touched here, so transient-error UX still works. + // Single-tool boundary. Does NOT demote to Idle on its + // own — a model turn often calls many tools, and + // flipping Working->Idle between each one made the F2 + // status badge flicker. The turn boundary + // ([`SessionEvent::TurnCompleted`], fired from + // `agent.stop`) owns the Working->Idle transition. + // + // Special case: Attention -> Working. Attention is + // reached via [`SessionEvent::Notification`] (e.g. + // Copilot's `ask_user` prompt or a permission_prompt + // hook). The matching ToolCompleted means the user + // answered, so we resume to Working — not Idle — + // because the agent will continue the turn. The + // turn-ending TurnCompleted will demote later. + // + // Working stays Working. Error stays Error (sticky + // until the next prompt cycle). Idle/Ended/Historical + // are untouched. + if entry.status == AgentStatus::Attention { + entry.status = AgentStatus::Working; + entry.attention_reason = None; + } + entry.current_tool = None; + entry.last_activity_at = now; + self.dirty = true; + } + } + + SessionEvent::TurnCompleted { key } => { + if let Some(entry) = self.sessions.get_mut(&key) { + // The model turn just ended (e.g. Claude/Copilot + // `Stop` hook, Gemini `AfterAgent`). Demote any + // turn-active status to Idle and clear the + // turn-scoped scratch fields. + // + // Resurrection guard: a stale `agent.stop` arriving + // after the row already terminated (Ended / + // Historical from `PaneClosed` or pane-handoff) + // must not promote it back into the Idle set — + // that would re-enable Enter / focus on a dead + // pane GUID. + // + // Error is intentionally NOT touched: a + // `ConnectionFailed` mid-turn should stay surfaced + // until the next prompt cycle (handled by the + // ToolStarting -> Working transition) so the user + // can see the failure reason. + let terminal = matches!( + entry.status, + AgentStatus::Ended | AgentStatus::Historical + ); + if terminal { + return; + } let demotable = entry.status == AgentStatus::Working || entry.status == AgentStatus::Attention; if demotable { @@ -1241,7 +1301,14 @@ mod tests { } #[test] - fn tool_completed_returns_working_to_idle() { + fn tool_completed_keeps_working_so_status_does_not_flicker_between_tools() { + // Headline behavior of the turn-scoped status split. A model + // turn typically calls many tools in sequence; under the old + // semantics each `agent.tool.finished` flipped Working->Idle + // and the next `agent.tool.starting` flipped it back, so the + // F2 status badge flickered every couple of hundred ms. + // ToolCompleted now clears only `current_tool`; status stays + // Working until TurnCompleted ends the turn. let mut reg = AgentSessionRegistry::new(); reg.apply(SessionEvent::SessionStarted { key: k("s"), cli_source: CliSource::Claude, @@ -1251,27 +1318,79 @@ mod tests { reg.apply(SessionEvent::ToolStarting { key: k("s"), tool_name: "bash".into() }); reg.apply(SessionEvent::ToolCompleted { key: k("s") }); let s = reg.sessions.get("s").unwrap(); + assert_eq!(s.status, AgentStatus::Working, "tool boundary must not demote"); + assert!(s.current_tool.is_none(), "current_tool cleared between tools"); + } + + #[test] + fn turn_completed_returns_working_to_idle() { + // The turn boundary (mapped from `agent.stop`) IS what demotes + // Working back to Idle. This is the old `tool_completed_returns_ + // working_to_idle` invariant, now scoped to the right event. + let mut reg = AgentSessionRegistry::new(); + reg.apply(SessionEvent::SessionStarted { + key: k("s"), cli_source: CliSource::Claude, + pane_session_id: pane("p"), cwd: PathBuf::from("/x"), + title: "t".into(), + }); + reg.apply(SessionEvent::ToolStarting { key: k("s"), tool_name: "bash".into() }); + reg.apply(SessionEvent::TurnCompleted { key: k("s") }); + let s = reg.sessions.get("s").unwrap(); assert_eq!(s.status, AgentStatus::Idle); assert!(s.current_tool.is_none()); } #[test] - fn tool_completed_demotes_attention_to_idle_but_not_error() { - // Attention is a transient "awaiting user" state set by either a - // user-input tool (e.g. ask_user) or a permission_prompt notification - // arriving mid-tool. The matching ToolCompleted/ToolFailed/Stop event - // means the user has answered (approve, reject, or supplied input), - // so Attention must clear back to Idle. Error, by contrast, is a real - // failure state set by ConnectionFailed and must persist until a new - // SessionStarted/ToolStarting event resets it. + fn multi_tool_turn_stays_working_until_turn_completed() { + // End-to-end at the reducer level: typical 4-tool turn never + // flickers through Idle. let mut reg = AgentSessionRegistry::new(); reg.apply(SessionEvent::SessionStarted { key: k("s"), cli_source: CliSource::Claude, pane_session_id: pane("p"), cwd: PathBuf::from("/x"), title: "t".into(), }); + // agent.prompt.submit + reg.apply(SessionEvent::ToolStarting { key: k("s"), tool_name: "prompt".into() }); + assert_eq!(reg.sessions.get("s").unwrap().status, AgentStatus::Working); + for tool in ["ls", "grep", "edit", "bash"] { + reg.apply(SessionEvent::ToolStarting { + key: k("s"), tool_name: tool.into(), + }); + assert_eq!( + reg.sessions.get("s").unwrap().status, + AgentStatus::Working, + "tool.starting keeps Working ({tool})", + ); + reg.apply(SessionEvent::ToolCompleted { key: k("s") }); + assert_eq!( + reg.sessions.get("s").unwrap().status, + AgentStatus::Working, + "tool.finished must NOT demote to Idle ({tool})", + ); + assert!( + reg.sessions.get("s").unwrap().current_tool.is_none(), + "current_tool cleared between tools", + ); + } + // agent.stop + reg.apply(SessionEvent::TurnCompleted { key: k("s") }); + assert_eq!(reg.sessions.get("s").unwrap().status, AgentStatus::Idle); + } - // Case 1: Attention from a permission_prompt-style notification. + #[test] + fn tool_completed_demotes_attention_to_working_so_turn_continues() { + // Attention is reached via Notification (e.g. `ask_user` or a + // permission_prompt). The matching ToolCompleted means the user + // answered — under the new semantics the agent is about to + // resume the turn, so we go to Working (not Idle). Only + // TurnCompleted demotes to Idle. + let mut reg = AgentSessionRegistry::new(); + reg.apply(SessionEvent::SessionStarted { + key: k("s"), cli_source: CliSource::Claude, + pane_session_id: pane("p"), cwd: PathBuf::from("/x"), + title: "t".into(), + }); reg.apply(SessionEvent::ToolStarting { key: k("s"), tool_name: "shell".into(), }); @@ -1279,20 +1398,91 @@ mod tests { key: k("s"), message: "approve: rm -rf foo".into(), }); assert_eq!(reg.sessions.get("s").unwrap().status, AgentStatus::Attention); - // User rejects → tool.failed → ToolCompleted. Should clear Attention. + reg.apply(SessionEvent::ToolCompleted { key: k("s") }); let s = reg.sessions.get("s").unwrap(); + assert_eq!(s.status, AgentStatus::Working, "attention -> working (turn continues)"); + assert!(s.attention_reason.is_none(), "attention_reason cleared"); + assert!(s.current_tool.is_none(), "current_tool cleared"); + } + + #[test] + fn turn_completed_demotes_attention_to_idle_and_clears_reason() { + // When `agent.stop` arrives while we're in Attention (e.g. the + // turn ended without a matching tool.completed for the + // Notification — possible with some CLIs), TurnCompleted both + // demotes to Idle and clears `attention_reason`. + let mut reg = AgentSessionRegistry::new(); + reg.apply(SessionEvent::SessionStarted { + key: k("s"), cli_source: CliSource::Claude, + pane_session_id: pane("p"), cwd: PathBuf::from("/x"), + title: "t".into(), + }); + reg.apply(SessionEvent::Notification { + key: k("s"), message: "approve: rm -rf foo".into(), + }); + reg.apply(SessionEvent::TurnCompleted { key: k("s") }); + let s = reg.sessions.get("s").unwrap(); assert_eq!(s.status, AgentStatus::Idle); - assert!(s.attention_reason.is_none(), "attention_reason should be cleared"); - assert!(s.current_tool.is_none()); + assert!(s.attention_reason.is_none()); + } - // Case 2: Error must NOT be demoted by ToolCompleted. + #[test] + fn turn_completed_does_not_clear_error_state() { + // Error is sticky — set by ConnectionFailed, must persist past + // the end of the turn so the user can read the failure reason + // until they start a new prompt cycle. + let mut reg = AgentSessionRegistry::new(); + reg.apply(SessionEvent::SessionStarted { + key: k("s"), cli_source: CliSource::Claude, + pane_session_id: pane("p"), cwd: PathBuf::from("/x"), + title: "t".into(), + }); reg.sessions.get_mut("s").unwrap().status = AgentStatus::Error; reg.sessions.get_mut("s").unwrap().last_error = Some("API failed".into()); + reg.apply(SessionEvent::TurnCompleted { key: k("s") }); + let s = reg.sessions.get("s").unwrap(); + assert_eq!(s.status, AgentStatus::Error); + assert_eq!(s.last_error.as_deref(), Some("API failed")); + } + + #[test] + fn tool_completed_does_not_clear_error_state() { + // Same as above for the per-tool boundary — Error must survive + // tool.completed/failed events fired between an error and the + // turn end. + let mut reg = AgentSessionRegistry::new(); + reg.apply(SessionEvent::SessionStarted { + key: k("s"), cli_source: CliSource::Claude, + pane_session_id: pane("p"), cwd: PathBuf::from("/x"), + title: "t".into(), + }); + reg.sessions.get_mut("s").unwrap().status = AgentStatus::Error; reg.apply(SessionEvent::ToolCompleted { key: k("s") }); assert_eq!(reg.sessions.get("s").unwrap().status, AgentStatus::Error); } + #[test] + fn turn_completed_on_terminal_row_is_noop_resurrection_guard() { + // Stale `agent.stop` arriving after the row already ended + // (PaneClosed / pane handoff) must not resurrect it into the + // Idle bucket — that would re-enable Enter / focus on a dead + // pane GUID. + let mut reg = AgentSessionRegistry::new(); + reg.apply(SessionEvent::SessionStarted { + key: k("s"), cli_source: CliSource::Claude, + pane_session_id: pane("p"), cwd: PathBuf::from("/x"), + title: "t".into(), + }); + reg.sessions.get_mut("s").unwrap().status = AgentStatus::Ended; + reg.apply(SessionEvent::TurnCompleted { key: k("s") }); + assert_eq!(reg.sessions.get("s").unwrap().status, AgentStatus::Ended); + + reg.sessions.get_mut("s").unwrap().status = AgentStatus::Historical; + reg.apply(SessionEvent::TurnCompleted { key: k("s") }); + assert_eq!(reg.sessions.get("s").unwrap().status, AgentStatus::Historical); + } + #[test] fn notification_sets_attention_with_reason() { let mut reg = AgentSessionRegistry::new(); @@ -1974,11 +2164,13 @@ mod tests { } #[test] - fn tool_completed_demotes_attention_when_current_tool_was_user_input() { + fn tool_completed_after_ask_user_resumes_working_not_idle() { // Models the Copilot ask_user flow: BeforeTool fires with // tool_name="ask_user" → registry sees it as Attention. When the // user answers, AfterTool fires → ToolCompleted should demote - // Attention back to Idle (so the row stops nagging). + // Attention to **Working** (not Idle): the agent is about to + // resume the turn with the user's answer. Only TurnCompleted + // demotes to Idle. let mut reg = AgentSessionRegistry::new(); reg.apply(SessionEvent::SessionStarted { key: k("s"), cli_source: CliSource::Copilot, @@ -1999,9 +2191,13 @@ mod tests { // User answers → AfterTool → ToolCompleted. reg.apply(SessionEvent::ToolCompleted { key: k("s") }); let s = reg.sessions.get("s").unwrap(); - assert_eq!(s.status, AgentStatus::Idle); + assert_eq!(s.status, AgentStatus::Working, "agent resumes the turn"); assert!(s.current_tool.is_none()); assert!(s.attention_reason.is_none()); + + // ... and a subsequent agent.stop ends the turn. + reg.apply(SessionEvent::TurnCompleted { key: k("s") }); + assert_eq!(reg.sessions.get("s").unwrap().status, AgentStatus::Idle); } #[test] diff --git a/tools/wta/src/app.rs b/tools/wta/src/app.rs index 8935e721d..141b66c3a 100644 --- a/tools/wta/src/app.rs +++ b/tools/wta/src/app.rs @@ -595,8 +595,16 @@ where "agent.tool.completed" | "agent.tool.finished" | "agent.tool.failed" - | "agent.stop" | "agent.subagent.stop" => SessionEvent::ToolCompleted { key }, + // Turn boundary. `agent.stop` is the only event that demotes + // Working/Attention -> Idle; the tool-boundary events above + // intentionally do not, so the F2 status badge stays stable + // across the many tool calls in a single turn. Sub-agent + // (Claude `SubagentStop`) finishing is treated as a tool + // completion because the main agent continues — making it + // demote here would re-introduce the very flicker this split + // is removing. + "agent.stop" => SessionEvent::TurnCompleted { key }, "agent.notification" => SessionEvent::Notification { key, message: payload @@ -9205,6 +9213,179 @@ mod tests { assert!(count >= 1, "at least one real-keyed event must reach master"); } + /// Headline UX guarantee of the turn-scoped status split: a model + /// turn calling many tools must stay in `Working` throughout, so + /// the F2 status badge doesn't flicker green↔grey at every tool + /// boundary. Drives the real `route_agent_event_to_registry` + /// pipeline with the hook-event vocabulary documented in + /// `tools/wta/wt-agent-hooks/README.md`. + #[test] + fn route_multi_tool_turn_stays_working_until_agent_stop() { + use crate::agent_sessions::AgentStatus; + let mut reg = crate::agent_sessions::AgentSessionRegistry::new(); + let pane = "pane-turn-1"; + let sid = "sid-turn-1"; + + // Seed the row so the routing layer has a real session to + // attach to (otherwise the orphan/synthetic-key path would + // run instead). + reg.apply(crate::agent_sessions::SessionEvent::SessionStarted { + key: sid.into(), + cli_source: crate::agent_sessions::CliSource::Copilot, + pane_session_id: pane.into(), + cwd: std::path::PathBuf::from("/x"), + title: "t".into(), + }); + + let route = |reg: &mut crate::agent_sessions::AgentSessionRegistry, + event: &str, + extra: serde_json::Value| { + let mut payload = json!({}); + if let Some(obj) = extra.as_object() { + for (k, v) in obj { payload[k] = v.clone(); } + } + let params = json!({ + "event": event, + "cli_source": "copilot", + "agent_session_id": sid, + "payload": payload, + }); + crate::app::route_agent_event_to_registry(reg, pane, ¶ms); + }; + + // agent.prompt.submit → Working + route(&mut reg, "agent.prompt.submit", json!({})); + assert_eq!(reg.get(&sid.to_string()).unwrap().status, AgentStatus::Working); + + // 4 tool round-trips. Status MUST stay Working throughout — + // any flip to Idle here would mean the badge flickered. + for tool in ["ls", "grep", "edit", "bash"] { + route(&mut reg, "agent.tool.starting", json!({"tool_name": tool})); + assert_eq!( + reg.get(&sid.to_string()).unwrap().status, + AgentStatus::Working, + "agent.tool.starting must keep Working ({tool})", + ); + route(&mut reg, "agent.tool.finished", json!({"tool_name": tool})); + assert_eq!( + reg.get(&sid.to_string()).unwrap().status, + AgentStatus::Working, + "agent.tool.finished must NOT demote to Idle ({tool})", + ); + } + + // Only agent.stop ends the turn. + route(&mut reg, "agent.stop", json!({})); + assert_eq!(reg.get(&sid.to_string()).unwrap().status, AgentStatus::Idle); + } + + /// `agent.subagent.stop` (Claude Code's `SubagentStop` hook) fires + /// when a Task-tool sub-agent finishes; the main agent continues + /// the turn. The old routing collapsed it into the `agent.stop` + /// → Idle path and the next `tool.starting` had to re-promote + /// Working — exactly the flicker the split is removing. The new + /// mapping routes `subagent.stop` to `ToolCompleted`, which is a + /// no-op on Working. + #[test] + fn route_subagent_stop_does_not_end_the_turn() { + use crate::agent_sessions::AgentStatus; + let mut reg = crate::agent_sessions::AgentSessionRegistry::new(); + let pane = "pane-sub"; + let sid = "sid-sub"; + reg.apply(crate::agent_sessions::SessionEvent::SessionStarted { + key: sid.into(), + cli_source: crate::agent_sessions::CliSource::Claude, + pane_session_id: pane.into(), + cwd: std::path::PathBuf::from("/x"), + title: "t".into(), + }); + + let route = |reg: &mut _, event: &str, extra: serde_json::Value| { + let mut payload = json!({}); + if let Some(obj) = extra.as_object() { + for (k, v) in obj { payload[k] = v.clone(); } + } + let params = json!({ + "event": event, + "cli_source": "claude", + "agent_session_id": sid, + "payload": payload, + }); + crate::app::route_agent_event_to_registry(reg, pane, ¶ms); + }; + + route(&mut reg, "agent.prompt.submit", json!({})); + route(&mut reg, "agent.tool.starting", json!({"tool_name": "Task"})); + route(&mut reg, "agent.subagent.stop", json!({})); + assert_eq!( + reg.get(&sid.to_string()).unwrap().status, + AgentStatus::Working, + "subagent.stop must not demote — main agent continues the turn", + ); + + // Now the actual turn end. + route(&mut reg, "agent.stop", json!({})); + assert_eq!(reg.get(&sid.to_string()).unwrap().status, AgentStatus::Idle); + } + + /// `ask_user` round-trip via the routing layer: Notification flips + /// to Attention, the matching `agent.tool.finished` (user answered) + /// resumes Working — not Idle. Only the subsequent `agent.stop` + /// demotes to Idle. + #[test] + fn route_ask_user_answer_resumes_working_not_idle() { + use crate::agent_sessions::AgentStatus; + let mut reg = crate::agent_sessions::AgentSessionRegistry::new(); + let pane = "pane-ask"; + let sid = "sid-ask"; + reg.apply(crate::agent_sessions::SessionEvent::SessionStarted { + key: sid.into(), + cli_source: crate::agent_sessions::CliSource::Copilot, + pane_session_id: pane.into(), + cwd: std::path::PathBuf::from("/x"), + title: "t".into(), + }); + + let route = |reg: &mut _, event: &str, extra: serde_json::Value| { + let mut payload = json!({}); + if let Some(obj) = extra.as_object() { + for (k, v) in obj { payload[k] = v.clone(); } + } + let params = json!({ + "event": event, + "cli_source": "copilot", + "agent_session_id": sid, + "payload": payload, + }); + crate::app::route_agent_event_to_registry(reg, pane, ¶ms); + }; + + route(&mut reg, "agent.prompt.submit", json!({})); + // is_user_input_tool("ask_user") => true, so the route layer + // synthesises a Notification on top of the ToolStarting. + route( + &mut reg, + "agent.tool.starting", + json!({ + "tool_name": "ask_user", + "tool_input": { "question": "Which folder?" }, + }), + ); + assert_eq!( + reg.get(&sid.to_string()).unwrap().status, + AgentStatus::Attention, + ); + + // User answers → agent.tool.finished → resume Working. + route(&mut reg, "agent.tool.finished", json!({"tool_name": "ask_user"})); + let s = reg.get(&sid.to_string()).unwrap(); + assert_eq!(s.status, AgentStatus::Working); + assert!(s.attention_reason.is_none()); + + route(&mut reg, "agent.stop", json!({})); + assert_eq!(reg.get(&sid.to_string()).unwrap().status, AgentStatus::Idle); + } + fn test_app_with_master_rx() -> ( App, tokio::sync::mpsc::UnboundedReceiver, diff --git a/tools/wta/src/master/mod.rs b/tools/wta/src/master/mod.rs index 25bdaedd4..7bfbc5ee1 100644 --- a/tools/wta/src/master/mod.rs +++ b/tools/wta/src/master/mod.rs @@ -1892,6 +1892,7 @@ fn session_event_key(event: &crate::agent_sessions::SessionEvent) -> Option<&str SessionEvent::SessionStarted { key, .. } | SessionEvent::ToolStarting { key, .. } | SessionEvent::ToolCompleted { key } + | SessionEvent::TurnCompleted { key } | SessionEvent::Notification { key, .. } | SessionEvent::SessionStopped { key, .. } | SessionEvent::ResumeDispatched { key } @@ -3356,6 +3357,7 @@ mod tests { Some("k2"), ), (SessionEvent::ToolCompleted { key: "k3".into() }, Some("k3")), + (SessionEvent::TurnCompleted { key: "k3b".into() }, Some("k3b")), ( SessionEvent::Notification { key: "k4".into(), diff --git a/tools/wta/src/session_registry.rs b/tools/wta/src/session_registry.rs index 1ceedb783..cc9436ce3 100644 --- a/tools/wta/src/session_registry.rs +++ b/tools/wta/src/session_registry.rs @@ -481,6 +481,11 @@ pub enum SessionHookParams { ToolCompleted { key: crate::agent_sessions::AgentKey, }, + /// Turn boundary — see [`crate::agent_sessions::SessionEvent::TurnCompleted`]. + /// Wire serialization: `{"kind":"TurnCompleted","key":"…"}`. + TurnCompleted { + key: crate::agent_sessions::AgentKey, + }, Notification { key: crate::agent_sessions::AgentKey, message: String, @@ -527,6 +532,7 @@ impl From<&crate::agent_sessions::SessionEvent> for SessionHookParams { tool_name: tool_name.clone(), }, SessionEvent::ToolCompleted { key } => Self::ToolCompleted { key: key.clone() }, + SessionEvent::TurnCompleted { key } => Self::TurnCompleted { key: key.clone() }, SessionEvent::Notification { key, message } => Self::Notification { key: key.clone(), message: message.clone(), @@ -577,6 +583,7 @@ impl From for crate::agent_sessions::SessionEvent { Self::ToolStarting { key, tool_name } } SessionHookParams::ToolCompleted { key } => Self::ToolCompleted { key }, + SessionHookParams::TurnCompleted { key } => Self::TurnCompleted { key }, SessionHookParams::Notification { key, message } => { Self::Notification { key, message } } @@ -1105,6 +1112,33 @@ fn apply_event_locked(state: &mut RegistryState, ev: SessionEvent) -> bool { if matches!(entry.status, Some(AgentStatus::Ended | AgentStatus::Historical)) { return false; } + // Single-tool boundary: do NOT demote Working -> Idle (the + // turn likely continues with more tools). The + // Working -> Idle transition happens on TurnCompleted + // (`agent.stop`). Attention -> Working models "user just + // answered an `ask_user`, agent resumes the turn." See + // helper-side reducer in `agent_sessions.rs` for the full + // rationale. + if matches!(entry.status, Some(AgentStatus::Attention)) { + entry.status = Some(AgentStatus::Working); + entry.attention_reason = None; + } + entry.current_tool = None; + entry.last_activity_at_ms = Some(now); + true + } + SessionEvent::TurnCompleted { key } => { + let sid = acp::SessionId::new(key); + let Some(entry) = state.sessions.get_mut(&sid) else { return false; }; + // Resurrection guard mirrors ToolStarting / ToolCompleted. + if matches!(entry.status, Some(AgentStatus::Ended | AgentStatus::Historical)) { + return false; + } + // The model turn ended (`agent.stop`). Demote + // Working/Attention -> Idle and clear turn-scoped scratch + // fields. `Error` is intentionally sticky so a + // `ConnectionFailed` from earlier in the turn stays + // surfaced until the next prompt cycle. if matches!(entry.status, Some(AgentStatus::Working | AgentStatus::Attention)) { entry.status = Some(AgentStatus::Idle); entry.attention_reason = None; @@ -1851,11 +1885,88 @@ mod tests { assert_eq!(row.status, Some(crate::agent_sessions::AgentStatus::Attention)); assert_eq!(row.attention_reason.as_deref(), Some("approve?")); + // Under the turn-scoped status split, ToolCompleted on Attention + // resumes Working (user just answered, agent continues the turn) + // — it does NOT demote to Idle. The Idle demotion comes from + // TurnCompleted (`agent.stop`) below. reg.apply_event(crate::agent_sessions::SessionEvent::ToolCompleted { key: "sid".into() }).await; let row = reg.lookup(&acp::SessionId::new("sid")).await.unwrap(); - assert_eq!(row.status, Some(crate::agent_sessions::AgentStatus::Idle)); + assert_eq!(row.status, Some(crate::agent_sessions::AgentStatus::Working)); assert!(row.current_tool.is_none()); assert!(row.attention_reason.is_none()); + + // TurnCompleted is what ends the turn → Idle. + reg.apply_event(crate::agent_sessions::SessionEvent::TurnCompleted { key: "sid".into() }).await; + let row = reg.lookup(&acp::SessionId::new("sid")).await.unwrap(); + assert_eq!(row.status, Some(crate::agent_sessions::AgentStatus::Idle)); + assert!(row.current_tool.is_none()); + } + + /// Master-side mirror of `agent_sessions::tests:: + /// multi_tool_turn_stays_working_until_turn_completed` — same + /// guarantee at the master reducer. + #[tokio::test] + async fn master_reducer_multi_tool_turn_stays_working_until_turn_completed() { + use crate::agent_sessions::{AgentStatus, CliSource, SessionEvent}; + let reg = InMemoryRegistry::new(); + reg.apply_event(SessionEvent::SessionStarted { + key: "sid".into(), + cli_source: CliSource::Copilot, + pane_session_id: "p".into(), + cwd: PathBuf::from("C:\\x"), + title: "t".into(), + }).await; + // agent.prompt.submit + reg.apply_event(SessionEvent::ToolStarting { + key: "sid".into(), tool_name: "prompt".into(), + }).await; + for tool in ["ls", "grep", "edit", "bash"] { + reg.apply_event(SessionEvent::ToolStarting { + key: "sid".into(), tool_name: tool.into(), + }).await; + reg.apply_event(SessionEvent::ToolCompleted { key: "sid".into() }).await; + let row = reg.lookup(&acp::SessionId::new("sid")).await.unwrap(); + assert_eq!( + row.status, + Some(AgentStatus::Working), + "between-tool boundary must keep Working ({tool})", + ); + } + reg.apply_event(SessionEvent::TurnCompleted { key: "sid".into() }).await; + let row = reg.lookup(&acp::SessionId::new("sid")).await.unwrap(); + assert_eq!(row.status, Some(AgentStatus::Idle)); + } + + #[tokio::test] + async fn master_reducer_turn_completed_does_not_resurrect_terminal_row() { + use crate::agent_sessions::{AgentStatus, SessionEvent}; + let reg = InMemoryRegistry::new(); + let mut info = SessionInfo::new(acp::SessionId::new("ended-sid"), PathBuf::from("/repo")); + info.status = Some(AgentStatus::Ended); + reg.upsert(info).await; + let applied = reg + .apply_event(SessionEvent::TurnCompleted { key: "ended-sid".into() }) + .await; + assert!(!applied, "TurnCompleted on Ended must be a no-op"); + let row = reg.lookup(&acp::SessionId::new("ended-sid")).await.unwrap(); + assert_eq!(row.status, Some(AgentStatus::Ended)); + } + + #[tokio::test] + async fn master_reducer_turn_completed_does_not_clear_error() { + use crate::agent_sessions::{AgentStatus, SessionEvent}; + let reg = InMemoryRegistry::new(); + let mut info = SessionInfo::new(acp::SessionId::new("err-sid"), PathBuf::from("/repo")); + info.status = Some(AgentStatus::Error); + info.last_error = Some("pipe closed".into()); + reg.upsert(info).await; + let applied = reg + .apply_event(SessionEvent::TurnCompleted { key: "err-sid".into() }) + .await; + assert!(applied, "non-terminal Error row still receives the event"); + let row = reg.lookup(&acp::SessionId::new("err-sid")).await.unwrap(); + assert_eq!(row.status, Some(AgentStatus::Error), "Error is sticky"); + assert_eq!(row.last_error.as_deref(), Some("pipe closed")); } #[tokio::test]