Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions src/app/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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, .. }
Expand Down
62 changes: 62 additions & 0 deletions src/app/api/agents.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<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();
Expand Down
95 changes: 94 additions & 1 deletion src/terminal/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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();
Expand Down
Loading