mirror of
https://github.com/l0ng-ai/tty7.git
synced 2026-09-21 16:02:20 +00:00
fix(agents): don't re-badge a turn the reader already saw when a pane is rebuilt
Switching workspaces or reopening a window from the tray throws a pane's TerminalView away and builds a new one over the same daemon pane. The new view starts with no last status, so an agent that was already Done arrives as None -> Done, which poll_agent_status cannot tell from a turn finishing live, and every unfocused rebuilt pane got its unread badge back. The daemon now counts finished turns per agent session (turns, bumped on entering Done), and views leave an app-lifetime mark per (host, pane) of the session, turn count and badge the reader was last shown. A rebuilt view's first sight of Done takes the badge back from a matching mark instead of raising a new one; a turn that finished while the view was gone has no mark or a lower count, and still badges. Fixes #870 Claude-Session: https://claude.ai/code/session_01JRqYZ9E153WpSHGS2AW3BM
This commit is contained in:
@@ -714,6 +714,12 @@ pub struct AgentSessionState {
|
||||
pub cwd: Option<std::path::PathBuf>,
|
||||
#[serde(default)]
|
||||
pub activity: u64,
|
||||
/// How many turns this session has finished: bumped each time it settles
|
||||
/// into `Done`. The status alone cannot tell a client that attaches to a
|
||||
/// `Done` pane whether that is the turn it already showed the reader or a
|
||||
/// later one that finished while nobody was watching (#870).
|
||||
#[serde(default)]
|
||||
pub turns: u64,
|
||||
}
|
||||
|
||||
impl AgentStatus {
|
||||
@@ -767,6 +773,9 @@ impl AgentSessionState {
|
||||
}
|
||||
}
|
||||
AgentEventKind::Stop => {
|
||||
if self.status != AgentStatus::Done {
|
||||
self.turns = self.turns.wrapping_add(1);
|
||||
}
|
||||
self.status = AgentStatus::Done;
|
||||
self.message = ev.message.clone();
|
||||
}
|
||||
@@ -1273,6 +1282,36 @@ mod tests {
|
||||
assert_eq!(s.activity, 4);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn each_finished_turn_is_counted_once() {
|
||||
let ev = |kind| AgentEvent {
|
||||
agent: Some(CLIAgent::Claude),
|
||||
kind,
|
||||
session_id: None,
|
||||
message: None,
|
||||
cwd: None,
|
||||
prompt: None,
|
||||
};
|
||||
|
||||
let mut s = AgentSessionState::default();
|
||||
s.apply_event(&ev(AgentEventKind::PromptSubmit));
|
||||
assert_eq!(s.turns, 0, "a turn starting has not finished anything");
|
||||
|
||||
s.apply_event(&ev(AgentEventKind::Stop));
|
||||
assert_eq!(s.turns, 1);
|
||||
s.apply_event(&ev(AgentEventKind::Stop));
|
||||
assert_eq!(s.turns, 1, "a repeated stop is the same turn");
|
||||
s.apply_event(&ev(AgentEventKind::Notification));
|
||||
assert_eq!(s.turns, 1);
|
||||
|
||||
s.apply_event(&ev(AgentEventKind::PromptSubmit));
|
||||
s.apply_event(&ev(AgentEventKind::Stop));
|
||||
assert_eq!(s.turns, 2, "a second turn is a second count");
|
||||
|
||||
s.apply_event(&ev(AgentEventKind::SessionEnd));
|
||||
assert_eq!(s.turns, 2);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn session_state_tracks_and_releases_the_agent_cwd() {
|
||||
use std::path::PathBuf;
|
||||
|
||||
@@ -1607,6 +1607,7 @@ mod tests {
|
||||
rich: true,
|
||||
cwd: Some("/work/api".into()),
|
||||
activity: 3,
|
||||
turns: 1,
|
||||
},
|
||||
}])),
|
||||
ControlReply::Ok(ReplyOk::AgentStates(Vec::new())),
|
||||
|
||||
@@ -5125,6 +5125,7 @@ mod tests {
|
||||
rich: true,
|
||||
cwd: None,
|
||||
activity: 0,
|
||||
turns: 0,
|
||||
});
|
||||
apply_signals(&mut st, sniffer.feed(b"\x1b]9;noise\x07"));
|
||||
assert_eq!(
|
||||
|
||||
@@ -1840,6 +1840,7 @@ mod tests {
|
||||
rich: true,
|
||||
cwd: Some("/repo/.claude/worktrees/fix-x".into()),
|
||||
activity: 12,
|
||||
turns: 4,
|
||||
})),
|
||||
DaemonMsg::AgentStatus(None),
|
||||
DaemonMsg::LoopbackForward(LoopbackForward { local_port: 49152 }),
|
||||
|
||||
@@ -5630,6 +5630,7 @@ mod tests {
|
||||
rich: true,
|
||||
cwd: None,
|
||||
activity: 0,
|
||||
turns: 0,
|
||||
}))
|
||||
.encode(&mut daemon_side)
|
||||
.unwrap();
|
||||
|
||||
+259
-8
@@ -191,6 +191,26 @@ pub struct ShellParts {
|
||||
pub(crate) owner: Option<crate::core::session::WorkspaceId>,
|
||||
}
|
||||
|
||||
/// What the reader was last shown of each pane's finished agent turn, kept for
|
||||
/// the life of the app rather than of a view.
|
||||
///
|
||||
/// A view is thrown away and built again over the same daemon pane whenever a
|
||||
/// workspace is switched out and back or a window is reopened from the tray,
|
||||
/// and a fresh view sees a `Done` agent arrive from nothing — exactly what a
|
||||
/// turn finishing live looks like. This is how the new view tells the two apart
|
||||
/// (#870).
|
||||
#[derive(Default)]
|
||||
struct AgentReadMarks(std::collections::HashMap<(crate::ui::host_ops::HostId, u64), AgentReadMark>);
|
||||
|
||||
impl gpui::Global for AgentReadMarks {}
|
||||
|
||||
#[derive(Clone)]
|
||||
struct AgentReadMark {
|
||||
session: (Option<String>, Option<Vec<String>>),
|
||||
turns: u64,
|
||||
unread: bool,
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy)]
|
||||
struct DragScroll {
|
||||
overshoot: f32,
|
||||
@@ -393,6 +413,9 @@ pub struct TerminalView {
|
||||
agent_was_rich: bool,
|
||||
agent_result_unread: bool,
|
||||
keep_unread_on_focus: bool,
|
||||
/// Whether this view has seen its pane's agent status move at all. The
|
||||
/// first move is where a rebuilt view consults [`AgentReadMarks`].
|
||||
agent_status_seen: bool,
|
||||
git_status_cwd: Option<std::path::PathBuf>,
|
||||
last_agent_activity: u64,
|
||||
cmd: CmdEditor,
|
||||
@@ -1428,6 +1451,7 @@ impl TerminalView {
|
||||
view.keep_unread_on_focus = false;
|
||||
} else {
|
||||
view.agent_result_unread = false;
|
||||
view.note_agent_result_unread(cx);
|
||||
}
|
||||
view.report_focus_change(true);
|
||||
cx.notify();
|
||||
@@ -1588,6 +1612,7 @@ impl TerminalView {
|
||||
agent_was_rich: false,
|
||||
agent_result_unread: false,
|
||||
keep_unread_on_focus: false,
|
||||
agent_status_seen: false,
|
||||
git_status_cwd: None,
|
||||
last_agent_activity: 0,
|
||||
cmd: CmdEditor::new(),
|
||||
@@ -1867,9 +1892,41 @@ impl TerminalView {
|
||||
self.agent_result_unread
|
||||
}
|
||||
|
||||
pub fn mark_agent_result_unread(&mut self, refocus_incoming: bool) {
|
||||
pub fn mark_agent_result_unread(&mut self, refocus_incoming: bool, cx: &mut App) {
|
||||
self.agent_result_unread = true;
|
||||
self.keep_unread_on_focus = refocus_incoming;
|
||||
self.note_agent_result_unread(cx);
|
||||
}
|
||||
|
||||
/// Leave what the reader has seen of this pane's agent where the pane's
|
||||
/// next view will look for it — see [`AgentReadMarks`]. Anything but a
|
||||
/// finished turn drops the mark: whatever finishes next is news.
|
||||
fn record_agent_read_mark(&self, turns: u64, cx: &mut App) {
|
||||
let key = (self.host_id, self.pane_id);
|
||||
let marks = &mut cx.default_global::<AgentReadMarks>().0;
|
||||
if self.last_agent_status == Some(crate::core::cli_agent::AgentStatus::Done) {
|
||||
marks.insert(
|
||||
key,
|
||||
AgentReadMark {
|
||||
session: self.last_agent_session.clone(),
|
||||
turns,
|
||||
unread: self.agent_result_unread,
|
||||
},
|
||||
);
|
||||
} else {
|
||||
marks.remove(&key);
|
||||
}
|
||||
}
|
||||
|
||||
/// Carry a change to the badge alone into the mark the last status left.
|
||||
fn note_agent_result_unread(&self, cx: &mut App) {
|
||||
if !cx.has_global::<AgentReadMarks>() {
|
||||
return;
|
||||
}
|
||||
let key = (self.host_id, self.pane_id);
|
||||
if let Some(mark) = cx.global_mut::<AgentReadMarks>().0.get_mut(&key) {
|
||||
mark.unread = self.agent_result_unread;
|
||||
}
|
||||
}
|
||||
|
||||
pub fn git_status(&self, cx: &App) -> Option<crate::terminal::git_status::GitStatus> {
|
||||
@@ -3909,6 +3966,30 @@ impl TerminalView {
|
||||
return false;
|
||||
}
|
||||
let prev = std::mem::replace(&mut self.last_agent_status, status);
|
||||
let first_sight = !std::mem::replace(&mut self.agent_status_seen, true);
|
||||
let turns = session.as_ref().map_or(0, |s| s.turns);
|
||||
|
||||
// A view built over a pane that already holds a finished turn sees
|
||||
// `Done` arrive from nothing, the same as a turn finishing now. If the
|
||||
// pane's previous view left a mark for this session at this turn count,
|
||||
// it is the turn the reader was already shown: take their badge back as
|
||||
// they left it instead of raising a new one (#870). A turn that
|
||||
// finished after the old view went has no such mark, or a lower count.
|
||||
if first_sight
|
||||
&& status == Some(AgentStatus::Done)
|
||||
&& let Some(mark) = cx
|
||||
.try_global::<AgentReadMarks>()
|
||||
.and_then(|marks| marks.0.get(&(self.host_id, self.pane_id)))
|
||||
.filter(|mark| mark.session == self.last_agent_session && mark.turns == turns)
|
||||
.cloned()
|
||||
{
|
||||
self.agent_result_unread = mark.unread && !self.focus_handle.is_focused(window);
|
||||
self.keep_unread_on_focus = false;
|
||||
self.record_agent_read_mark(turns, cx);
|
||||
cx.notify();
|
||||
return false;
|
||||
}
|
||||
|
||||
let turn_finished = status == Some(AgentStatus::Done) && prev != Some(AgentStatus::Done);
|
||||
|
||||
match status {
|
||||
@@ -3928,6 +4009,7 @@ impl TerminalView {
|
||||
self.keep_unread_on_focus = false;
|
||||
}
|
||||
}
|
||||
self.record_agent_read_mark(turns, cx);
|
||||
|
||||
let rich = session.as_ref().is_some_and(|s| s.rich);
|
||||
let agent_name = self
|
||||
@@ -9870,6 +9952,7 @@ mod gpui_tests {
|
||||
rich: true,
|
||||
cwd: None,
|
||||
activity: 0,
|
||||
turns: 0,
|
||||
}))
|
||||
.encode(daemon)
|
||||
.unwrap();
|
||||
@@ -9925,6 +10008,7 @@ mod gpui_tests {
|
||||
rich: true,
|
||||
cwd: None,
|
||||
activity: 0,
|
||||
turns: 0,
|
||||
}))
|
||||
.encode(&mut daemon)
|
||||
.unwrap();
|
||||
@@ -9969,10 +10053,21 @@ mod gpui_tests {
|
||||
pane: &gpui::Entity<TerminalView>,
|
||||
cx: &mut TestAppContext,
|
||||
daemon: &mut Stream,
|
||||
) {
|
||||
report_agent_turn(status, 0, pane, cx, daemon);
|
||||
}
|
||||
|
||||
/// The same, for a session that has finished `turns` turns so far.
|
||||
fn report_agent_turn(
|
||||
status: crate::core::cli_agent::AgentStatus,
|
||||
turns: u64,
|
||||
pane: &gpui::Entity<TerminalView>,
|
||||
cx: &mut TestAppContext,
|
||||
daemon: &mut Stream,
|
||||
) {
|
||||
use crate::core::cli_agent::AgentSessionState;
|
||||
|
||||
DaemonMsg::AgentStatus(Some(AgentSessionState {
|
||||
let state = AgentSessionState {
|
||||
status,
|
||||
message: None,
|
||||
session_id: Some("sid-abc".into()),
|
||||
@@ -9980,13 +10075,13 @@ mod gpui_tests {
|
||||
rich: true,
|
||||
cwd: None,
|
||||
activity: 0,
|
||||
}))
|
||||
.encode(daemon)
|
||||
.unwrap();
|
||||
turns,
|
||||
};
|
||||
DaemonMsg::AgentStatus(Some(state.clone()))
|
||||
.encode(daemon)
|
||||
.unwrap();
|
||||
for _ in 0..200 {
|
||||
if cx.update(|cx| pane.read(cx).terminal.agent_session().map(|s| s.status))
|
||||
== Some(status)
|
||||
{
|
||||
if cx.update(|cx| pane.read(cx).terminal.agent_session()) == Some(state.clone()) {
|
||||
return;
|
||||
}
|
||||
std::thread::sleep(std::time::Duration::from_millis(5));
|
||||
@@ -9994,6 +10089,161 @@ mod gpui_tests {
|
||||
panic!("the agent status never reached the pane");
|
||||
}
|
||||
|
||||
/// Poll `pane`'s agent status inside `window` and read its badge back.
|
||||
fn poll_unread(
|
||||
window: gpui::WindowHandle<TerminalView>,
|
||||
pane: &gpui::Entity<TerminalView>,
|
||||
cx: &mut TestAppContext,
|
||||
) -> bool {
|
||||
// Through the untyped handle: the typed one leases the root view, and
|
||||
// `pane` may be that view.
|
||||
cx.update_window(window.into(), |_, window, cx| {
|
||||
pane.update(cx, |pane, cx| {
|
||||
pane.poll_agent_status(false, window, cx);
|
||||
pane.agent_result_unread()
|
||||
})
|
||||
})
|
||||
.unwrap()
|
||||
}
|
||||
|
||||
/// Switching workspaces, or reopening a window from the tray, throws the
|
||||
/// pane's view away and builds a new one on the same daemon pane (#870).
|
||||
/// The new view's first look at a `Done` agent is not a turn finishing —
|
||||
/// the reader watched that one finish before the old view went.
|
||||
#[gpui::test]
|
||||
fn a_rebuilt_pane_does_not_re_badge_a_turn_the_reader_already_saw(cx: &mut TestAppContext) {
|
||||
use crate::core::cli_agent::AgentStatus;
|
||||
|
||||
let (window, mut before_daemon) = harness(cx);
|
||||
let before = window.update(cx, |_, _, cx| cx.entity()).unwrap();
|
||||
window
|
||||
.update(cx, |view, window, cx| {
|
||||
view.focus_handle.clone().focus(window, cx)
|
||||
})
|
||||
.unwrap();
|
||||
cx.run_until_parked();
|
||||
report_agent_turn(AgentStatus::Done, 1, &before, cx, &mut before_daemon);
|
||||
assert!(
|
||||
!poll_unread(window, &before, cx),
|
||||
"the reader watched it finish"
|
||||
);
|
||||
|
||||
// The same daemon pane, rebuilt the way `tabs_from_session` rebuilds it,
|
||||
// with the reader's focus somewhere else.
|
||||
let (after, mut daemon) = window
|
||||
.update(cx, |_, window, cx| super::quiet_test_pane(1, window, cx))
|
||||
.unwrap();
|
||||
window
|
||||
.update(cx, |view, window, cx| {
|
||||
view.focus_handle.clone().focus(window, cx)
|
||||
})
|
||||
.unwrap();
|
||||
cx.run_until_parked();
|
||||
report_agent_turn(AgentStatus::Done, 1, &after, cx, &mut daemon);
|
||||
assert!(
|
||||
!poll_unread(window, &after, cx),
|
||||
"rebuilding the pane is not a turn finishing"
|
||||
);
|
||||
}
|
||||
|
||||
/// What the rebuild must not swallow: a turn that was still running when
|
||||
/// the view went away and finished before the new one arrived.
|
||||
#[gpui::test]
|
||||
fn a_turn_that_finished_while_the_pane_was_away_still_badges(cx: &mut TestAppContext) {
|
||||
use crate::core::cli_agent::AgentStatus;
|
||||
|
||||
let (window, mut before_daemon) = harness(cx);
|
||||
let before = window.update(cx, |_, _, cx| cx.entity()).unwrap();
|
||||
window
|
||||
.update(cx, |view, window, cx| {
|
||||
view.focus_handle.clone().focus(window, cx)
|
||||
})
|
||||
.unwrap();
|
||||
cx.run_until_parked();
|
||||
report_agent_turn(AgentStatus::Working, 0, &before, cx, &mut before_daemon);
|
||||
assert!(!poll_unread(window, &before, cx));
|
||||
|
||||
let (after, mut daemon) = window
|
||||
.update(cx, |_, window, cx| super::quiet_test_pane(1, window, cx))
|
||||
.unwrap();
|
||||
window
|
||||
.update(cx, |view, window, cx| {
|
||||
view.focus_handle.clone().focus(window, cx)
|
||||
})
|
||||
.unwrap();
|
||||
cx.run_until_parked();
|
||||
report_agent_turn(AgentStatus::Done, 1, &after, cx, &mut daemon);
|
||||
assert!(
|
||||
poll_unread(window, &after, cx),
|
||||
"nobody saw this turn finish"
|
||||
);
|
||||
}
|
||||
|
||||
/// Nor a whole later turn: the reader saw turn one, the agent was sent
|
||||
/// another and finished it while the pane was away. The status reads
|
||||
/// `Done` both times; only the turn count tells them apart.
|
||||
#[gpui::test]
|
||||
fn a_later_turn_that_finished_while_the_pane_was_away_still_badges(cx: &mut TestAppContext) {
|
||||
use crate::core::cli_agent::AgentStatus;
|
||||
|
||||
let (window, mut before_daemon) = harness(cx);
|
||||
let before = window.update(cx, |_, _, cx| cx.entity()).unwrap();
|
||||
window
|
||||
.update(cx, |view, window, cx| {
|
||||
view.focus_handle.clone().focus(window, cx)
|
||||
})
|
||||
.unwrap();
|
||||
cx.run_until_parked();
|
||||
report_agent_turn(AgentStatus::Done, 1, &before, cx, &mut before_daemon);
|
||||
assert!(!poll_unread(window, &before, cx));
|
||||
|
||||
let (after, mut daemon) = window
|
||||
.update(cx, |_, window, cx| super::quiet_test_pane(1, window, cx))
|
||||
.unwrap();
|
||||
window
|
||||
.update(cx, |view, window, cx| {
|
||||
view.focus_handle.clone().focus(window, cx)
|
||||
})
|
||||
.unwrap();
|
||||
cx.run_until_parked();
|
||||
report_agent_turn(AgentStatus::Done, 2, &after, cx, &mut daemon);
|
||||
assert!(
|
||||
poll_unread(window, &after, cx),
|
||||
"the second turn finished unseen"
|
||||
);
|
||||
}
|
||||
|
||||
/// And a badge the reader had not cleared yet comes back with the pane.
|
||||
#[gpui::test]
|
||||
fn an_unread_turn_is_still_unread_after_the_pane_is_rebuilt(cx: &mut TestAppContext) {
|
||||
use crate::core::cli_agent::AgentStatus;
|
||||
|
||||
let (window, mut before_daemon) = harness(cx);
|
||||
let before = window.update(cx, |_, _, cx| cx.entity()).unwrap();
|
||||
// Building another pane takes the window's focus off `before`.
|
||||
let (_elsewhere, _elsewhere_daemon) = window
|
||||
.update(cx, |_, window, cx| super::quiet_test_pane(5, window, cx))
|
||||
.unwrap();
|
||||
cx.run_until_parked();
|
||||
report_agent_turn(AgentStatus::Done, 1, &before, cx, &mut before_daemon);
|
||||
assert!(poll_unread(window, &before, cx), "nobody was looking");
|
||||
|
||||
let (after, mut daemon) = window
|
||||
.update(cx, |_, window, cx| super::quiet_test_pane(1, window, cx))
|
||||
.unwrap();
|
||||
window
|
||||
.update(cx, |view, window, cx| {
|
||||
view.focus_handle.clone().focus(window, cx)
|
||||
})
|
||||
.unwrap();
|
||||
cx.run_until_parked();
|
||||
report_agent_turn(AgentStatus::Done, 1, &after, cx, &mut daemon);
|
||||
assert!(
|
||||
poll_unread(window, &after, cx),
|
||||
"rebuilding the pane is not reading it"
|
||||
);
|
||||
}
|
||||
|
||||
/// The badge answers "did the reader see this?", so it has to read the
|
||||
/// window's live focus rather than anything a focus callback left behind.
|
||||
///
|
||||
@@ -10091,6 +10341,7 @@ mod gpui_tests {
|
||||
rich: true,
|
||||
cwd: Some(working_in.clone()),
|
||||
activity: 0,
|
||||
turns: 0,
|
||||
}))
|
||||
.encode(&mut daemon)
|
||||
.unwrap();
|
||||
|
||||
+1
-1
@@ -4834,7 +4834,7 @@ impl Tty7App {
|
||||
refocus.as_ref().map(|s| s.entity_id()) == Some(leaf.entity_id());
|
||||
leaf.update(cx, |view, cx| {
|
||||
if view.agent_session().map(|s| s.status) == Some(AgentStatus::Done) {
|
||||
view.mark_agent_result_unread(refocus_incoming);
|
||||
view.mark_agent_result_unread(refocus_incoming, cx);
|
||||
cx.notify();
|
||||
}
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user