From 59fb216090f4859b3814d946fd356596234fe038 Mon Sep 17 00:00:00 2001 From: Tim Date: Wed, 20 May 2026 13:01:01 +0200 Subject: [PATCH] fix(tui): defer visible fresh inbox delivery --- src/tmux.rs | 67 ++++++++++++++++++++++++++++++++- src/tui/app.rs | 49 ++++++++++++++++++++++++ src/use_cases.rs | 18 +++++++++ tests/supervisor_tmux_test.rs | 6 ++- tests/tmux_visibility_test.rs | 71 +++++++++++++++++++++++++++++++++++ tests/use_cases_test.rs | 59 +++++++++++++++++++++++++++++ 6 files changed, 268 insertions(+), 2 deletions(-) create mode 100644 tests/tmux_visibility_test.rs diff --git a/src/tmux.rs b/src/tmux.rs index e6ee7ec..1543486 100644 --- a/src/tmux.rs +++ b/src/tmux.rs @@ -369,6 +369,53 @@ impl Tmux { } } + fn required_window_id(target: &str) -> Result { + let output = Command::new("tmux") + .args(["display-message", "-p", "-t", target, "#{window_id}"]) + .output() + .context("failed to query tmux window id")?; + if !output.status.success() { + anyhow::bail!( + "failed to query tmux window id: {}", + String::from_utf8_lossy(&output.stderr) + ); + } + let id = String::from_utf8_lossy(&output.stdout).trim().to_string(); + if id.is_empty() { + anyhow::bail!("tmux target '{target}' did not report a window id"); + } + Ok(id) + } + + /// Return whether the target window is currently visible to any attached + /// tmux client. Linked windows share the same tmux `window_id`, so this + /// also treats task-session links to an agent window as visible. + pub fn is_window_visible_to_any_client( + session_name: &str, + window: Option<&str>, + ) -> Result { + let target = Self::tmux_target(session_name, window); + let target_window_id = Self::required_window_id(&target)?; + + let output = Command::new("tmux") + .args(["list-clients", "-F", "#{window_id}"]) + .output() + .context("failed to list tmux clients")?; + + if !output.status.success() { + anyhow::bail!( + "failed to list tmux clients: {}", + String::from_utf8_lossy(&output.stderr) + ); + } + + let client_window_ids = String::from_utf8_lossy(&output.stdout); + Ok(window_id_is_visible( + &target_window_id, + client_window_ids.lines(), + )) + } + fn window_exists(session_name: &str, window_name: &str) -> Result { let output = Command::new("tmux") .args(["list-windows", "-t", session_name, "-F", "#{window_name}"]) @@ -921,9 +968,20 @@ fn sanitize_tmux_window_tokens(raw: &str) -> Vec { tokens } +fn window_id_is_visible<'a>( + target_window_id: &str, + client_window_ids: impl IntoIterator, +) -> bool { + client_window_ids + .into_iter() + .any(|id| id.trim() == target_window_id) +} + #[cfg(test)] mod tests { - use super::{link_window_args, rename_window_args, unlink_window_args, Tmux}; + use super::{ + link_window_args, rename_window_args, unlink_window_args, window_id_is_visible, Tmux, + }; #[test] fn link_window_command_targets_canonical_agent_window() { @@ -1030,4 +1088,11 @@ mod tests { assert!(name.starts_with("Engineer-agent-")); assert!(name.len() <= 24); } + + #[test] + fn visible_window_id_matches_attached_client_window_ids() { + assert!(window_id_is_visible("@42", ["@1", "@42", "@9"])); + assert!(window_id_is_visible("@42", [" @42 "])); + assert!(!window_id_is_visible("@42", ["@1", "@9"])); + } } diff --git a/src/tui/app.rs b/src/tui/app.rs index 155b8e2..9273caf 100644 --- a/src/tui/app.rs +++ b/src/tui/app.rs @@ -4884,8 +4884,43 @@ impl App { } } + let visibility = match Tmux::is_window_visible_to_any_client( + &session_name, + window_ref, + ) { + Ok(visible) => Some(visible), + Err(e) => { + tracing::debug!( + target_name = &target, + session = &session_name, + error = %e, + "target visibility check failed; treating fresh messages as visible" + ); + None + } + }; + // Decision 5: already-pasted rescue (after readiness+buffer) let first_msg = &undelivered[0]; + if use_cases::should_defer_visible_fresh_inbox_message( + first_msg, + chrono::Utc::now(), + visibility, + ) { + tracing::debug!( + target_name = &target, + session = &session_name, + seq = first_msg.seq, + "target window visible and inbox message is fresh; deferring delivery" + ); + results.push(InboxPollResult { + target, + delivered: 0, + errors: vec![], + }); + continue; + } + let first_snippet = format!("[msg:{}:{}]", first_msg.from, first_msg.seq); let already_pasted = Tmux::capture_pane_window(&session_name, window_ref) .map(|content| content.contains(&first_snippet)) @@ -4922,6 +4957,20 @@ impl App { } 'msg_loop: for msg in &undelivered { + if use_cases::should_defer_visible_fresh_inbox_message( + msg, + chrono::Utc::now(), + visibility, + ) { + tracing::debug!( + target_name = &target, + session = &session_name, + seq = msg.seq, + "target window visible and inbox message is fresh; deferring delivery" + ); + break 'msg_loop; + } + let formatted_snippet = format!("[msg:{}:{}]", msg.from, msg.seq); for attempt in 0..MAX_RETRIES { diff --git a/src/use_cases.rs b/src/use_cases.rs index 2b91ff9..057a9f8 100644 --- a/src/use_cases.rs +++ b/src/use_cases.rs @@ -22,6 +22,7 @@ use crate::tmux::Tmux; /// it's resolved per-config via [`Config::default_harness`] and prepended /// dynamically by [`check_dependencies`]). const REQUIRED_TOOLS: &[&str] = &["tmux", "git", "nvim", "lazygit", "gh", "direnv"]; +pub const INBOX_VISIBLE_FRESH_DEFERRAL_SECS: i64 = 5 * 60; /// Check that all required external tools are present on $PATH. Includes /// the configured harness binary. Returns a list of @@ -4298,6 +4299,23 @@ pub fn stalled_targets_from_counts( .collect() } +/// Return true when an inbox message should be deferred because its target +/// window is visible and the message is still fresh enough that the user may +/// be manually typing in that chat. +pub fn should_defer_visible_fresh_inbox_message( + msg: &inbox::InboxMessage, + now: DateTime, + target_visible: Option, +) -> bool { + let age = now.signed_duration_since(msg.timestamp); + let is_fresh = age.num_seconds() < INBOX_VISIBLE_FRESH_DEFERRAL_SECS; + if !is_fresh { + return false; + } + + target_visible.unwrap_or(true) +} + // --------------------------------------------------------------------------- // Task query (for CLI commands) // --------------------------------------------------------------------------- diff --git a/tests/supervisor_tmux_test.rs b/tests/supervisor_tmux_test.rs index 739dadb..463b8a9 100644 --- a/tests/supervisor_tmux_test.rs +++ b/tests/supervisor_tmux_test.rs @@ -7,8 +7,11 @@ use agman::tmux::Tmux; use agman::use_cases; use helpers::{create_test_project, create_test_researcher, create_test_task, test_config}; use std::process::Command; +use std::sync::atomic::{AtomicU64, Ordering}; use std::time::{SystemTime, UNIX_EPOCH}; +static UNIQUE_COUNTER: AtomicU64 = AtomicU64::new(0); + struct TmuxCleanup { sessions: Vec, } @@ -326,7 +329,8 @@ fn unique_test_name() -> String { .duration_since(UNIX_EPOCH) .unwrap() .as_nanos(); - format!("{}-{nanos}", std::process::id()) + let counter = UNIQUE_COUNTER.fetch_add(1, Ordering::Relaxed); + format!("{}-{counter}-{nanos}", std::process::id()) } fn tmux_available() -> bool { diff --git a/tests/tmux_visibility_test.rs b/tests/tmux_visibility_test.rs new file mode 100644 index 0000000..039e43c --- /dev/null +++ b/tests/tmux_visibility_test.rs @@ -0,0 +1,71 @@ +use agman::tmux::Tmux; +use std::process::Command; +use std::time::{SystemTime, UNIX_EPOCH}; + +struct TmuxCleanup { + sessions: Vec, +} + +impl Drop for TmuxCleanup { + fn drop(&mut self) { + for session in &self.sessions { + let _ = Command::new("tmux") + .args(["kill-session", "-t", session]) + .output(); + } + } +} + +#[test] +fn detached_tmux_window_is_not_visible_to_any_client() { + if !tmux_available() { + eprintln!("skipping tmux visibility smoke: tmux binary is unavailable"); + return; + } + + let session = format!("agman-visibility-{}", unique_test_name()); + let tmp = tempfile::tempdir().unwrap(); + let _cleanup = TmuxCleanup { + sessions: vec![session.clone()], + }; + create_tmux_session(&session, tmp.path()); + + assert!(!Tmux::is_window_visible_to_any_client(&session, None).unwrap()); +} + +#[test] +fn missing_tmux_target_returns_visibility_error() { + if !tmux_available() { + eprintln!("skipping tmux visibility smoke: tmux binary is unavailable"); + return; + } + + assert!( + Tmux::is_window_visible_to_any_client("agman-visibility-missing-session", None).is_err() + ); +} + +fn unique_test_name() -> String { + let nanos = SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_nanos(); + format!("{}-{nanos}", std::process::id()) +} + +fn tmux_available() -> bool { + Command::new("tmux").arg("-V").output().is_ok() +} + +fn create_tmux_session(session: &str, cwd: &std::path::Path) { + let output = Command::new("tmux") + .args(["new-session", "-d", "-s", session, "-n", "main", "-c"]) + .arg(cwd) + .output() + .unwrap(); + assert!( + output.status.success(), + "failed to create tmux session {session}: {}", + String::from_utf8_lossy(&output.stderr) + ); +} diff --git a/tests/use_cases_test.rs b/tests/use_cases_test.rs index b801f56..3700b47 100644 --- a/tests/use_cases_test.rs +++ b/tests/use_cases_test.rs @@ -4,6 +4,7 @@ use agman::agent_model::{AgentAttachment, AgentKind, AgentRecord}; use agman::inbox; use agman::repo_stats::RepoStats; use agman::use_cases::{self, WorktreeSource}; +use chrono::{Duration, Utc}; use helpers::{ create_test_project, create_test_researcher, create_test_task, init_test_repo, test_config, }; @@ -48,6 +49,64 @@ fn create_task_creates_one_attached_engineer_and_initial_inbox_message() { assert!(messages[0].message.contains("Build the widget")); } +#[test] +fn visible_fresh_inbox_message_is_deferred() { + let now = Utc::now(); + let msg = inbox::InboxMessage { + seq: 1, + from: "pm".to_string(), + message: "fresh".to_string(), + timestamp: now - Duration::seconds(60), + }; + + assert!(use_cases::should_defer_visible_fresh_inbox_message( + &msg, + now, + Some(true) + )); +} + +#[test] +fn hidden_fresh_inbox_message_is_not_deferred() { + let now = Utc::now(); + let msg = inbox::InboxMessage { + seq: 1, + from: "pm".to_string(), + message: "fresh".to_string(), + timestamp: now - Duration::seconds(60), + }; + + assert!(!use_cases::should_defer_visible_fresh_inbox_message( + &msg, + now, + Some(false) + )); +} + +#[test] +fn visibility_error_defers_fresh_but_not_old_inbox_message() { + let now = Utc::now(); + let fresh = inbox::InboxMessage { + seq: 1, + from: "pm".to_string(), + message: "fresh".to_string(), + timestamp: now - Duration::seconds(60), + }; + let old = inbox::InboxMessage { + seq: 2, + from: "pm".to_string(), + message: "old".to_string(), + timestamp: now - Duration::seconds(use_cases::INBOX_VISIBLE_FRESH_DEFERRAL_SECS), + }; + + assert!(use_cases::should_defer_visible_fresh_inbox_message( + &fresh, now, None + )); + assert!(!use_cases::should_defer_visible_fresh_inbox_message( + &old, now, None + )); +} + #[test] fn empty_description_task_still_creates_attached_engineer() { let tmp = tempfile::tempdir().unwrap();