From 27738112feec8eccc9a40d3b89eff26480f91dc6 Mon Sep 17 00:00:00 2001 From: Caner Akca Date: Thu, 3 Sep 2026 14:57:47 +0300 Subject: [PATCH] fix: keep an agent name through a process-exit observation A process-exit observation is not proof the agent is gone: the same observation can be wrong while the agent keeps running, and the name is the only handle its owner has on the pane. Detection uncertainty already preserves the name, so releasing it on the observation alone contradicts that contract and leaves a live agent unreachable by name with a rename as the only recovery. Free the name at the point the agent actually leaves the pane - a recorded exit with no agent detected any more - so a wrong observation costs nothing and a real exit still releases the name for reuse. refs #3225 --- src/app/api.rs | 12 +++++- src/app/api/agents.rs | 62 ++++++++++++++++++++++++++++ src/terminal/state.rs | 95 ++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 166 insertions(+), 3 deletions(-) diff --git a/src/app/api.rs b/src/app/api.rs index 9bd0248100..30fc3be2ed 100644 --- a/src/app/api.rs +++ b/src/app/api.rs @@ -2067,7 +2067,13 @@ mod tests { observed_at: std::time::Instant::now(), }); - assert!(app.state.terminals[&terminal_id].agent_name.is_none()); + // The release event is this test's subject; the name outliving the + // observation is pinned by + // `a_process_exit_observation_alone_does_not_free_the_name`. + assert_eq!( + app.state.terminals[&terminal_id].agent_name.as_deref(), + agent_name + ); assert!(event_hub.events_after(0).iter().any(|(_, event)| matches!( &event.data, crate::api::schema::EventData::PaneAgentDetected { @@ -2123,7 +2129,9 @@ mod tests { let terminal = &app.state.terminals[&terminal_id]; assert_eq!(terminal.state, AgentState::Idle); - assert!(terminal.agent_name.is_none()); + // Releasing the registration does not free the name yet; a wrong + // observation must not cost a live agent the handle its owner gave it. + assert_eq!(terminal.agent_name.as_deref(), Some("reviewer")); assert!(event_hub.events_after(0).iter().any(|(_, event)| matches!( event.data, crate::api::schema::EventData::PaneAgentDetected { released: true, .. } diff --git a/src/app/api/agents.rs b/src/app/api/agents.rs index 1c0a5369b1..9289ae02f4 100644 --- a/src/app/api/agents.rs +++ b/src/app/api/agents.rs @@ -472,6 +472,68 @@ mod tests { assert_eq!(rx.try_recv().unwrap(), Bytes::from_static(b"\r")); } + #[tokio::test] + async fn a_false_process_exit_makes_a_named_live_agent_unreachable_by_name() { + // Reproduces the registration loss reported on #3225 by rszrszrsz: + // a live agent pane with an assigned name stops resolving by that name + // while its process keeps running, and renaming is the only recovery. + let mut app = app_with_agent(); + let pane_id = app.state.workspaces[0].tabs[0].root_pane; + let terminal_id = app.state.workspaces[0].tabs[0].panes[&pane_id] + .attached_terminal_id + .clone(); + let observed_at = std::time::Instant::now(); + let terminal = app.state.terminals.get_mut(&terminal_id).unwrap(); + terminal.set_detected_state(Some(Agent::Pi), AgentState::Working); + terminal.set_agent_name("reviewer".into()); + + let found = app.handle_agent_get( + "req:before".into(), + AgentTarget { + target: "reviewer".into(), + }, + ); + assert!( + serde_json::from_str::(&found).is_ok(), + "the assigned name must resolve while the agent is running: {found}" + ); + + // One process-exit observation, then the same agent is observed alive + // again on the next probe - the process never actually went away. + app.handle_internal_event(crate::events::AppEvent::StateChanged { + pane_id, + agent: Some(Agent::Pi), + state: AgentState::Idle, + visible_blocker: false, + visible_working: false, + process_exited: true, + observed_at, + }); + app.handle_internal_event(crate::events::AppEvent::AgentProcessDetected { + pane_id, + agent: Agent::Pi, + observed_at: observed_at + std::time::Duration::from_secs(1), + }); + + let terminal = &app.state.terminals[&terminal_id]; + assert_eq!( + terminal.detected_agent, + Some(Agent::Pi), + "the agent process is still there" + ); + + let after = app.handle_agent_get( + "req:after".into(), + AgentTarget { + target: "reviewer".into(), + }, + ); + assert!( + serde_json::from_str::(&after).is_ok(), + "a live agent must stay reachable by its assigned name: {after}" + ); + } + #[tokio::test] async fn agent_prompt_sends_text_then_delays_enter() { let mut app = app_with_agent(); diff --git a/src/terminal/state.rs b/src/terminal/state.rs index a7c8457481..7ceaede7a4 100644 --- a/src/terminal/state.rs +++ b/src/terminal/state.rs @@ -574,7 +574,12 @@ impl TerminalState { self.hook_authority = None; self.persisted_agent_session = durable_session; } - if agent_released { + // Observing a process exit is not the same as the agent being gone: the + // observation can be wrong while the agent keeps running, and the name + // is the only handle its owner has on the pane. Detection uncertainty + // already keeps the name, so free it at the point the agent actually + // leaves the pane - a recorded exit with no agent detected any more. + if agent.is_none() && self.recent_agent_process_exit.is_some() { self.clear_agent_name(); } TerminalStateMutation { @@ -5548,6 +5553,94 @@ mod tests { assert!(terminal.hook_authority.is_none()); } + #[test] + fn a_process_exit_observation_alone_does_not_free_the_name() { + // The pane keeps reporting the same agent throughout: this models a + // process-exit observation that is wrong (the agent is still running), + // which is what `agent_alias_survives_detection_uncertainty_...` + // already refuses to let destroy the name through the detection path. + let now = Instant::now(); + let mut terminal = test_terminal(); + terminal.set_detected_state(Some(Agent::Pi), AgentState::Working); + terminal.set_agent_name("reviewer".into()); + + // Contract today: losing detection does not cost the pane its name. + terminal.set_detected_state(None, AgentState::Unknown); + assert_eq!( + terminal.agent_name.as_deref(), + Some("reviewer"), + "detection uncertainty must not release the name" + ); + terminal.set_detected_state(Some(Agent::Pi), AgentState::Working); + + // A single process-exit observation for that same agent destroys it. + let exit = terminal.set_detected_state_with_screen_signals_at( + Some(Agent::Pi), + AgentState::Idle, + false, + false, + false, + true, + now, + ); + assert!(exit.agent_released); + assert_eq!( + terminal.agent_name.as_deref(), + Some("reviewer"), + "a process-exit observation for the still-detected agent must not \ + destroy a name that detection uncertainty is allowed to keep" + ); + + // And the agent proving it is alive again must not leave the pane + // permanently unreachable by the name its owner assigned. + terminal.set_detected_agent_process_at(Agent::Pi, now + Duration::from_secs(1)); + assert_eq!( + terminal.agent_name.as_deref(), + Some("reviewer"), + "the name must still resolve once the agent is observed alive again" + ); + assert_eq!(terminal.detected_agent, Some(Agent::Pi)); + } + + #[test] + fn a_confirmed_agent_exit_still_frees_the_name_for_reuse() { + // The other side of `a_process_exit_observation_alone_does_not_free_the_name`: + // once the agent is actually gone from the pane the name must be + // released, so `agent start` can reuse it. + let now = Instant::now(); + let mut terminal = test_terminal(); + terminal.set_detected_state(Some(Agent::Pi), AgentState::Working); + terminal.set_agent_name("reviewer".into()); + + terminal.set_detected_state_with_screen_signals_at( + Some(Agent::Pi), + AgentState::Idle, + false, + false, + false, + true, + now, + ); + assert_eq!(terminal.agent_name.as_deref(), Some("reviewer")); + + // The agent really is gone: the next observation finds no agent while + // the recorded exit still stands. + terminal.set_detected_state_with_screen_signals_at( + None, + AgentState::Unknown, + false, + false, + false, + false, + now + Duration::from_secs(1), + ); + assert_eq!( + terminal.agent_name, None, + "a confirmed exit must release the name" + ); + assert!(!terminal.is_agent_terminal()); + } + #[test] fn agent_alias_survives_detection_uncertainty_and_reported_release_but_not_replacement() { let mut terminal = test_terminal();