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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 66 additions & 1 deletion src/tmux.rs
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,53 @@ impl Tmux {
}
}

fn required_window_id(target: &str) -> Result<String> {
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<bool> {
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<bool> {
let output = Command::new("tmux")
.args(["list-windows", "-t", session_name, "-F", "#{window_name}"])
Expand Down Expand Up @@ -921,9 +968,20 @@ fn sanitize_tmux_window_tokens(raw: &str) -> Vec<String> {
tokens
}

fn window_id_is_visible<'a>(
target_window_id: &str,
client_window_ids: impl IntoIterator<Item = &'a str>,
) -> 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() {
Expand Down Expand Up @@ -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"]));
}
}
49 changes: 49 additions & 0 deletions src/tui/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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 {
Expand Down
18 changes: 18 additions & 0 deletions src/use_cases.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<Utc>,
target_visible: Option<bool>,
) -> 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)
// ---------------------------------------------------------------------------
Expand Down
6 changes: 5 additions & 1 deletion tests/supervisor_tmux_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<String>,
}
Expand Down Expand Up @@ -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 {
Expand Down
71 changes: 71 additions & 0 deletions tests/tmux_visibility_test.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
use agman::tmux::Tmux;
use std::process::Command;
use std::time::{SystemTime, UNIX_EPOCH};

struct TmuxCleanup {
sessions: Vec<String>,
}

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)
);
}
59 changes: 59 additions & 0 deletions tests/use_cases_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -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();
Expand Down
Loading