Order the session list by when each agent was turned on
A running session no longer moves: the ones with a process come first, oldest start first, so starting one appends it to the bottom of that group and nothing it goes on to do -- beginning a turn, finishing one, asking a question -- can shift it. Sorting by activity with the awaiting-answer ones floated to the top is what this replaces; the status word and its colour already say which session wants something without the row having to move to say it. Stopped sessions are a group below, most recently active first. The order is the server's: `SessionConfig::started` is written each time a process is started for a session and reported as `started`, so it is the same on every device and survives a backend restart -- which adopts processes rather than starting them, and so could not work the times out for itself. Applied on the phone, because presentation order is a display decision. `LiveSession::info` takes the session's config entry rather than a parameter per field read from it, which is what `AutoResumeView` existed to bundle; that goes. Verified on the emulator against the sandbox: three echo sessions kept their order while the newest-active one was messaged; a stopped and restarted session moved below one started after it; a stopped session dropped below every running one; and after a backend restart the recorded times came back unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
a9cfea89e5
commit
cbae7ee8c0
6 files changed
+255
-64
No files matched your search
@@ -392,6 +392,25 @@ pub struct SessionConfig {
|
||||
#[serde(default, skip_serializing_if = "not_set")]
|
||||
pub throwaway: bool,
|
||||
pub created: f64,
|
||||
/// Epoch seconds a process was last started for this session, and `None`
|
||||
/// for a session written before this was recorded.
|
||||
///
|
||||
/// The session list orders running sessions by it, so one that is turned
|
||||
/// on joins the bottom of that group and stays where it is however busy
|
||||
/// it gets. Persisted rather than held in memory because that order has
|
||||
/// to survive a backend restart -- sessions are adopted, not restarted,
|
||||
/// so nothing in the run that adopts them knows when they began.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub started: Option<f64>,
|
||||
}
|
||||
|
||||
impl SessionConfig {
|
||||
/// When this session's agent was turned on. A session written before that
|
||||
/// was recorded answers with its creation, which is when its first
|
||||
/// process started.
|
||||
pub fn started_at(&self) -> f64 {
|
||||
self.started.unwrap_or(self.created)
|
||||
}
|
||||
}
|
||||
|
||||
/// A message owed to a session whose account ran out, and when to try sending
|
||||
@@ -598,6 +617,7 @@ mod tests {
|
||||
resume: None,
|
||||
throwaway: false,
|
||||
created: 1234.5,
|
||||
started: Some(2345.5),
|
||||
}],
|
||||
};
|
||||
config.save(&path).expect("save");
|
||||
@@ -608,6 +628,8 @@ mod tests {
|
||||
// The label and the id are separate, and the session holds the id.
|
||||
assert_eq!(loaded.machine("vm").expect("machine").name, "the vm");
|
||||
assert_eq!(loaded.sessions[0].provider, "claude-cli");
|
||||
// The list's order is made of this, so it has to survive the file.
|
||||
assert_eq!(loaded.sessions[0].started, Some(2345.5));
|
||||
assert_eq!(
|
||||
loaded
|
||||
.machine("vm")
|
||||
|
||||
+99
-57
@@ -139,29 +139,6 @@ pub enum NotificationKind {
|
||||
Finished,
|
||||
}
|
||||
|
||||
/// What a session's auto-resume setting looks like from outside: on or off,
|
||||
/// what it would say, and when it next intends to check.
|
||||
///
|
||||
/// One struct rather than three parameters on [`LiveSession::info`], and read
|
||||
/// from the config rather than from the launch snapshot beside it, for the
|
||||
/// reason `cwd` is: all three change under a running session.
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct AutoResumeView {
|
||||
pub on: bool,
|
||||
pub message: String,
|
||||
pub at: Option<f64>,
|
||||
}
|
||||
|
||||
impl AutoResumeView {
|
||||
fn of(meta: &SessionConfig) -> Self {
|
||||
Self {
|
||||
on: meta.auto_resume,
|
||||
message: resume_message(meta),
|
||||
at: meta.resume.map(|scheduled| scheduled.at),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// A session with a message owed to it once its account has quota again --
|
||||
/// see [`SessionManager::owed_resumes`].
|
||||
///
|
||||
@@ -272,6 +249,10 @@ pub struct SessionInfo {
|
||||
pub status: SessionStatus,
|
||||
pub last_activity: f64,
|
||||
pub created: f64,
|
||||
/// Epoch seconds this session's agent was last turned on, which the
|
||||
/// session list orders the running ones by -- see
|
||||
/// [`SessionConfig::started`], whose fallback this resolves.
|
||||
pub started: f64,
|
||||
/// The provider's latest measured number of live background tasks.
|
||||
/// Absent until it reports one; absence is not a measured zero.
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
@@ -566,11 +547,12 @@ impl LiveSession {
|
||||
Ok((name, path))
|
||||
}
|
||||
|
||||
/// `machine_name` and `cwd` are passed in rather than read from the
|
||||
/// snapshot this session launched with: only the manager holds the
|
||||
/// config, and both can change under a running session. Passed rather
|
||||
/// than mirrored into `Shared`, so there is one answer, read where the
|
||||
/// row is built.
|
||||
/// `machine_name` and the session's own config entry are passed in
|
||||
/// rather than read from the snapshot this session launched with: only
|
||||
/// the manager holds the config, and a setting, a working directory or
|
||||
/// the time of the latest start can all change under a running session.
|
||||
/// Passed rather than mirrored into `Shared`, so there is one answer,
|
||||
/// read where the row is built.
|
||||
///
|
||||
/// `kind` rather than the facts derived from it, or every caller
|
||||
/// derives each one separately. `None` where the provider has been
|
||||
@@ -579,11 +561,9 @@ impl LiveSession {
|
||||
fn info(
|
||||
&self,
|
||||
machine_name: &str,
|
||||
cwd: Option<&Path>,
|
||||
effort: Option<&str>,
|
||||
current: &SessionConfig,
|
||||
imported: bool,
|
||||
kind: Option<DriverKind>,
|
||||
resume: AutoResumeView,
|
||||
) -> SessionInfo {
|
||||
SessionInfo {
|
||||
id: self.meta.id.clone(),
|
||||
@@ -596,22 +576,23 @@ impl LiveSession {
|
||||
// From the config rather than from `shared`, like the cwd beside
|
||||
// it: neither can change under a running process, so there is no
|
||||
// live value for one to disagree with.
|
||||
effort: effort.map(str::to_string),
|
||||
effort: current.effort.clone(),
|
||||
takes_effort: kind.is_some_and(DriverKind::takes_effort),
|
||||
context_tokens: *self.shared.context_tokens.lock().unwrap(),
|
||||
notify: *self.shared.notify.lock().unwrap(),
|
||||
auto_resume: resume.on,
|
||||
auto_resume_message: resume.message,
|
||||
resume_at: resume.at,
|
||||
auto_resume: current.auto_resume,
|
||||
auto_resume_message: resume_message(current),
|
||||
resume_at: current.resume.map(|scheduled| scheduled.at),
|
||||
max_image_edge: kind.and_then(DriverKind::max_image_edge),
|
||||
usage_provider: kind.and_then(DriverKind::usage_provider),
|
||||
imported,
|
||||
keeps_own_transcript: kind.is_some_and(DriverKind::keeps_own_transcript),
|
||||
own_transcript_name: kind.and_then(DriverKind::own_transcript_name),
|
||||
cwd: cwd.map(Path::to_path_buf),
|
||||
cwd: current.cwd.clone(),
|
||||
status: *self.shared.status.lock().unwrap(),
|
||||
last_activity: *self.shared.last_activity.lock().unwrap(),
|
||||
created: self.meta.created,
|
||||
started: current.started_at(),
|
||||
background_tasks: self.driver().and_then(|driver| driver.background_tasks()),
|
||||
subagents: subagent::count(self.dir()),
|
||||
}
|
||||
@@ -1107,11 +1088,9 @@ impl SessionManager {
|
||||
.map(|meta| match inner.live.get(&meta.id) {
|
||||
Some(session) => session.info(
|
||||
label_of(&inner.config, &meta.machine),
|
||||
meta.cwd.as_deref(),
|
||||
meta.effort.as_deref(),
|
||||
meta,
|
||||
import::read_cursor(&self.data_dir.join(&meta.id)).is_some(),
|
||||
kind_of(&inner.config, &meta.machine, &meta.provider),
|
||||
AutoResumeView::of(meta),
|
||||
),
|
||||
None => SessionInfo {
|
||||
id: meta.id.clone(),
|
||||
@@ -1145,6 +1124,7 @@ impl SessionManager {
|
||||
status: status_of_unlaunched(&self.data_dir.join(&meta.id)),
|
||||
last_activity: meta.created,
|
||||
created: meta.created,
|
||||
started: meta.started_at(),
|
||||
background_tasks: None,
|
||||
subagents: subagent::count(&self.data_dir.join(&meta.id)),
|
||||
},
|
||||
@@ -1223,6 +1203,7 @@ impl SessionManager {
|
||||
})?
|
||||
.clone();
|
||||
let id = unique_id(&inner.config);
|
||||
let created = now();
|
||||
let title = spec
|
||||
.title
|
||||
.filter(|title| !title.trim().is_empty())
|
||||
@@ -1268,7 +1249,10 @@ impl SessionManager {
|
||||
// whichever server is running when the time comes knows what to
|
||||
// do with it -- see `SessionConfig::throwaway`.
|
||||
throwaway: self.spawn_throwaway,
|
||||
created: now(),
|
||||
created,
|
||||
// The spawn starts a process, so this session is turned on from
|
||||
// the moment it exists.
|
||||
started: Some(created),
|
||||
};
|
||||
|
||||
let session = launch(
|
||||
@@ -1293,11 +1277,9 @@ impl SessionManager {
|
||||
// of the directory a moment later.
|
||||
let info = session.info(
|
||||
&machine.name,
|
||||
session.meta.cwd.as_deref(),
|
||||
session.meta.effort.as_deref(),
|
||||
&session.meta,
|
||||
import::read_cursor(&self.data_dir.join(&id)).is_some(),
|
||||
Some(provider.kind),
|
||||
AutoResumeView::of(&session.meta),
|
||||
);
|
||||
inner.live.insert(id, session);
|
||||
Ok(info)
|
||||
@@ -1931,6 +1913,18 @@ impl SessionManager {
|
||||
inner.live.insert(id.to_string(), session);
|
||||
}
|
||||
}
|
||||
// Turned on now, so it takes its place at the bottom of the running
|
||||
// group -- see `SessionConfig::started`. Best effort: a config that
|
||||
// cannot be written is not a reason to fail a start that has already
|
||||
// happened, and the cost is one row in the wrong place.
|
||||
let mut candidate = inner.config.clone();
|
||||
if let Some(meta) = candidate.sessions.iter_mut().find(|meta| meta.id == id) {
|
||||
meta.started = Some(now());
|
||||
match candidate.save(&self.config_path) {
|
||||
Ok(()) => inner.config = candidate,
|
||||
Err(err) => tracing::warn!("couldn't record that session {id} started: {err:#}"),
|
||||
}
|
||||
}
|
||||
// Nothing is announced from here. A driver that starts a process
|
||||
// reports the session idle itself, in order with everything else it
|
||||
// says about that process. Saying it here too would be a second
|
||||
@@ -3219,20 +3213,7 @@ mod tests {
|
||||
// open to look one up on.
|
||||
assert_eq!(
|
||||
first.title,
|
||||
session
|
||||
.info(
|
||||
"m",
|
||||
None,
|
||||
None,
|
||||
false,
|
||||
None,
|
||||
AutoResumeView {
|
||||
on: false,
|
||||
message: DEFAULT_RESUME_MESSAGE.to_string(),
|
||||
at: None,
|
||||
},
|
||||
)
|
||||
.title
|
||||
session.info("m", &session.meta, false, None).title
|
||||
);
|
||||
|
||||
manager.set_session_notify(&info.id, false).expect("off");
|
||||
@@ -4278,6 +4259,67 @@ mod tests {
|
||||
.await;
|
||||
}
|
||||
|
||||
/// The session list keeps the running sessions in the order their agents
|
||||
/// were turned on, so what that order is made of has to outlive both the
|
||||
/// process and the backend: a session started again belongs at the bottom
|
||||
/// of the group, and a restart adopts processes rather than starting them,
|
||||
/// so nothing in the new run could work the time out for itself.
|
||||
#[tokio::test]
|
||||
async fn starting_a_session_again_records_when_it_was_turned_on() {
|
||||
let dir = tempfile::tempdir().expect("tempdir");
|
||||
let config_path = dir.path().join("config.ron");
|
||||
let data_dir = dir.path().join("sessions");
|
||||
seed_echo_only(&config_path);
|
||||
let manager = SessionManager::new(
|
||||
config_path.clone(),
|
||||
data_dir.clone(),
|
||||
data_dir.join("models"),
|
||||
)
|
||||
.expect("manager");
|
||||
let info = manager.spawn_session(echo_spec()).expect("spawn");
|
||||
let spawned = info.started;
|
||||
assert!(
|
||||
(spawned - info.created).abs() < 1.0,
|
||||
"a spawned session was turned on at {spawned} rather than when it was created",
|
||||
);
|
||||
|
||||
let session = manager.session(&info.id).expect("live session");
|
||||
let mut rx = session.subscribe();
|
||||
let _ = session.sink.send(Event::Status {
|
||||
state: SessionStatus::Exited,
|
||||
});
|
||||
collect_until(&mut rx, |event| {
|
||||
matches!(
|
||||
event,
|
||||
Event::Status {
|
||||
state: SessionStatus::Exited
|
||||
}
|
||||
)
|
||||
})
|
||||
.await;
|
||||
// Otherwise the two starts can land in the same instant and the test
|
||||
// cannot tell a recorded time from a kept one.
|
||||
tokio::time::sleep(Duration::from_millis(20)).await;
|
||||
manager.start_session(&info.id).expect("start again");
|
||||
collect_until(&mut rx, is_idle).await;
|
||||
|
||||
let restarted = manager.sessions()[0].started;
|
||||
assert!(
|
||||
restarted > spawned,
|
||||
"starting the session again left it reporting {restarted}, the {spawned} of the \
|
||||
process that had already ended",
|
||||
);
|
||||
drop(manager);
|
||||
|
||||
let manager = SessionManager::new(config_path, data_dir.clone(), data_dir.join("models"))
|
||||
.expect("manager restart");
|
||||
assert_eq!(
|
||||
manager.sessions()[0].started,
|
||||
restarted,
|
||||
"the backend restart lost when this session's agent was turned on",
|
||||
);
|
||||
}
|
||||
|
||||
/// Stopping and starting a session is about its *process*, and the two
|
||||
/// refusals are what keeps starting one from becoming a second one on the
|
||||
/// same conversation. Echo has no process, which makes it the right
|
||||
|
||||
Reference in new issue
Block a user