diff options
Diffstat (limited to 'src')
| -rw-r--r-- | src/app/api.rs | 12 | ||||
| -rw-r--r-- | src/app/api/agents.rs | 62 | ||||
| -rw-r--r-- | 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 9bd02481..30fc3be2 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 1c0a5369..9289ae02 100644 --- a/src/app/api/agents.rs +++ b/src/app/api/agents.rs @@ -473,6 +473,68 @@ mod tests { } #[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::<SuccessResponse>(&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::<SuccessResponse>(&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(); let pane_id = app.state.workspaces[0].tabs[0].root_pane; diff --git a/src/terminal/state.rs b/src/terminal/state.rs index a7c84574..7ceaede7 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 { @@ -5549,6 +5554,94 @@ mod tests { } #[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(); terminal.set_detected_state(Some(Agent::Pi), AgentState::Working); |