Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
fa846000e7 | ||
|
|
8325bc6d9a | ||
|
|
8f499dbee3 | ||
|
|
511fed408c |
@@ -252,6 +252,7 @@ mod tests {
|
||||
current_cwd: None,
|
||||
blocked_work: None,
|
||||
last_focused_at: None,
|
||||
agent_session_id: None,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -26,11 +26,17 @@ pub(crate) async fn serve(
|
||||
) -> Result<(), axum::Error> {
|
||||
let (mut sink, mut stream) = socket.split();
|
||||
|
||||
// This client's claim on the shared window size. Registered up front and
|
||||
// released on every exit below, so a socket that dies mid-session stops
|
||||
// constraining the size for the clients still attached.
|
||||
let viewport = manager.attach_viewport();
|
||||
|
||||
// NAP-229: send buffered scrollback up-front so the client sees the last
|
||||
// screenful immediately instead of a blank terminal. The replay lock the
|
||||
// bridge held during snapshot+subscribe guarantees no byte appears both
|
||||
// here and on the live receiver, and none is dropped at the seam.
|
||||
if !replay.is_empty() && sink.send(Message::Binary(replay)).await.is_err() {
|
||||
manager.release_viewport(id, viewport).await;
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
@@ -70,7 +76,7 @@ pub(crate) async fn serve(
|
||||
if let Ok(ctrl) = serde_json::from_str::<ClientControl>(&text) {
|
||||
match ctrl {
|
||||
ClientControl::Resize { cols, rows } => {
|
||||
let _ = manager.resize(id, cols, rows).await;
|
||||
let _ = manager.set_viewport(id, viewport, cols, rows).await;
|
||||
}
|
||||
ClientControl::Ping => {
|
||||
let pong = serde_json::to_string(&ClientControl::Pong)
|
||||
@@ -88,6 +94,8 @@ pub(crate) async fn serve(
|
||||
}
|
||||
}
|
||||
|
||||
manager.release_viewport(id, viewport).await;
|
||||
|
||||
if session_ended {
|
||||
let _ = sink.send(Message::Close(None)).await;
|
||||
}
|
||||
|
||||
@@ -30,6 +30,7 @@ pub mod spawn_history;
|
||||
#[cfg(feature = "web")]
|
||||
pub mod static_bundle;
|
||||
pub mod tmux;
|
||||
pub mod viewport;
|
||||
pub mod worktree;
|
||||
pub mod worktree_probe;
|
||||
|
||||
|
||||
@@ -307,7 +307,11 @@ async fn report_activity(
|
||||
Path(id): Path<Uuid>,
|
||||
Json(body): Json<ReportActivityRequest>,
|
||||
) -> Response {
|
||||
match s.manager.report_activity(id, body.state, body.detail).await {
|
||||
match s
|
||||
.manager
|
||||
.report_activity(id, body.state, body.detail, body.session_id)
|
||||
.await
|
||||
{
|
||||
Ok(()) => StatusCode::NO_CONTENT.into_response(),
|
||||
Err(e) => {
|
||||
let info = e.to_error_info();
|
||||
|
||||
@@ -7,6 +7,7 @@
|
||||
|
||||
use std::collections::{HashMap, HashSet};
|
||||
use std::path::PathBuf;
|
||||
use std::sync::atomic::{AtomicU64, Ordering};
|
||||
use std::sync::Arc;
|
||||
use std::time::Duration;
|
||||
|
||||
@@ -23,6 +24,7 @@ use crate::pty::{pipe_pane_command, PtyBridge};
|
||||
use crate::sidecar::{SidecarError, SidecarStore};
|
||||
use crate::spawn_history::{RecentDir, SpawnHistory, SpawnHistoryRecord};
|
||||
use crate::tmux::TmuxError;
|
||||
use crate::viewport::{ViewportId, Viewports};
|
||||
use crate::worktree::{WorktreeError, WorktreeManager};
|
||||
|
||||
/// Capacity of the session-events broadcast channel. A subscriber that falls
|
||||
@@ -166,6 +168,14 @@ struct Inner {
|
||||
report_ttl: Duration,
|
||||
sessions: Mutex<HashMap<Uuid, SessionRecord>>,
|
||||
events: broadcast::Sender<CatalogEvent>,
|
||||
/// Sizes reported by the clients attached to each session, and what tmux
|
||||
/// was last told. See [`crate::viewport`] for why the window is not simply
|
||||
/// set to whatever arrived most recently.
|
||||
viewports: Mutex<Viewports>,
|
||||
/// Hands out [`ViewportId`]s. Monotonic for the daemon's lifetime, so a
|
||||
/// client that reconnects never collides with the registration its
|
||||
/// previous socket has not finished releasing.
|
||||
next_viewport: AtomicU64,
|
||||
}
|
||||
|
||||
impl SessionManager {
|
||||
@@ -206,6 +216,8 @@ impl SessionManager {
|
||||
report_ttl: crate::activity::REPORT_TTL,
|
||||
sessions: Mutex::new(HashMap::new()),
|
||||
events,
|
||||
viewports: Mutex::new(Viewports::new()),
|
||||
next_viewport: AtomicU64::new(0),
|
||||
}),
|
||||
}
|
||||
}
|
||||
@@ -466,6 +478,7 @@ impl SessionManager {
|
||||
// fresh tree has nothing in the way anyway.
|
||||
blocked_work: None,
|
||||
last_focused_at: None,
|
||||
agent_session_id: None,
|
||||
};
|
||||
|
||||
if let Err(e) = self.inner.sidecar.write(&session) {
|
||||
@@ -671,6 +684,7 @@ impl SessionManager {
|
||||
current_cwd: None,
|
||||
blocked_work: None,
|
||||
last_focused_at: None,
|
||||
agent_session_id: None,
|
||||
},
|
||||
};
|
||||
let fifo_path = self.inner.fifo_dir.join(format!("{id}.fifo"));
|
||||
@@ -770,6 +784,52 @@ impl SessionManager {
|
||||
.map_err(SessionError::Tmux)
|
||||
}
|
||||
|
||||
/// Register a client that is about to start reporting its display size,
|
||||
/// and hand back the handle it reports under.
|
||||
///
|
||||
/// Cheap and infallible: the session it will report against is not
|
||||
/// consulted until the first [`Self::set_viewport`].
|
||||
pub fn attach_viewport(&self) -> ViewportId {
|
||||
self.inner.next_viewport.fetch_add(1, Ordering::Relaxed)
|
||||
}
|
||||
|
||||
/// A client reported the size it can display.
|
||||
///
|
||||
/// The shared tmux window is resized only when the size arbitrated across
|
||||
/// every attached client actually changes — see [`crate::viewport`]. Two
|
||||
/// clients of different sizes used to resize the window past each other on
|
||||
/// every report, which leaves a full-screen agent repainting into rows it
|
||||
/// never clears and looking, to the operator, like a pane that has stopped
|
||||
/// accepting input.
|
||||
pub async fn set_viewport(
|
||||
&self,
|
||||
id: Uuid,
|
||||
viewport: ViewportId,
|
||||
cols: u16,
|
||||
rows: u16,
|
||||
) -> Result<(), SessionError> {
|
||||
let target = self
|
||||
.inner
|
||||
.viewports
|
||||
.lock()
|
||||
.await
|
||||
.set(id, viewport, cols, rows);
|
||||
match target {
|
||||
Some((cols, rows)) => self.resize(id, cols, rows).await,
|
||||
None => Ok(()),
|
||||
}
|
||||
}
|
||||
|
||||
/// Drop a detached client's claim on the window size, resizing to what the
|
||||
/// clients still attached can display. Errors are the caller's to ignore:
|
||||
/// a socket closing because its session died has nothing left to resize.
|
||||
pub async fn release_viewport(&self, id: Uuid, viewport: ViewportId) {
|
||||
let target = self.inner.viewports.lock().await.release(id, viewport);
|
||||
if let Some((cols, rows)) = target {
|
||||
let _ = self.resize(id, cols, rows).await;
|
||||
}
|
||||
}
|
||||
|
||||
/// Rename a session (#125): set `metadata.name`, or clear it when the
|
||||
/// requested name is blank so the session falls back to its command.
|
||||
///
|
||||
@@ -823,11 +883,18 @@ impl SessionManager {
|
||||
/// A report that changes the effective state restarts `activity_since`
|
||||
/// (#104's clock); re-reporting an unchanged state only extends the TTL
|
||||
/// and stays silent on the event stream at steady state.
|
||||
///
|
||||
/// `agent_session_id` (#161) is the agent's own conversation id, parsed
|
||||
/// by the caller from the hook payload's `session_id` field. It is
|
||||
/// written once — the first report that carries one wins — and the
|
||||
/// sidecar is only rewritten on that transition, never on every report,
|
||||
/// since an agent's session id never changes mid-session.
|
||||
pub async fn report_activity(
|
||||
&self,
|
||||
id: Uuid,
|
||||
state: ActivityState,
|
||||
detail: Option<String>,
|
||||
agent_session_id: Option<String>,
|
||||
) -> Result<(), SessionError> {
|
||||
let updated = {
|
||||
let mut sessions = self.inner.sessions.lock().await;
|
||||
@@ -843,7 +910,16 @@ impl SessionManager {
|
||||
state,
|
||||
self.inner.report_ttl,
|
||||
));
|
||||
changed.then(|| rec.session.clone())
|
||||
let newly_identified =
|
||||
rec.session.agent_session_id.is_none() && agent_session_id.is_some();
|
||||
if newly_identified {
|
||||
rec.session.agent_session_id = agent_session_id;
|
||||
self.inner
|
||||
.sidecar
|
||||
.write(&rec.session)
|
||||
.map_err(SessionError::Sidecar)?;
|
||||
}
|
||||
(changed || newly_identified).then(|| rec.session.clone())
|
||||
};
|
||||
if let Some(session) = updated {
|
||||
let _ = self
|
||||
|
||||
@@ -0,0 +1,186 @@
|
||||
//! What size the shared tmux window should be, given every client watching it.
|
||||
//!
|
||||
//! A tmux window has one size, but a session can have several clients attached
|
||||
//! at once — two browser panes, a browser and the TUI. Each reports the size
|
||||
//! *it* can display, and applying whichever arrived last makes the window
|
||||
//! flip-flop. Every flip forces a full-screen application (Claude Code, vim) to
|
||||
//! repaint at a new height, and the rows it drew at the old height are never
|
||||
//! cleared: the operator ends up looking at a stale input box, types into a
|
||||
//! box that is not where the screen says it is, and concludes the pane is
|
||||
//! stuck.
|
||||
//!
|
||||
//! So the window is sized to the smallest attached client on each axis — the
|
||||
//! same rule tmux uses for its own multiple clients, and the only size every
|
||||
//! client can render without truncation. The size last handed to tmux is
|
||||
//! remembered, so a report that changes nothing costs no `tmux` fork at all.
|
||||
|
||||
use std::collections::HashMap;
|
||||
|
||||
use uuid::Uuid;
|
||||
|
||||
/// One attached client within a session. Allocated by
|
||||
/// [`crate::session::SessionManager`] and opaque to everything else.
|
||||
pub type ViewportId = u64;
|
||||
|
||||
/// Per-session client sizes, and what tmux was last told.
|
||||
#[derive(Debug, Default)]
|
||||
pub struct Viewports {
|
||||
clients: HashMap<Uuid, HashMap<ViewportId, (u16, u16)>>,
|
||||
applied: HashMap<Uuid, (u16, u16)>,
|
||||
}
|
||||
|
||||
impl Viewports {
|
||||
pub fn new() -> Self {
|
||||
Self::default()
|
||||
}
|
||||
|
||||
/// Record what `viewport` can display. Returns the size tmux must be set
|
||||
/// to, or `None` when the window already has it.
|
||||
pub fn set(
|
||||
&mut self,
|
||||
session: Uuid,
|
||||
viewport: ViewportId,
|
||||
cols: u16,
|
||||
rows: u16,
|
||||
) -> Option<(u16, u16)> {
|
||||
self.clients
|
||||
.entry(session)
|
||||
.or_default()
|
||||
.insert(viewport, (cols, rows));
|
||||
self.settle(session)
|
||||
}
|
||||
|
||||
/// Drop a client that has detached. Returns the size tmux must be set to
|
||||
/// now that it no longer constrains the window.
|
||||
///
|
||||
/// The last client leaving settles nothing: the window keeps the size it
|
||||
/// has rather than snapping to some remembered default no one is watching.
|
||||
pub fn release(&mut self, session: Uuid, viewport: ViewportId) -> Option<(u16, u16)> {
|
||||
let clients = self.clients.get_mut(&session)?;
|
||||
clients.remove(&viewport);
|
||||
if clients.is_empty() {
|
||||
self.clients.remove(&session);
|
||||
self.applied.remove(&session);
|
||||
return None;
|
||||
}
|
||||
self.settle(session)
|
||||
}
|
||||
|
||||
/// The smallest size every attached client can render, but only when it
|
||||
/// differs from what tmux was last told.
|
||||
///
|
||||
/// A zero on either axis is a client that cannot display anything yet — a
|
||||
/// pane still laying out, a hidden tab — and must not drag the shared
|
||||
/// window down with it, so it is held but left out of the arbitration.
|
||||
fn settle(&mut self, session: Uuid) -> Option<(u16, u16)> {
|
||||
let clients = self.clients.get(&session)?;
|
||||
let mut cols = u16::MAX;
|
||||
let mut rows = u16::MAX;
|
||||
for &(c, r) in clients.values() {
|
||||
if c == 0 || r == 0 {
|
||||
continue;
|
||||
}
|
||||
cols = cols.min(c);
|
||||
rows = rows.min(r);
|
||||
}
|
||||
if cols == u16::MAX || rows == u16::MAX {
|
||||
return None;
|
||||
}
|
||||
let target = (cols, rows);
|
||||
if self.applied.get(&session) == Some(&target) {
|
||||
return None;
|
||||
}
|
||||
self.applied.insert(session, target);
|
||||
Some(target)
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn the_first_report_settles_the_window() {
|
||||
let mut v = Viewports::new();
|
||||
let s = Uuid::new_v4();
|
||||
assert_eq!(v.set(s, 1, 80, 24), Some((80, 24)));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_report_that_changes_nothing_costs_no_resize() {
|
||||
let mut v = Viewports::new();
|
||||
let s = Uuid::new_v4();
|
||||
v.set(s, 1, 80, 24);
|
||||
assert_eq!(v.set(s, 1, 80, 24), None);
|
||||
assert_eq!(v.set(s, 1, 80, 24), None);
|
||||
}
|
||||
|
||||
/// The bug this module exists for: two clients of different sizes used to
|
||||
/// resize the window past each other on every report.
|
||||
#[test]
|
||||
fn two_clients_settle_on_the_smaller_instead_of_fighting() {
|
||||
let mut v = Viewports::new();
|
||||
let s = Uuid::new_v4();
|
||||
assert_eq!(v.set(s, 1, 53, 46), Some((53, 46)));
|
||||
assert_eq!(v.set(s, 2, 53, 20), Some((53, 20)));
|
||||
// Whatever order they re-report in, the answer stays the smaller one.
|
||||
assert_eq!(v.set(s, 1, 53, 46), None);
|
||||
assert_eq!(v.set(s, 2, 53, 20), None);
|
||||
assert_eq!(v.set(s, 1, 53, 46), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn each_axis_is_arbitrated_on_its_own() {
|
||||
let mut v = Viewports::new();
|
||||
let s = Uuid::new_v4();
|
||||
v.set(s, 1, 200, 20);
|
||||
assert_eq!(v.set(s, 2, 53, 46), Some((53, 20)));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_zero_sized_client_does_not_shrink_the_window() {
|
||||
let mut v = Viewports::new();
|
||||
let s = Uuid::new_v4();
|
||||
assert_eq!(v.set(s, 1, 80, 24), Some((80, 24)));
|
||||
assert_eq!(v.set(s, 2, 0, 0), None);
|
||||
assert_eq!(v.set(s, 2, 0, 30), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn releasing_the_small_client_gives_the_window_back() {
|
||||
let mut v = Viewports::new();
|
||||
let s = Uuid::new_v4();
|
||||
v.set(s, 1, 80, 46);
|
||||
v.set(s, 2, 80, 20);
|
||||
assert_eq!(v.release(s, 2), Some((80, 46)));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn releasing_the_last_client_settles_nothing() {
|
||||
let mut v = Viewports::new();
|
||||
let s = Uuid::new_v4();
|
||||
v.set(s, 1, 80, 24);
|
||||
assert_eq!(v.release(s, 1), None);
|
||||
// ...and the session is forgotten, so a fresh attach settles again.
|
||||
assert_eq!(v.set(s, 2, 80, 24), Some((80, 24)));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn releasing_an_unknown_viewport_is_harmless() {
|
||||
let mut v = Viewports::new();
|
||||
let s = Uuid::new_v4();
|
||||
assert_eq!(v.release(s, 7), None);
|
||||
v.set(s, 1, 80, 24);
|
||||
assert_eq!(v.release(s, 7), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sessions_do_not_arbitrate_against_each_other() {
|
||||
let mut v = Viewports::new();
|
||||
let a = Uuid::new_v4();
|
||||
let b = Uuid::new_v4();
|
||||
assert_eq!(v.set(a, 1, 80, 24), Some((80, 24)));
|
||||
assert_eq!(v.set(b, 2, 53, 20), Some((53, 20)));
|
||||
assert_eq!(v.set(a, 1, 80, 24), None);
|
||||
}
|
||||
}
|
||||
@@ -18,6 +18,7 @@ use uuid::Uuid;
|
||||
|
||||
struct Fixture {
|
||||
manager: SessionManager,
|
||||
sidecar: SidecarStore,
|
||||
_tmux: TmuxFixture,
|
||||
_state_dir: TempDir,
|
||||
}
|
||||
@@ -25,15 +26,17 @@ struct Fixture {
|
||||
async fn fixture(ttl: Duration) -> Fixture {
|
||||
let tmux = TmuxFixture::new();
|
||||
let state_dir = TempDir::new().unwrap();
|
||||
let sidecar = SidecarStore::new(state_dir.path().join("sessions"));
|
||||
let manager = SessionManager::new(
|
||||
Arc::new(tmux.tmux.clone()),
|
||||
SidecarStore::new(state_dir.path().join("sessions")),
|
||||
sidecar.clone(),
|
||||
WorktreeManager::new(None),
|
||||
state_dir.path().join("fifos"),
|
||||
)
|
||||
.with_activity_report_ttl(ttl);
|
||||
Fixture {
|
||||
manager,
|
||||
sidecar,
|
||||
_tmux: tmux,
|
||||
_state_dir: state_dir,
|
||||
}
|
||||
@@ -67,7 +70,7 @@ async fn report_on_unknown_session_is_not_found() {
|
||||
let fx = fixture(Duration::from_secs(300)).await;
|
||||
let err = fx
|
||||
.manager
|
||||
.report_activity(Uuid::new_v4(), ActivityState::Idle, None)
|
||||
.report_activity(Uuid::new_v4(), ActivityState::Idle, None, None)
|
||||
.await
|
||||
.unwrap_err();
|
||||
assert!(matches!(err, maestro_daemon::SessionError::NotFound(_)));
|
||||
@@ -83,6 +86,7 @@ async fn report_sets_error_state_and_detail() {
|
||||
id,
|
||||
ActivityState::Error,
|
||||
Some("cargo build failed: link error".into()),
|
||||
None,
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
@@ -101,7 +105,7 @@ async fn replacing_a_report_replaces_both_state_and_detail() {
|
||||
let id = spawn_sleeping(&fx.manager).await;
|
||||
|
||||
fx.manager
|
||||
.report_activity(id, ActivityState::Error, Some("first failure".into()))
|
||||
.report_activity(id, ActivityState::Error, Some("first failure".into()), None)
|
||||
.await
|
||||
.unwrap();
|
||||
fx.manager
|
||||
@@ -109,6 +113,7 @@ async fn replacing_a_report_replaces_both_state_and_detail() {
|
||||
id,
|
||||
ActivityState::AwaitingInput,
|
||||
Some("needs approval".into()),
|
||||
None,
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
@@ -125,7 +130,7 @@ async fn detail_is_capped_server_side() {
|
||||
|
||||
let long = "x".repeat(10_000);
|
||||
fx.manager
|
||||
.report_activity(id, ActivityState::Error, Some(long))
|
||||
.report_activity(id, ActivityState::Error, Some(long), None)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
@@ -144,7 +149,12 @@ async fn reported_state_survives_watcher_ticks_until_the_ttl_then_inference_resu
|
||||
fx.manager.start_liveness_watcher(Duration::from_millis(50));
|
||||
|
||||
fx.manager
|
||||
.report_activity(id, ActivityState::Error, Some("agent died mid-run".into()))
|
||||
.report_activity(
|
||||
id,
|
||||
ActivityState::Error,
|
||||
Some("agent died mid-run".into()),
|
||||
None,
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
@@ -162,3 +172,55 @@ async fn reported_state_survives_watcher_ticks_until_the_ttl_then_inference_resu
|
||||
})
|
||||
.await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn a_reported_session_id_is_recorded_on_the_session() {
|
||||
let fx = fixture(Duration::from_secs(300)).await;
|
||||
let id = spawn_sleeping(&fx.manager).await;
|
||||
|
||||
fx.manager
|
||||
.report_activity(id, ActivityState::Active, None, Some("conv-abc123".into()))
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let s = observed(&fx.manager, id).await;
|
||||
assert_eq!(s.agent_session_id.as_deref(), Some("conv-abc123"));
|
||||
}
|
||||
|
||||
/// #161: a session id does not change mid-session, so once set it is never
|
||||
/// overwritten by a later report — even one that names a different id.
|
||||
#[tokio::test]
|
||||
async fn a_reported_session_id_is_written_once_and_never_replaced() {
|
||||
let fx = fixture(Duration::from_secs(300)).await;
|
||||
let id = spawn_sleeping(&fx.manager).await;
|
||||
|
||||
fx.manager
|
||||
.report_activity(id, ActivityState::Active, None, Some("conv-first".into()))
|
||||
.await
|
||||
.unwrap();
|
||||
fx.manager
|
||||
.report_activity(id, ActivityState::Active, None, Some("conv-second".into()))
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let s = observed(&fx.manager, id).await;
|
||||
assert_eq!(s.agent_session_id.as_deref(), Some("conv-first"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn a_reported_session_id_survives_recovery_from_the_sidecar() {
|
||||
let fx = fixture(Duration::from_secs(300)).await;
|
||||
let id = spawn_sleeping(&fx.manager).await;
|
||||
|
||||
fx.manager
|
||||
.report_activity(id, ActivityState::Active, None, Some("conv-abc123".into()))
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let stored = fx
|
||||
.sidecar
|
||||
.read(id)
|
||||
.expect("sidecar read")
|
||||
.expect("sidecar present");
|
||||
assert_eq!(stored.agent_session_id.as_deref(), Some("conv-abc123"));
|
||||
}
|
||||
|
||||
@@ -526,6 +526,33 @@ async fn post_activity_reports_error_with_detail() {
|
||||
assert_eq!(s.activity_detail.as_deref(), Some("cargo build failed"));
|
||||
}
|
||||
|
||||
/// #161: a reported `session_id` lands on the session as `agent_session_id`.
|
||||
#[tokio::test]
|
||||
async fn post_activity_records_the_reported_session_id() {
|
||||
let h = spawn("naps-laptop").await;
|
||||
let id = spawn_via_manager(&h).await;
|
||||
|
||||
let resp = reqwest::Client::new()
|
||||
.post(url(h.addr, &format!("/api/sessions/{id}/activity")))
|
||||
.json(&serde_json::json!({
|
||||
"state": "Active",
|
||||
"session_id": "conv-abc123"
|
||||
}))
|
||||
.send()
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(resp.status(), reqwest::StatusCode::NO_CONTENT);
|
||||
|
||||
let sessions: Vec<Session> = reqwest::get(url(h.addr, "/api/sessions"))
|
||||
.await
|
||||
.unwrap()
|
||||
.json()
|
||||
.await
|
||||
.unwrap();
|
||||
let s = sessions.iter().find(|s| s.id == id).unwrap();
|
||||
assert_eq!(s.agent_session_id.as_deref(), Some("conv-abc123"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn post_activity_to_unknown_session_returns_404() {
|
||||
let h = spawn("naps-laptop").await;
|
||||
|
||||
@@ -555,3 +555,63 @@ async fn spawn_overrides_client_supplied_maestro_variables() {
|
||||
Some(&"http://127.0.0.1:7070".to_string())
|
||||
);
|
||||
}
|
||||
|
||||
/// Two clients attached to one session used to resize the shared window past
|
||||
/// each other on every report they made. Every flip repaints a full-screen
|
||||
/// agent at a new height and orphans the rows it drew at the old one, which
|
||||
/// reads to the operator as a pane that has stopped taking input.
|
||||
#[tokio::test]
|
||||
async fn clients_of_different_sizes_do_not_resize_the_window_past_each_other() {
|
||||
let fx = Fx::new();
|
||||
let s = fx.manager.create(fx.request("bash")).await.expect("create");
|
||||
let web = fx.manager.attach_viewport();
|
||||
let tui = fx.manager.attach_viewport();
|
||||
|
||||
fx.manager.set_viewport(s.id, web, 53, 46).await.unwrap();
|
||||
fx.manager.set_viewport(s.id, tui, 53, 20).await.unwrap();
|
||||
// Both keep reporting, as a browser's ResizeObserver and the TUI's own
|
||||
// redraw loop do. Neither report changes the arbitrated size.
|
||||
for _ in 0..5 {
|
||||
fx.manager.set_viewport(s.id, web, 53, 46).await.unwrap();
|
||||
fx.manager.set_viewport(s.id, tui, 53, 20).await.unwrap();
|
||||
}
|
||||
|
||||
let resizes: Vec<_> = fx
|
||||
.fake
|
||||
.calls()
|
||||
.into_iter()
|
||||
.filter_map(|c| match c {
|
||||
Call::ResizeWindow { cols, rows, .. } => Some((cols, rows)),
|
||||
_ => None,
|
||||
})
|
||||
.collect();
|
||||
assert_eq!(
|
||||
resizes,
|
||||
vec![(53, 46), (53, 20)],
|
||||
"the window was resized more than the arbitrated size actually changed"
|
||||
);
|
||||
}
|
||||
|
||||
/// The smaller client leaving hands the window back to the one still attached.
|
||||
#[tokio::test]
|
||||
async fn releasing_a_client_resizes_to_what_is_left() {
|
||||
let fx = Fx::new();
|
||||
let s = fx.manager.create(fx.request("bash")).await.expect("create");
|
||||
let web = fx.manager.attach_viewport();
|
||||
let tui = fx.manager.attach_viewport();
|
||||
fx.manager.set_viewport(s.id, web, 80, 46).await.unwrap();
|
||||
fx.manager.set_viewport(s.id, tui, 80, 20).await.unwrap();
|
||||
|
||||
fx.manager.release_viewport(s.id, tui).await;
|
||||
|
||||
let resizes: Vec<_> = fx
|
||||
.fake
|
||||
.calls()
|
||||
.into_iter()
|
||||
.filter_map(|c| match c {
|
||||
Call::ResizeWindow { cols, rows, .. } => Some((cols, rows)),
|
||||
_ => None,
|
||||
})
|
||||
.collect();
|
||||
assert_eq!(resizes, vec![(80, 46), (80, 20), (80, 46)]);
|
||||
}
|
||||
|
||||
@@ -32,6 +32,7 @@ fn sample(id: Uuid) -> Session {
|
||||
current_cwd: Some(PathBuf::from("/tmp/work/crates")),
|
||||
blocked_work: None,
|
||||
last_focused_at: None,
|
||||
agent_session_id: Some("conv-abc123".into()),
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -84,15 +84,23 @@ if [ "$state" = "Prompt" ]; then
|
||||
>/dev/null 2>&1 || true
|
||||
exit 0
|
||||
fi
|
||||
body="{\"state\":\"$state\"}"
|
||||
fields="\"state\":\"$state\""
|
||||
# StopFailure payloads carry the outcome in "error". Only a plain enum
|
||||
# spelling is reported as the detail — anything else (quotes, backslashes,
|
||||
# prose) would risk breaking the JSON body, and the spelling already names
|
||||
# the failure.
|
||||
detail=$(printf '%s' "$payload" | sed -n 's/.*"error"[[:space:]]*:[[:space:]]*"\([a-z_]*\)".*/\1/p')
|
||||
if [ -n "$detail" ]; then
|
||||
body="{\"state\":\"$state\",\"detail\":\"$detail\"}"
|
||||
fields="$fields,\"detail\":\"$detail\""
|
||||
fi
|
||||
# Every Claude Code hook payload carries its own conversation id (#161).
|
||||
# Constrained to a safe charset for the same reason as "detail" above: it is
|
||||
# spliced into the JSON body unescaped.
|
||||
session_id=$(printf '%s' "$payload" | sed -n 's/.*"session_id"[[:space:]]*:[[:space:]]*"\([A-Za-z0-9_-]*\)".*/\1/p')
|
||||
if [ -n "$session_id" ]; then
|
||||
fields="$fields,\"session_id\":\"$session_id\""
|
||||
fi
|
||||
body="{$fields}"
|
||||
curl -fsS --connect-timeout 1 --max-time 2 \
|
||||
-H 'Content-Type: application/json' \
|
||||
-d "$body" \
|
||||
|
||||
@@ -128,6 +128,41 @@ fn script_reports_the_error_spelling_as_detail() {
|
||||
assert!(request.contains(r#""detail":"rate_limit""#), "{request}");
|
||||
}
|
||||
|
||||
/// #161: the payload's own `session_id` — the agent's conversation id — is
|
||||
/// parsed out and forwarded in the activity body, so the daemon can persist
|
||||
/// it on the session.
|
||||
#[test]
|
||||
fn script_forwards_the_payloads_session_id_in_the_activity_body() {
|
||||
let request = capture_request(|url| {
|
||||
sh(
|
||||
HOOK_SCRIPT,
|
||||
&["Active"],
|
||||
&owned_envs(&session_envs(&url)),
|
||||
br#"{"session_id":"conv-abc123","hook_event_name":"PreToolUse"}"#,
|
||||
)
|
||||
});
|
||||
assert!(request.contains(r#""state":"Active""#), "{request}");
|
||||
assert!(
|
||||
request.contains(r#""session_id":"conv-abc123""#),
|
||||
"{request}"
|
||||
);
|
||||
}
|
||||
|
||||
/// A payload without a `session_id` (or one with an unexpected shape) must
|
||||
/// never produce a broken body — no `session_id` field is posted.
|
||||
#[test]
|
||||
fn script_posts_no_session_id_when_the_payload_has_none() {
|
||||
let request = capture_request(|url| {
|
||||
sh(
|
||||
HOOK_SCRIPT,
|
||||
&["AwaitingInput"],
|
||||
&owned_envs(&session_envs(&url)),
|
||||
br#"{"hook_event_name":"Stop","stop_hook_active":false}"#,
|
||||
)
|
||||
});
|
||||
assert!(!request.contains("session_id"), "{request}");
|
||||
}
|
||||
|
||||
/// An `error` value that is not a plain enum spelling (or a payload without
|
||||
/// one at all) must never produce a broken body — no detail is posted.
|
||||
#[test]
|
||||
|
||||
@@ -284,6 +284,7 @@ mod tests {
|
||||
current_cwd: None,
|
||||
blocked_work: None,
|
||||
last_focused_at: None,
|
||||
agent_session_id: None,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -116,6 +116,13 @@ pub struct Session {
|
||||
/// sorts a never-focused session after every session that has one.
|
||||
#[serde(default)]
|
||||
pub last_focused_at: Option<DateTime<Utc>>,
|
||||
/// The conversation id the agent running in this session reported for
|
||||
/// itself (#161), parsed from a hook's `session_id` field. Written once,
|
||||
/// from the first activity report that carries one, and left alone after
|
||||
/// — an agent's own session id does not change mid-session. `None` for
|
||||
/// sessions that never run an agent (plain shells) or predate the field.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub agent_session_id: Option<String>,
|
||||
}
|
||||
|
||||
/// Default for [`Session::last_activity_at`] when an older payload omits it.
|
||||
@@ -184,6 +191,11 @@ pub struct ReportActivityRequest {
|
||||
/// The daemon caps its length server-side.
|
||||
#[serde(default)]
|
||||
pub detail: Option<String>,
|
||||
/// The agent's own conversation id (#161), forwarded from the hook
|
||||
/// payload's `session_id` field. `None` for hook events that don't carry
|
||||
/// one, or hooks predating this field.
|
||||
#[serde(default)]
|
||||
pub session_id: Option<String>,
|
||||
}
|
||||
|
||||
/// Reject an unknown [`ActivityState`] spelling instead of degrading it to
|
||||
@@ -862,6 +874,7 @@ mod tests {
|
||||
current_cwd: Some(PathBuf::from("/home/naps62/projects/maestro/crates")),
|
||||
blocked_work: None,
|
||||
last_focused_at: None,
|
||||
agent_session_id: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1031,6 +1044,7 @@ mod tests {
|
||||
let full = ReportActivityRequest {
|
||||
state: ActivityState::AwaitingInput,
|
||||
detail: Some("waiting on permission".into()),
|
||||
session_id: Some("conv-abc123".into()),
|
||||
};
|
||||
assert_eq!(full, round_trip(&full));
|
||||
|
||||
|
||||
@@ -47,6 +47,7 @@ pub fn session(id: Uuid, cwd: &str, command: &str) -> Session {
|
||||
current_cwd: None,
|
||||
blocked_work: None,
|
||||
last_focused_at: None,
|
||||
agent_session_id: None,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -1898,6 +1898,7 @@ mod tests {
|
||||
current_cwd: None,
|
||||
blocked_work: None,
|
||||
last_focused_at: None,
|
||||
agent_session_id: None,
|
||||
})
|
||||
.collect(),
|
||||
});
|
||||
@@ -2079,6 +2080,7 @@ mod tests {
|
||||
current_cwd: None,
|
||||
blocked_work: None,
|
||||
last_focused_at: None,
|
||||
agent_session_id: None,
|
||||
}],
|
||||
});
|
||||
app.dispatch(Action::SidebarNext);
|
||||
@@ -2106,6 +2108,7 @@ mod tests {
|
||||
current_cwd: None,
|
||||
blocked_work: None,
|
||||
last_focused_at: None,
|
||||
agent_session_id: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -3214,6 +3217,7 @@ mod tests {
|
||||
current_cwd: None,
|
||||
blocked_work: None,
|
||||
last_focused_at: focused,
|
||||
agent_session_id: None,
|
||||
};
|
||||
let older = Utc::now() - chrono::Duration::seconds(60);
|
||||
app.catalog.apply_event(CatalogEvent::Snapshot {
|
||||
@@ -3492,6 +3496,7 @@ mod tests {
|
||||
current_cwd: None,
|
||||
blocked_work: None,
|
||||
last_focused_at: None,
|
||||
agent_session_id: None,
|
||||
}],
|
||||
});
|
||||
app.dispatch(Action::KillHighlighted);
|
||||
@@ -3527,6 +3532,7 @@ mod tests {
|
||||
current_cwd: None,
|
||||
blocked_work: None,
|
||||
last_focused_at: None,
|
||||
agent_session_id: None,
|
||||
}],
|
||||
});
|
||||
}
|
||||
|
||||
@@ -223,6 +223,7 @@ mod tests {
|
||||
current_cwd: None,
|
||||
blocked_work: None,
|
||||
last_focused_at: None,
|
||||
agent_session_id: None,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -809,6 +809,7 @@ mod tests {
|
||||
current_cwd: None,
|
||||
blocked_work: None,
|
||||
last_focused_at: None,
|
||||
agent_session_id: None,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -500,6 +500,7 @@ mod tests {
|
||||
current_cwd: None,
|
||||
blocked_work: None,
|
||||
last_focused_at: None,
|
||||
agent_session_id: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1266,6 +1267,7 @@ mod tests {
|
||||
current_cwd: None,
|
||||
blocked_work: None,
|
||||
last_focused_at: None,
|
||||
agent_session_id: None,
|
||||
};
|
||||
let sb = Sidebar {
|
||||
sessions: vec![s],
|
||||
|
||||
+6
-6
@@ -158,12 +158,12 @@ searchable — the query line keeps working, now over those actions.
|
||||
ambiguous.
|
||||
- Escape, or backspace at an empty query, returns to the result list **with the
|
||||
previous query intact**. Drilling in and backing out is free.
|
||||
- Each action raises the flow it already has: kill goes through the existing
|
||||
confirm-and-blockers dialog (`docs/WORKTREES.md`), rename through the
|
||||
existing prompt pre-filled with the session's own name
|
||||
(`docs/SIDEBAR.md`), spawn similar through the spawn form seeded from the
|
||||
session's directory and command. The bar picks the action; it never
|
||||
reimplements one.
|
||||
- Each action raises the flow it already has: kill through the confirmation
|
||||
both clients ask for (`docs/SIDEBAR.md`) and the blockers dialog behind it
|
||||
(`docs/WORKTREES.md`), rename through the existing prompt pre-filled with the
|
||||
session's own name (`docs/SIDEBAR.md`), spawn similar through the spawn form
|
||||
seeded from the session's directory and command. The bar picks the action; it
|
||||
never reimplements one.
|
||||
|
||||
No actions beyond those three, and no action across more than one session.
|
||||
|
||||
|
||||
+16
-1
@@ -316,7 +316,7 @@ Everything else is the same key on both clients:
|
||||
| `Tab` | hand the keyboard to the pane |
|
||||
| `c` | collapse or expand the highlighted folder |
|
||||
| `r` | rename the highlighted session |
|
||||
| `x` | kill the highlighted session |
|
||||
| `x` | kill the highlighted session (confirms) |
|
||||
| `n` | open the spawn form |
|
||||
| `N` | spawn with every default resolved |
|
||||
| `?` | show the shortcut overlay |
|
||||
@@ -324,3 +324,18 @@ Everything else is the same key on both clients:
|
||||
The TUI binds these through its keymap, so a user can rebind any of them; the
|
||||
web client's are fixed. A binding added to one client belongs in this table and
|
||||
in both help overlays.
|
||||
|
||||
## Killing a session
|
||||
|
||||
**Every kill confirms first, in both clients** (#160). A kill is destructive
|
||||
and unattended agents live in these sessions, so neither client sends
|
||||
`DELETE /api/sessions/:id` off a single press. The TUI raises
|
||||
`OverlayKind::KillConfirm` (`[y]`/`[n]`); the web raises the same dialog its
|
||||
`✕` button and the command bar's action page both use. Cancel holds the focus
|
||||
in the web's dialog, so nothing kills on a stray Enter.
|
||||
|
||||
The confirmation names what the card last saw in the way, when anything was —
|
||||
the daemon will refuse an unforced kill over such a worktree, and saying so
|
||||
before the attempt is cheaper than a refusal. What comes back from that refusal
|
||||
is a second, separate prompt offering `force=true`; `docs/WORKTREES.md` owns
|
||||
what a forced kill leaves behind.
|
||||
|
||||
@@ -65,7 +65,7 @@ async function flush(): Promise<void> {
|
||||
}
|
||||
|
||||
describe("kill from the action page", () => {
|
||||
it("goes through the daemon, without a first confirmation", async () => {
|
||||
it("goes through the same confirmation the sidebar raises (#160)", async () => {
|
||||
const calls: string[] = [];
|
||||
window.fetch = ((url: string, init?: RequestInit) => {
|
||||
calls.push(`${init?.method ?? "GET"} ${url}`);
|
||||
@@ -75,6 +75,12 @@ describe("kill from the action page", () => {
|
||||
mount(s);
|
||||
raise(KILL_SESSION_EVENT, s);
|
||||
await flush();
|
||||
expect(document.querySelector(".kill-dialog")?.getAttribute("aria-label")).toBe(
|
||||
"Kill amber-falcon",
|
||||
);
|
||||
expect(calls.filter((c) => c.startsWith("DELETE"))).toHaveLength(0);
|
||||
document.querySelector<HTMLButtonElement>(".kill-dialog-kill")?.click();
|
||||
await flush();
|
||||
expect(calls).toContain(`DELETE /api/sessions/${s.id}`);
|
||||
expect(document.querySelector(".kill-dialog")).toBeNull();
|
||||
});
|
||||
@@ -101,6 +107,8 @@ describe("kill from the action page", () => {
|
||||
mount(s);
|
||||
raise(KILL_SESSION_EVENT, s);
|
||||
await flush();
|
||||
document.querySelector<HTMLButtonElement>(".kill-dialog-kill")?.click();
|
||||
await flush();
|
||||
const dialog = document.querySelector(".kill-dialog");
|
||||
expect(dialog?.getAttribute("aria-label")).toBe("Kill amber-falcon");
|
||||
expect(dialog?.textContent).toContain("1 untracked file");
|
||||
|
||||
+40
-1
@@ -98,7 +98,7 @@ describe("blocked-work indicator (#83)", () => {
|
||||
expect(rowFor(el, unprobed.id)?.querySelector(".leaf-blocked")).toBeNull();
|
||||
});
|
||||
|
||||
it("kills straight away, without a first confirmation", async () => {
|
||||
it("confirms before an unguarded kill (#160)", async () => {
|
||||
const calls: string[] = [];
|
||||
window.fetch = ((url: string, init?: RequestInit) => {
|
||||
calls.push(`${init?.method ?? "GET"} ${url}`);
|
||||
@@ -108,10 +108,45 @@ describe("blocked-work indicator (#83)", () => {
|
||||
const el = renderSidebar(storeWith([plain])).el;
|
||||
rowFor(el, plain.id)?.querySelector<HTMLButtonElement>(".leaf-kill")?.click();
|
||||
await flush();
|
||||
const dialog = document.querySelector(".kill-dialog");
|
||||
expect(dialog?.getAttribute("aria-label")).toBe("Kill claude");
|
||||
expect(calls.filter((c) => c.startsWith("DELETE"))).toHaveLength(0);
|
||||
dialog?.querySelector<HTMLButtonElement>(".kill-dialog-kill")?.click();
|
||||
await flush();
|
||||
expect(calls).toContain(`DELETE /api/sessions/${plain.id}`);
|
||||
expect(document.querySelector(".kill-dialog")).toBeNull();
|
||||
});
|
||||
|
||||
it("kills nothing when the confirmation is cancelled (#160)", async () => {
|
||||
const calls: string[] = [];
|
||||
window.fetch = ((url: string, init?: RequestInit) => {
|
||||
calls.push(`${init?.method ?? "GET"} ${url}`);
|
||||
return Promise.resolve(new Response(null, { status: 204 }));
|
||||
}) as typeof fetch;
|
||||
const plain = session({ id: "aaaaaaaa-0000-0000-0000-000000000007" });
|
||||
const el = renderSidebar(storeWith([plain])).el;
|
||||
rowFor(el, plain.id)?.querySelector<HTMLButtonElement>(".leaf-kill")?.click();
|
||||
await flush();
|
||||
document.querySelector<HTMLButtonElement>(".kill-dialog-cancel")?.click();
|
||||
await flush();
|
||||
expect(document.querySelector(".kill-dialog")).toBeNull();
|
||||
expect(calls.filter((c) => c.startsWith("DELETE"))).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("names the blockers in the confirmation, before the daemon refuses (#160)", async () => {
|
||||
window.fetch = (() => Promise.reject(new Error("never called"))) as typeof fetch;
|
||||
const blocked = session({
|
||||
id: "aaaaaaaa-0000-0000-0000-000000000008",
|
||||
blocked_work: { untracked_files: 1 },
|
||||
});
|
||||
const el = renderSidebar(storeWith([blocked])).el;
|
||||
rowFor(el, blocked.id)?.querySelector<HTMLButtonElement>(".leaf-kill")?.click();
|
||||
await flush();
|
||||
const dialog = document.querySelector(".kill-dialog");
|
||||
expect(dialog?.textContent).toContain("1 untracked file");
|
||||
expect(dialog?.textContent).toContain("refused unless it is forced");
|
||||
});
|
||||
|
||||
it("offers the override when the daemon refuses a guarded kill", async () => {
|
||||
const calls: string[] = [];
|
||||
window.fetch = ((url: string, init?: RequestInit) => {
|
||||
@@ -138,6 +173,8 @@ describe("blocked-work indicator (#83)", () => {
|
||||
const el = renderSidebar(storeWith([blocked])).el;
|
||||
rowFor(el, blocked.id)?.querySelector<HTMLButtonElement>(".leaf-kill")?.click();
|
||||
await flush();
|
||||
document.querySelector<HTMLButtonElement>(".kill-dialog-kill")?.click();
|
||||
await flush();
|
||||
const dialog = document.querySelector(".kill-dialog");
|
||||
expect(dialog?.textContent).toContain("1 untracked file");
|
||||
dialog?.querySelector<HTMLButtonElement>(".kill-dialog-force")?.click();
|
||||
@@ -166,6 +203,8 @@ describe("blocked-work indicator (#83)", () => {
|
||||
const el = renderSidebar(storeWith([blocked])).el;
|
||||
rowFor(el, blocked.id)?.querySelector<HTMLButtonElement>(".leaf-kill")?.click();
|
||||
await flush();
|
||||
document.querySelector<HTMLButtonElement>(".kill-dialog-kill")?.click();
|
||||
await flush();
|
||||
document.querySelector<HTMLButtonElement>(".kill-dialog-cancel")?.click();
|
||||
expect(document.querySelector(".kill-dialog")).toBeNull();
|
||||
expect(calls.filter((c) => c.includes("force=true"))).toHaveLength(0);
|
||||
|
||||
+31
-24
@@ -1,6 +1,6 @@
|
||||
import { escapeHtml } from "./html.js";
|
||||
import { ApiRequestError, getDaemonInfo, killSession, renameSession } from "./api.js";
|
||||
import { openForceKillDialog } from "./killDialog.js";
|
||||
import { openForceKillDialog, openKillConfirmDialog } from "./killDialog.js";
|
||||
import { openContextMenu } from "./contextMenu.js";
|
||||
import { toggleHelp } from "./help.js";
|
||||
import { CommandSet, defineCommands } from "./commands.js";
|
||||
@@ -230,7 +230,7 @@ function renderNode(node: TreeNode, depth: number): HTMLElement {
|
||||
killBtn.addEventListener("click", (ev) => {
|
||||
ev.preventDefault();
|
||||
ev.stopPropagation();
|
||||
void requestKill(cur, sessionName(cur));
|
||||
requestKill(cur, sessionName(cur));
|
||||
});
|
||||
row.addEventListener("click", () => {
|
||||
document.dispatchEvent(new CustomEvent(SESSION_SELECT_EVENT, { detail: cur.id }));
|
||||
@@ -248,7 +248,7 @@ function renderNode(node: TreeNode, depth: number): HTMLElement {
|
||||
{
|
||||
label: "Kill session",
|
||||
danger: true,
|
||||
onSelect: () => void requestKill(cur, sessionName(cur)),
|
||||
onSelect: () => requestKill(cur, sessionName(cur)),
|
||||
},
|
||||
]);
|
||||
});
|
||||
@@ -293,26 +293,33 @@ function treeSignature(node: TreeNode): string {
|
||||
}
|
||||
|
||||
/**
|
||||
* Kill on the first click (#81): the daemon is the one that decides whether a
|
||||
* session may go, and it refuses over a worktree holding unsaved work. Only
|
||||
* that refusal is worth a prompt, and it needs one `confirm()` cannot give —
|
||||
* a forced kill. Anything else surfaces as an alert.
|
||||
* Kill behind a confirmation (#160): a kill is destructive and unattended
|
||||
* agents live in these sessions, so the web asks first, the way the TUI always
|
||||
* has. The daemon decides the rest — it refuses a kill over a worktree holding
|
||||
* unsaved work, and that refusal needs a prompt the confirmation cannot give,
|
||||
* a forced kill. Anything else surfaces in the dialog that asked.
|
||||
*/
|
||||
async function requestKill(s: Session, name: string): Promise<void> {
|
||||
try {
|
||||
await killSession(s.id);
|
||||
} catch (e) {
|
||||
if (e instanceof ApiRequestError && e.code === "kill_guarded") {
|
||||
openForceKillDialog({
|
||||
name,
|
||||
blockers: e.unsavedWork ?? sessionBlockers(s),
|
||||
reason: e.detail,
|
||||
onForce: () => killSession(s.id, true),
|
||||
});
|
||||
return;
|
||||
}
|
||||
alert(`Failed to kill: ${(e as Error).message}`);
|
||||
}
|
||||
function requestKill(s: Session, name: string): void {
|
||||
openKillConfirmDialog({
|
||||
name,
|
||||
blockers: sessionBlockers(s),
|
||||
onKill: async () => {
|
||||
try {
|
||||
await killSession(s.id);
|
||||
} catch (e) {
|
||||
if (e instanceof ApiRequestError && e.code === "kill_guarded") {
|
||||
openForceKillDialog({
|
||||
name,
|
||||
blockers: e.unsavedWork ?? sessionBlockers(s),
|
||||
reason: e.detail,
|
||||
onForce: () => killSession(s.id, true),
|
||||
});
|
||||
return;
|
||||
}
|
||||
throw e;
|
||||
}
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -719,7 +726,7 @@ export function renderSidebar(store: CatalogStore, opts: SidebarOptions = {}): S
|
||||
},
|
||||
"list.kill": () => {
|
||||
const hit = highlightedSession();
|
||||
if (hit) void requestKill(hit.session, sessionName(hit.session));
|
||||
if (hit) requestKill(hit.session, sessionName(hit.session));
|
||||
},
|
||||
"list.focusPane": () => document.dispatchEvent(new CustomEvent(FOCUS_PANE_EVENT)),
|
||||
}),
|
||||
@@ -782,7 +789,7 @@ export function renderSidebar(store: CatalogStore, opts: SidebarOptions = {}): S
|
||||
// than by row.
|
||||
const killHandler = (ev: Event) => {
|
||||
const s = (ev as CustomEvent<Session>).detail;
|
||||
void requestKill(s, sessionName(s));
|
||||
requestKill(s, sessionName(s));
|
||||
};
|
||||
const renameHandler = (ev: Event) => {
|
||||
const s = (ev as CustomEvent<Session>).detail;
|
||||
|
||||
+83
-22
@@ -1,14 +1,44 @@
|
||||
/**
|
||||
* Forced-kill prompt (#81).
|
||||
* The two kill prompts (#81, #160).
|
||||
*
|
||||
* The daemon refuses to kill a session whose worktree still holds uncommitted
|
||||
* or unpushed work. `confirm()` can only ask the same question twice, so the
|
||||
* refusal gets its own dialog: it names what is in the way and offers the
|
||||
* override the daemon's `force=true` already implements. A kill the daemon
|
||||
* accepts never reaches here — there is nothing to warn about.
|
||||
* A kill is destructive and unattended agents live in these sessions, so both
|
||||
* clients ask before one goes through — the TUI's `KillConfirm`, and
|
||||
* `openKillConfirmDialog` here (#160). Where the session's worktree still
|
||||
* holds uncommitted or unpushed work, the confirmation says so up front, the
|
||||
* same way the TUI's does.
|
||||
*
|
||||
* The daemon decides the rest: it refuses a kill over such a worktree, and
|
||||
* that refusal needs an answer the confirmation cannot give — a forced kill.
|
||||
* `openForceKillDialog` is that second prompt. A kill the daemon accepts never
|
||||
* reaches it.
|
||||
*/
|
||||
import { formatInTheWay, type InTheWay } from "./types.js";
|
||||
|
||||
/** What both prompts need: a target to name and a button that acts on it. */
|
||||
interface KillDialogSpec {
|
||||
/** Session name, as the sidebar shows it. */
|
||||
name: string;
|
||||
/** Sentence under the title. */
|
||||
body: string;
|
||||
/** Label on the acting button. */
|
||||
confirmLabel: string;
|
||||
/** Extra class on the acting button, so tests and styles can find it. */
|
||||
confirmClass: string;
|
||||
/** Run it. A rejection is rendered in the dialog, which stays open so the
|
||||
* press can be retried. */
|
||||
onConfirm: () => Promise<void>;
|
||||
}
|
||||
|
||||
export interface KillConfirmPrompt {
|
||||
name: string;
|
||||
/** What the card last saw in the way, when anything was. Shown before the
|
||||
* attempt, so "this will be refused" is not a surprise. */
|
||||
blockers: InTheWay | null;
|
||||
/** Run the plain kill. Opening a further dialog from here is allowed: the
|
||||
* confirmation only closes itself if it is still the one on screen. */
|
||||
onKill: () => Promise<void>;
|
||||
}
|
||||
|
||||
export interface ForceKillPrompt {
|
||||
/** Session name, as the sidebar shows it. */
|
||||
name: string;
|
||||
@@ -30,9 +60,9 @@ export function closeForceKillDialog(): void {
|
||||
open = null;
|
||||
}
|
||||
|
||||
/** Mount the prompt. Opening while one is up replaces it, so a second refusal
|
||||
* can never stack two dialogs. */
|
||||
export function openForceKillDialog(p: ForceKillPrompt): HTMLElement {
|
||||
/** Mount one of the prompts. Opening while one is up replaces it, so the
|
||||
* confirmation and a refusal that follows it can never stack. */
|
||||
function mount(p: KillDialogSpec): HTMLElement {
|
||||
closeForceKillDialog();
|
||||
|
||||
const el = document.createElement("div");
|
||||
@@ -50,9 +80,7 @@ export function openForceKillDialog(p: ForceKillPrompt): HTMLElement {
|
||||
|
||||
const body = document.createElement("p");
|
||||
body.className = "kill-dialog-body";
|
||||
body.textContent = p.blockers
|
||||
? `Its worktree still has ${formatInTheWay(p.blockers)}. Forcing the kill removes the session and leaves the worktree on disk.`
|
||||
: `${p.reason} Forcing the kill removes the session and leaves the worktree on disk.`;
|
||||
body.textContent = p.body;
|
||||
|
||||
const error = document.createElement("p");
|
||||
error.className = "kill-dialog-error";
|
||||
@@ -65,22 +93,27 @@ export function openForceKillDialog(p: ForceKillPrompt): HTMLElement {
|
||||
cancel.className = "btn btn-ghost kill-dialog-cancel";
|
||||
cancel.textContent = "Cancel";
|
||||
cancel.addEventListener("click", closeForceKillDialog);
|
||||
const force = document.createElement("button");
|
||||
force.type = "button";
|
||||
force.className = "btn btn-danger kill-dialog-force";
|
||||
force.textContent = "Force kill";
|
||||
force.addEventListener("click", () => {
|
||||
force.disabled = true;
|
||||
const confirm = document.createElement("button");
|
||||
confirm.type = "button";
|
||||
confirm.className = `btn btn-danger ${p.confirmClass}`;
|
||||
confirm.textContent = p.confirmLabel;
|
||||
confirm.addEventListener("click", () => {
|
||||
confirm.disabled = true;
|
||||
void p
|
||||
.onForce()
|
||||
.then(closeForceKillDialog)
|
||||
.onConfirm()
|
||||
.then(() => {
|
||||
// A handler that opened the refusal prompt has already replaced this
|
||||
// dialog; closing here would take that one down with it.
|
||||
if (open?.el === el) closeForceKillDialog();
|
||||
})
|
||||
.catch((e: unknown) => {
|
||||
force.disabled = false;
|
||||
if (open?.el !== el) return;
|
||||
confirm.disabled = false;
|
||||
error.hidden = false;
|
||||
error.textContent = `Failed to kill: ${(e as Error).message}`;
|
||||
});
|
||||
});
|
||||
actions.append(cancel, force);
|
||||
actions.append(cancel, confirm);
|
||||
|
||||
panel.append(title, body, error, actions);
|
||||
|
||||
@@ -99,6 +132,34 @@ export function openForceKillDialog(p: ForceKillPrompt): HTMLElement {
|
||||
};
|
||||
window.addEventListener("keydown", onKeydown, true);
|
||||
open = { el, onKeydown };
|
||||
// Cancel takes the focus in both prompts: a kill is never one keystroke
|
||||
// away from whatever asked for the dialog.
|
||||
cancel.focus();
|
||||
return el;
|
||||
}
|
||||
|
||||
/** Ask before a kill the daemon has not seen yet (#160). */
|
||||
export function openKillConfirmDialog(p: KillConfirmPrompt): HTMLElement {
|
||||
return mount({
|
||||
name: p.name,
|
||||
body: p.blockers
|
||||
? `Its worktree still has ${formatInTheWay(p.blockers)}. The kill will be refused unless it is forced.`
|
||||
: "The session and everything running in it go away. Its worktree stays on disk.",
|
||||
confirmLabel: "Kill",
|
||||
confirmClass: "kill-dialog-kill",
|
||||
onConfirm: p.onKill,
|
||||
});
|
||||
}
|
||||
|
||||
/** Offer the override after the daemon refuses a guarded kill (#81). */
|
||||
export function openForceKillDialog(p: ForceKillPrompt): HTMLElement {
|
||||
return mount({
|
||||
name: p.name,
|
||||
body: p.blockers
|
||||
? `Its worktree still has ${formatInTheWay(p.blockers)}. Forcing the kill removes the session and leaves the worktree on disk.`
|
||||
: `${p.reason} Forcing the kill removes the session and leaves the worktree on disk.`,
|
||||
confirmLabel: "Force kill",
|
||||
confirmClass: "kill-dialog-force",
|
||||
onConfirm: p.onForce,
|
||||
});
|
||||
}
|
||||
|
||||
@@ -143,7 +143,7 @@ describe("sidebar keyboard navigation", () => {
|
||||
field!.dispatchEvent(new KeyboardEvent("keydown", { key: "Escape", bubbles: true }));
|
||||
});
|
||||
|
||||
it("x kills the highlighted session", async () => {
|
||||
it("x kills the highlighted session, once confirmed (#160)", async () => {
|
||||
const { el } = twoFolders();
|
||||
const killed: string[] = [];
|
||||
window.fetch = ((input: RequestInfo | URL, init?: RequestInit) => {
|
||||
@@ -153,6 +153,8 @@ describe("sidebar keyboard navigation", () => {
|
||||
|
||||
press(tree(el), "j");
|
||||
press(tree(el), "x");
|
||||
expect(killed.some((r) => r.startsWith("DELETE"))).toBe(false);
|
||||
document.querySelector<HTMLButtonElement>(".kill-dialog-kill")?.click();
|
||||
await Promise.resolve();
|
||||
expect(killed.some((r) => r.startsWith("DELETE") && r.includes(A))).toBe(true);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user