feat: add --idle-timeout CLI flag for daemon auto-shutdown (#856)
* feat: add --idle-timeout CLI flag for daemon auto-shutdown Add user-friendly --idle-timeout flag that converts time strings to milliseconds. Supports formats like '10s', '3m', '1h', or raw ms. This addresses a common need for ephemeral/CI environments where daemon processes can be orphaned if not explicitly closed, leading to resource consumption from zombie chrome-headless-shell processes. Co-authored-by: Hermes (via claude-sonnet-4-20250520) <agent@hermes.ai> * fix: address idle-timeout review feedback * fix: normalize idle-timeout parsing --------- Co-authored-by: Merlin <merlin@rbeckner.com> Co-authored-by: Hermes (via claude-sonnet-4-20250520) <agent@hermes.ai>
This commit is contained in:
@@ -2181,6 +2181,7 @@ mod tests {
|
|||||||
screenshot_dir: None,
|
screenshot_dir: None,
|
||||||
screenshot_quality: None,
|
screenshot_quality: None,
|
||||||
screenshot_format: None,
|
screenshot_format: None,
|
||||||
|
idle_timeout: None,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -234,6 +234,7 @@ pub struct DaemonOptions<'a> {
|
|||||||
pub action_policy: Option<&'a str>,
|
pub action_policy: Option<&'a str>,
|
||||||
pub confirm_actions: Option<&'a str>,
|
pub confirm_actions: Option<&'a str>,
|
||||||
pub engine: Option<&'a str>,
|
pub engine: Option<&'a str>,
|
||||||
|
pub idle_timeout: Option<&'a str>,
|
||||||
}
|
}
|
||||||
|
|
||||||
fn apply_daemon_env(cmd: &mut Command, session: &str, opts: &DaemonOptions) {
|
fn apply_daemon_env(cmd: &mut Command, session: &str, opts: &DaemonOptions) {
|
||||||
@@ -300,6 +301,9 @@ fn apply_daemon_env(cmd: &mut Command, session: &str, opts: &DaemonOptions) {
|
|||||||
if let Some(engine) = opts.engine {
|
if let Some(engine) = opts.engine {
|
||||||
cmd.env("AGENT_BROWSER_ENGINE", engine);
|
cmd.env("AGENT_BROWSER_ENGINE", engine);
|
||||||
}
|
}
|
||||||
|
if let Some(idle) = opts.idle_timeout {
|
||||||
|
cmd.env("AGENT_BROWSER_IDLE_TIMEOUT_MS", idle);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
pub fn ensure_daemon(session: &str, opts: &DaemonOptions) -> Result<DaemonResult, String> {
|
pub fn ensure_daemon(session: &str, opts: &DaemonOptions) -> Result<DaemonResult, String> {
|
||||||
|
|||||||
+130
-1
@@ -8,6 +8,48 @@ const CONFIG_DIR: &str = ".agent-browser";
|
|||||||
const CONFIG_FILENAME: &str = "config.json";
|
const CONFIG_FILENAME: &str = "config.json";
|
||||||
const PROJECT_CONFIG_FILENAME: &str = "agent-browser.json";
|
const PROJECT_CONFIG_FILENAME: &str = "agent-browser.json";
|
||||||
|
|
||||||
|
/// Parse idle timeout from user-friendly format.
|
||||||
|
/// Supports: "10s" (seconds), "3m" (minutes), "1h" (hours), or raw milliseconds.
|
||||||
|
fn parse_idle_timeout(s: &str) -> Result<String, String> {
|
||||||
|
let s = s.trim();
|
||||||
|
if s.is_empty() {
|
||||||
|
return Err("Empty idle timeout".to_string());
|
||||||
|
}
|
||||||
|
|
||||||
|
// If the value ends with a unit suffix, convert it to milliseconds.
|
||||||
|
if s.chars().last().is_some_and(|c| c.is_ascii_alphabetic()) {
|
||||||
|
let (num_str, unit) = s.split_at(s.len() - 1);
|
||||||
|
let num: u64 = num_str.parse().map_err(|_| "Invalid number")?;
|
||||||
|
|
||||||
|
let ms = match unit {
|
||||||
|
"s" => num * 1000,
|
||||||
|
"m" => num * 60 * 1000,
|
||||||
|
"h" => num * 60 * 60 * 1000,
|
||||||
|
_ => return Err("Invalid idle timeout unit (use s, m, h, or raw ms)".to_string()),
|
||||||
|
};
|
||||||
|
return Ok(ms.to_string());
|
||||||
|
}
|
||||||
|
|
||||||
|
// Pure numbers are already expressed in milliseconds.
|
||||||
|
s.parse::<u64>().map_err(|_| "Invalid idle timeout")?;
|
||||||
|
Ok(s.to_string())
|
||||||
|
}
|
||||||
|
|
||||||
|
fn parse_idle_timeout_value(value: Option<String>, source: &str) -> Option<String> {
|
||||||
|
value.and_then(|raw| match parse_idle_timeout(&raw) {
|
||||||
|
Ok(ms) => Some(ms),
|
||||||
|
Err(e) => {
|
||||||
|
eprintln!(
|
||||||
|
"{} invalid idle timeout from {}: {}",
|
||||||
|
color::warning_indicator(),
|
||||||
|
source,
|
||||||
|
e
|
||||||
|
);
|
||||||
|
None
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
#[derive(Debug, Default, Deserialize)]
|
#[derive(Debug, Default, Deserialize)]
|
||||||
#[serde(default, rename_all = "camelCase")]
|
#[serde(default, rename_all = "camelCase")]
|
||||||
pub struct Config {
|
pub struct Config {
|
||||||
@@ -45,6 +87,7 @@ pub struct Config {
|
|||||||
pub screenshot_dir: Option<String>,
|
pub screenshot_dir: Option<String>,
|
||||||
pub screenshot_quality: Option<u32>,
|
pub screenshot_quality: Option<u32>,
|
||||||
pub screenshot_format: Option<String>,
|
pub screenshot_format: Option<String>,
|
||||||
|
pub idle_timeout: Option<String>,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl Config {
|
impl Config {
|
||||||
@@ -90,6 +133,7 @@ impl Config {
|
|||||||
screenshot_dir: other.screenshot_dir.or(self.screenshot_dir),
|
screenshot_dir: other.screenshot_dir.or(self.screenshot_dir),
|
||||||
screenshot_quality: other.screenshot_quality.or(self.screenshot_quality),
|
screenshot_quality: other.screenshot_quality.or(self.screenshot_quality),
|
||||||
screenshot_format: other.screenshot_format.or(self.screenshot_format),
|
screenshot_format: other.screenshot_format.or(self.screenshot_format),
|
||||||
|
idle_timeout: other.idle_timeout.or(self.idle_timeout),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -97,7 +141,13 @@ impl Config {
|
|||||||
fn read_config_file(path: &Path) -> Option<Config> {
|
fn read_config_file(path: &Path) -> Option<Config> {
|
||||||
let content = fs::read_to_string(path).ok()?;
|
let content = fs::read_to_string(path).ok()?;
|
||||||
match serde_json::from_str::<Config>(&content) {
|
match serde_json::from_str::<Config>(&content) {
|
||||||
Ok(config) => Some(config),
|
Ok(mut config) => {
|
||||||
|
config.idle_timeout = parse_idle_timeout_value(
|
||||||
|
config.idle_timeout.take(),
|
||||||
|
&format!("config file {}", path.display()),
|
||||||
|
);
|
||||||
|
Some(config)
|
||||||
|
}
|
||||||
Err(e) => {
|
Err(e) => {
|
||||||
eprintln!(
|
eprintln!(
|
||||||
"{} invalid config file {}: {}",
|
"{} invalid config file {}: {}",
|
||||||
@@ -168,6 +218,7 @@ fn extract_config_path(args: &[String]) -> Option<Option<String>> {
|
|||||||
"--screenshot-dir",
|
"--screenshot-dir",
|
||||||
"--screenshot-quality",
|
"--screenshot-quality",
|
||||||
"--screenshot-format",
|
"--screenshot-format",
|
||||||
|
"--idle-timeout",
|
||||||
];
|
];
|
||||||
let mut i = 0;
|
let mut i = 0;
|
||||||
while i < args.len() {
|
while i < args.len() {
|
||||||
@@ -249,6 +300,7 @@ pub struct Flags {
|
|||||||
pub screenshot_dir: Option<String>,
|
pub screenshot_dir: Option<String>,
|
||||||
pub screenshot_quality: Option<u32>,
|
pub screenshot_quality: Option<u32>,
|
||||||
pub screenshot_format: Option<String>,
|
pub screenshot_format: Option<String>,
|
||||||
|
pub idle_timeout: Option<String>, // Canonical milliseconds string for AGENT_BROWSER_IDLE_TIMEOUT_MS
|
||||||
|
|
||||||
// Track which launch-time options were explicitly passed via CLI
|
// Track which launch-time options were explicitly passed via CLI
|
||||||
// (as opposed to being set only via environment variables)
|
// (as opposed to being set only via environment variables)
|
||||||
@@ -366,6 +418,11 @@ pub fn parse_flags(args: &[String]) -> Flags {
|
|||||||
.ok()
|
.ok()
|
||||||
.or(config.screenshot_format)
|
.or(config.screenshot_format)
|
||||||
.filter(|s| s == "png" || s == "jpeg"),
|
.filter(|s| s == "png" || s == "jpeg"),
|
||||||
|
idle_timeout: parse_idle_timeout_value(
|
||||||
|
env::var("AGENT_BROWSER_IDLE_TIMEOUT_MS").ok(),
|
||||||
|
"AGENT_BROWSER_IDLE_TIMEOUT_MS",
|
||||||
|
)
|
||||||
|
.or(config.idle_timeout),
|
||||||
cli_executable_path: false,
|
cli_executable_path: false,
|
||||||
cli_extensions: false,
|
cli_extensions: false,
|
||||||
cli_profile: false,
|
cli_profile: false,
|
||||||
@@ -418,6 +475,19 @@ pub fn parse_flags(args: &[String]) -> Flags {
|
|||||||
i += 1;
|
i += 1;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
"--idle-timeout" => {
|
||||||
|
if let Some(s) = args.get(i + 1) {
|
||||||
|
match parse_idle_timeout(s) {
|
||||||
|
Ok(ms) => flags.idle_timeout = Some(ms),
|
||||||
|
Err(e) => eprintln!(
|
||||||
|
"{} Invalid --idle-timeout: {}",
|
||||||
|
color::warning_indicator(),
|
||||||
|
e
|
||||||
|
),
|
||||||
|
}
|
||||||
|
i += 1;
|
||||||
|
}
|
||||||
|
}
|
||||||
"--headers" => {
|
"--headers" => {
|
||||||
if let Some(h) = args.get(i + 1) {
|
if let Some(h) = args.get(i + 1) {
|
||||||
flags.headers = Some(h.clone());
|
flags.headers = Some(h.clone());
|
||||||
@@ -690,6 +760,7 @@ pub fn clean_args(args: &[String]) -> Vec<String> {
|
|||||||
"--screenshot-dir",
|
"--screenshot-dir",
|
||||||
"--screenshot-quality",
|
"--screenshot-quality",
|
||||||
"--screenshot-format",
|
"--screenshot-format",
|
||||||
|
"--idle-timeout",
|
||||||
];
|
];
|
||||||
|
|
||||||
let mut i = 0;
|
let mut i = 0;
|
||||||
@@ -734,6 +805,36 @@ mod tests {
|
|||||||
assert_eq!(flags.headers, Some(r#"{"Auth":"token"}"#.to_string()));
|
assert_eq!(flags.headers, Some(r#"{"Auth":"token"}"#.to_string()));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_parse_idle_timeout_raw_ms() {
|
||||||
|
assert_eq!(parse_idle_timeout("10").unwrap(), "10");
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_parse_idle_timeout_seconds() {
|
||||||
|
assert_eq!(parse_idle_timeout("10s").unwrap(), "10000");
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_parse_idle_timeout_minutes() {
|
||||||
|
assert_eq!(parse_idle_timeout("3m").unwrap(), "180000");
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_parse_idle_timeout_hours() {
|
||||||
|
assert_eq!(parse_idle_timeout("1h").unwrap(), "3600000");
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_parse_idle_timeout_rejects_capital_m() {
|
||||||
|
assert!(parse_idle_timeout("10M").is_err());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_parse_idle_timeout_rejects_unknown_unit() {
|
||||||
|
assert!(parse_idle_timeout("10x").is_err());
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn test_parse_headers_flag_with_spaces() {
|
fn test_parse_headers_flag_with_spaces() {
|
||||||
// Headers JSON is passed as a single quoted argument in shell
|
// Headers JSON is passed as a single quoted argument in shell
|
||||||
@@ -829,6 +930,18 @@ mod tests {
|
|||||||
assert_eq!(cleaned, vec!["open", "example.com"]);
|
assert_eq!(cleaned, vec!["open", "example.com"]);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_clean_args_removes_idle_timeout_before_command() {
|
||||||
|
let cleaned = clean_args(&args("--idle-timeout 10s open example.com"));
|
||||||
|
assert_eq!(cleaned, vec!["open", "example.com"]);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_parse_idle_timeout_flag_converts_to_ms() {
|
||||||
|
let flags = parse_flags(&args("--idle-timeout 10s open example.com"));
|
||||||
|
assert_eq!(flags.idle_timeout.as_deref(), Some("10000"));
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn test_parse_flags_with_session_and_executable_path() {
|
fn test_parse_flags_with_session_and_executable_path() {
|
||||||
let flags = parse_flags(&args(
|
let flags = parse_flags(&args(
|
||||||
@@ -1034,6 +1147,22 @@ mod tests {
|
|||||||
let _ = fs::remove_dir(&dir);
|
let _ = fs::remove_dir(&dir);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_load_config_from_file_parses_idle_timeout() {
|
||||||
|
use std::io::Write;
|
||||||
|
let dir = std::env::temp_dir().join("ab-test-idle-timeout-config");
|
||||||
|
let _ = fs::create_dir_all(&dir);
|
||||||
|
let config_path = dir.join("test-config.json");
|
||||||
|
let mut f = fs::File::create(&config_path).unwrap();
|
||||||
|
writeln!(f, r#"{{"idleTimeout": "10s"}}"#).unwrap();
|
||||||
|
|
||||||
|
let config = read_config_file(&config_path).unwrap();
|
||||||
|
assert_eq!(config.idle_timeout.as_deref(), Some("10000"));
|
||||||
|
|
||||||
|
let _ = fs::remove_file(&config_path);
|
||||||
|
let _ = fs::remove_dir(&dir);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn test_load_config_missing_file_returns_none() {
|
fn test_load_config_missing_file_returns_none() {
|
||||||
let result = read_config_file(&PathBuf::from("/nonexistent/agent-browser.json"));
|
let result = read_config_file(&PathBuf::from("/nonexistent/agent-browser.json"));
|
||||||
|
|||||||
@@ -316,6 +316,7 @@ fn main() {
|
|||||||
action_policy: flags.action_policy.as_deref(),
|
action_policy: flags.action_policy.as_deref(),
|
||||||
confirm_actions: flags.confirm_actions.as_deref(),
|
confirm_actions: flags.confirm_actions.as_deref(),
|
||||||
engine: flags.engine.as_deref(),
|
engine: flags.engine.as_deref(),
|
||||||
|
idle_timeout: flags.idle_timeout.as_deref(),
|
||||||
};
|
};
|
||||||
let daemon_result = match ensure_daemon(&flags.session, &daemon_opts) {
|
let daemon_result = match ensure_daemon(&flags.session, &daemon_opts) {
|
||||||
Ok(result) => result,
|
Ok(result) => result,
|
||||||
|
|||||||
Reference in New Issue
Block a user