diff --git a/desktop/src-tauri/src/managed_agents/storage.rs b/desktop/src-tauri/src/managed_agents/storage.rs index 839fe65b88..ce5f4617b0 100644 --- a/desktop/src-tauri/src/managed_agents/storage.rs +++ b/desktop/src-tauri/src/managed_agents/storage.rs @@ -1,7 +1,7 @@ use std::{ collections::HashMap, - fs::{self, File, OpenOptions}, - io::{Read as _, Seek, SeekFrom, Write}, + fs, + io::Write, path::{Path, PathBuf}, }; @@ -10,10 +10,13 @@ use tauri::{AppHandle, Manager}; use crate::app_state::keyring_service; use crate::managed_agents::{ secret_seam::{hydrate_all_secrets_for_records, strip_and_persist_all_for_records}, - ManagedAgentRecord, ManagedAgentRuntimeKey, ManagedAgentRuntimeReceipt, + ManagedAgentRecord, }; use crate::secret_store::{KeyringProbe, SecretStore}; +mod logs; +pub use logs::*; + /// Keyring key name for an agent's nsec, namespaced from the human identity /// key (`"identity"`) which shares the service. fn agent_keyring_name(pubkey: &str) -> String { @@ -47,88 +50,6 @@ pub(crate) fn managed_agents_store_path(app: &AppHandle) -> Result Result { - let dir = managed_agents_base_dir(app)?.join("logs"); - fs::create_dir_all(&dir).map_err(|error| format!("failed to create logs dir: {error}"))?; - Ok(dir) -} - -/// Install-log path for `runtime_id`, alongside the agent logs. -pub fn install_log_path(app: &AppHandle, runtime_id: &str) -> Result { - Ok(managed_agents_logs_dir(app)?.join(install_log_filename(runtime_id)?)) -} - -/// Filename for a runtime's install log, or an error for an id that must not -/// become one. -/// -/// The id is validated rather than trusted: ids reach this from user-defined -/// custom harnesses as well as the catalog, and a `../` or a separator in one -/// would place the log outside the logs directory. Rejecting beats sanitizing — -/// a rejected id means no log, while a rewritten one could collide with another -/// runtime's. -fn install_log_filename(runtime_id: &str) -> Result { - if runtime_id.is_empty() || !runtime_id.chars().all(is_safe_id_char) { - return Err(format!( - "unsafe runtime id for a log filename: {runtime_id}" - )); - } - Ok(format!("install-{runtime_id}.log")) -} - -/// Characters allowed in a runtime id used as a filename. Excludes `/`, `\`, -/// `:` and `.`, so no id can traverse or escape the logs directory. -fn is_safe_id_char(c: char) -> bool { - c.is_ascii_alphanumeric() || c == '-' || c == '_' -} - -pub fn managed_agent_log_path(app: &AppHandle, pubkey: &str) -> Result { - Ok(managed_agents_logs_dir(app)?.join(format!("{pubkey}.log"))) -} - -/// Pair-scoped log path for a managed runtime. The relay URL never appears in -/// the filename; the suffix is a hash of the canonical URL. -pub fn managed_agent_runtime_log_path( - app: &AppHandle, - key: &ManagedAgentRuntimeKey, -) -> Result { - Ok(managed_agents_logs_dir(app)?.join(format!("{}.log", key.runtime_id()))) -} - -/// Log path to surface for an agent whose runtime is not tracked in memory: -/// the most recently written of its pair-scoped logs, falling back to the -/// legacy single-runtime path when the agent has not run since harnesses -/// became per (agent, relay) pair. -pub fn latest_managed_agent_log_path(app: &AppHandle, pubkey: &str) -> Result { - match newest_agent_log_in_dir(&managed_agents_logs_dir(app)?, pubkey) { - Some(path) => Ok(path), - None => managed_agent_log_path(app, pubkey), - } -} - -/// Newest log in `dir` belonging to `pubkey` — either a pair-scoped -/// `{pubkey}__{relay_hash}.log` or the legacy `{pubkey}.log`. Ties break -/// toward the higher filename so the choice is deterministic. -fn newest_agent_log_in_dir(dir: &Path, pubkey: &str) -> Option { - let legacy_name = format!("{pubkey}.log"); - let pair_prefix = format!("{pubkey}__"); - fs::read_dir(dir) - .ok()? - .flatten() - .filter_map(|entry| { - let name = entry.file_name(); - let matches = name.to_str().is_some_and(|name| { - name == legacy_name || (name.starts_with(&pair_prefix) && name.ends_with(".log")) - }); - if !matches { - return None; - } - let modified = entry.metadata().ok()?.modified().ok()?; - Some((modified, name, entry.path())) - }) - .max_by(|left, right| left.0.cmp(&right.0).then_with(|| left.1.cmp(&right.1))) - .map(|(_, _, path)| path) -} - /// The keyring operations the migration chokepoint needs. Abstracted so the /// migrate-and-strip decision logic ([`migrate_inline_key`]) can be unit-tested /// against a fake without touching the live OS keyring. @@ -759,276 +680,6 @@ pub(crate) fn atomic_write_json_restricted(path: &Path, payload: &[u8]) -> Resul .map_err(|e| format!("commit {}: {e}", resolved.display())) } -/// Maximum log file size before rotation (10 MB). -const MAX_LOG_FILE_SIZE: u64 = 10 * 1024 * 1024; - -/// If `path` exceeds [`MAX_LOG_FILE_SIZE`], rotate it to `.1`. -fn maybe_rotate_log(path: &Path) { - let size = match fs::metadata(path) { - Ok(m) => m.len(), - Err(_) => return, - }; - if size <= MAX_LOG_FILE_SIZE { - return; - } - let mut rotated = path.as_os_str().to_owned(); - rotated.push(".1"); - let _ = fs::rename(path, &rotated); -} - -pub(crate) fn open_log_file(path: &Path) -> Result { - maybe_rotate_log(path); - OpenOptions::new() - .create(true) - .append(true) - .open(path) - .map_err(|error| format!("failed to open log file {}: {error}", path.display())) -} - -/// Start a new install-log session at `path`: keep the previous run as -/// `.1` and return a freshly created, empty current file. -/// -/// Rotating per *run* rather than by size is what bounds this file. A run -/// writes one record per executed attempt, each capped by the log-scale -/// capture, so one run's file is bounded by steps × attempts × cap and the -/// history on disk is bounded at two runs. Size-triggered rotation could not -/// promise either: it never replaced an existing `.1`, and on Windows — -/// where rename does not replace its destination — it stopped working -/// altogether once `.1` existed, leaving the current file to grow. -/// -/// The old `.1` is therefore *removed* before the rename rather than renamed -/// over. Every step is best-effort: a rotation that fails must not cost the -/// user the install, so the session continues with a truncated current file. -pub(crate) fn start_install_log_session(path: &Path) -> Result { - if path.exists() { - let mut previous = path.as_os_str().to_owned(); - previous.push(".1"); - let previous = PathBuf::from(previous); - let _ = fs::remove_file(&previous); - let _ = fs::rename(path, &previous); - } - open_install_log(path, /* truncate */ true) -} - -/// Open an install log for appending one more record to the current session. -pub(crate) fn open_install_log_file(path: &Path) -> Result { - open_install_log(path, /* truncate */ false) -} - -/// Open an install log owner-only. -/// -/// The mode is set *in the create* rather than chmod'd afterwards, so the file -/// is never briefly group/world-readable. Install output can carry registry -/// tokens and proxy credentials echoed by a failing installer, so the window -/// matters even though it is short. An existing file's mode is left as-is — -/// `OpenOptions::mode` only applies on creation, and silently re-tightening a -/// file the user relaxed is not this function's call to make. -fn open_install_log(path: &Path, truncate: bool) -> Result { - let mut options = OpenOptions::new(); - options.create(true); - if truncate { - options.write(true).truncate(true); - } else { - options.append(true); - } - #[cfg(unix)] - { - use std::os::unix::fs::OpenOptionsExt; - options.mode(0o600); - } - options - .open(path) - .map_err(|error| format!("failed to open log file {}: {error}", path.display())) -} - -pub(crate) fn append_log_marker(path: &Path, message: &str) -> Result<(), String> { - let mut file = open_log_file(path)?; - writeln!(file, "{message}").map_err(|error| format!("failed to write log marker: {error}")) -} - -fn agent_pids_dir(app: &AppHandle) -> Result { - let dir = managed_agents_base_dir(app)?.join("agent-pids"); - fs::create_dir_all(&dir) - .map_err(|error| format!("failed to create agent-pids dir: {error}"))?; - Ok(dir) -} - -/// Persist a pair-scoped runtime receipt atomically. Callers must register the -/// process in memory in the same runtime transition; on write failure they must -/// terminate the child before releasing that transition. -pub fn write_agent_runtime_receipt( - app: &AppHandle, - receipt: &ManagedAgentRuntimeReceipt, -) -> Result<(), String> { - let path = agent_pids_dir(app)?.join(format!("{}.json", receipt.key.runtime_id())); - let payload = serde_json::to_vec(receipt) - .map_err(|error| format!("failed to serialize runtime receipt: {error}"))?; - atomic_write_json_restricted(&path, &payload) -} - -pub fn remove_agent_runtime_receipt(app: &AppHandle, key: &ManagedAgentRuntimeKey) { - if let Ok(dir) = agent_pids_dir(app) { - let _ = fs::remove_file(dir.join(format!("{}.json", key.runtime_id()))); - } -} - -pub fn remove_agent_runtime_receipt_path(path: &Path) { - let _ = fs::remove_file(path); -} - -pub fn read_all_agent_runtime_receipts( - app: &AppHandle, -) -> Vec<(PathBuf, ManagedAgentRuntimeReceipt)> { - let Ok(dir) = agent_pids_dir(app) else { - return Vec::new(); - }; - let Ok(entries) = fs::read_dir(dir) else { - return Vec::new(); - }; - entries - .flatten() - .filter(|entry| entry.path().extension().is_some_and(|ext| ext == "json")) - .filter_map(|entry| { - let path = entry.path(); - let bytes = fs::read(&path).ok()?; - serde_json::from_slice(&bytes) - .ok() - .map(|receipt| (path, receipt)) - }) - .collect() -} - -/// Remove the PID file for an agent (e.g. on normal stop). -pub fn remove_agent_pid_file(app: &AppHandle, pubkey: &str) { - if let Ok(dir) = agent_pids_dir(app) { - let _ = fs::remove_file(dir.join(format!("{pubkey}.pid"))); - } -} - -/// Read all PID files from `agent-pids/`, returning `(pubkey, pid)` pairs. -pub fn read_all_agent_pid_files(app: &AppHandle) -> Vec<(String, u32)> { - let Ok(dir) = agent_pids_dir(app) else { - return Vec::new(); - }; - let Ok(entries) = fs::read_dir(&dir) else { - return Vec::new(); - }; - entries - .flatten() - .filter_map(|entry| { - let name = entry.file_name(); - let name = name.to_str()?; - let pubkey = name.strip_suffix(".pid")?; - let pid: u32 = fs::read_to_string(entry.path()).ok()?.trim().parse().ok()?; - Some((pubkey.to_string(), pid)) - }) - .collect() -} - -pub fn read_log_tail(path: &Path, max_lines: usize) -> Result { - if !path.exists() { - return Ok(String::new()); - } - - let mut file = File::open(path) - .map_err(|error| format!("failed to read log file {}: {error}", path.display()))?; - - let file_len = file - .seek(SeekFrom::End(0)) - .map_err(|error| format!("failed to seek log file: {error}"))?; - - if file_len == 0 { - return Ok(String::new()); - } - - // Read backward in chunks to find enough newlines. - const CHUNK_SIZE: u64 = 8 * 1024; - let mut buf = Vec::new(); - let mut remaining = file_len; - let mut newline_count: usize = 0; - // We need max_lines + 1 newlines to delimit max_lines lines (the trailing - // newline of the last line counts as one). - let target_newlines = max_lines + 1; - - while remaining > 0 && newline_count < target_newlines { - let chunk = remaining.min(CHUNK_SIZE); - remaining -= chunk; - file.seek(SeekFrom::Start(remaining)) - .map_err(|error| format!("failed to seek log file: {error}"))?; - - let mut tmp = vec![0u8; chunk as usize]; - file.read_exact(&mut tmp) - .map_err(|error| format!("failed to read log chunk: {error}"))?; - - // Prepend this chunk so buf always has the tail of the file. - tmp.append(&mut buf); - buf = tmp; - - newline_count = bytecount_newlines(&buf); - } - - // Strip ANSI escapes here (not in the harness) so the desktop log view - // renders cleanly while terminals and other tools still get the colors - // buzz-acp emits. - let cleaned = strip_ansi_escapes::strip_str(String::from_utf8_lossy(&buf)); - let lines: Vec<&str> = cleaned.lines().collect(); - let start = lines.len().saturating_sub(max_lines); - Ok(lines[start..].join("\n")) -} - -fn bytecount_newlines(buf: &[u8]) -> usize { - buf.iter().filter(|&&b| b == b'\n').count() -} - -/// A meaningful error recovered from an exited agent's log tail. -pub struct AgentLogError { - /// The full log line, wrapped as `Agent reported error…` for display. - pub message: String, - /// JSON-RPC error code parsed from the line's `(code N)` marker, or a - /// synthetic code for known bare prefixes. `None` for legacy-format - /// lines that carry no code (or when the code fails to parse as i64). - pub code: Option, -} - -pub fn meaningful_agent_error_from_log(path: &Path) -> Option { - let tail = read_log_tail(path, 200).ok()?; - tail.lines().rev().map(str::trim).find_map(|line| { - // New format: "Agent reported error (code -32002): ..." - if let Some(rest) = line.strip_prefix("Agent reported error (code ") { - if let Some(paren_end) = rest.find("): ") { - let code = rest[..paren_end].parse::().ok(); - return Some(AgentLogError { - message: line.to_string(), - code, - }); - } - } - // Legacy format (older buzz-acp builds): "Agent reported error: ..." - if line.starts_with("Agent reported error:") { - return Some(AgentLogError { - message: line.to_string(), - code: None, - }); - } - // Bare prefixes emitted by older agent binaries whose Display still leaks - // unwrapped errors. Promote these so they surface instead of the generic - // "harness exited with status N" fallback. - if line.starts_with("llm auth:") { - return Some(AgentLogError { - message: format!("Agent reported error: {line}"), - code: Some(-32001), - }); - } - if line.starts_with("llm model not found:") { - return Some(AgentLogError { - message: format!("Agent reported error: {line}"), - code: Some(-32002), - }); - } - None - }) -} - #[cfg(test)] #[path = "storage_tests.rs"] mod tests; diff --git a/desktop/src-tauri/src/managed_agents/storage/logs.rs b/desktop/src-tauri/src/managed_agents/storage/logs.rs new file mode 100644 index 0000000000..a1d8e9b334 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/storage/logs.rs @@ -0,0 +1,374 @@ +//! Agent log files, install logs, and runtime receipt/PID persistence. +//! +//! Split out of `storage.rs` to keep that file under the size gate. These are +//! the filesystem helpers for the managed-agents `logs/` and `agent-pids/` +//! directories: log-path resolution, size- and run-based rotation, owner-only +//! install logs, runtime receipts, PID files, and log-tail error extraction. + +use std::{ + fs::{self, File, OpenOptions}, + io::{Read as _, Seek, SeekFrom, Write}, + path::{Path, PathBuf}, +}; + +use tauri::AppHandle; + +use crate::managed_agents::{ManagedAgentRuntimeKey, ManagedAgentRuntimeReceipt}; + +use super::{atomic_write_json_restricted, managed_agents_base_dir}; + +fn managed_agents_logs_dir(app: &AppHandle) -> Result { + let dir = managed_agents_base_dir(app)?.join("logs"); + fs::create_dir_all(&dir).map_err(|error| format!("failed to create logs dir: {error}"))?; + Ok(dir) +} + +/// Install-log path for `runtime_id`, alongside the agent logs. +pub fn install_log_path(app: &AppHandle, runtime_id: &str) -> Result { + Ok(managed_agents_logs_dir(app)?.join(install_log_filename(runtime_id)?)) +} + +/// Filename for a runtime's install log, or an error for an id that must not +/// become one. +/// +/// The id is validated rather than trusted: ids reach this from user-defined +/// custom harnesses as well as the catalog, and a `../` or a separator in one +/// would place the log outside the logs directory. Rejecting beats sanitizing — +/// a rejected id means no log, while a rewritten one could collide with another +/// runtime's. +fn install_log_filename(runtime_id: &str) -> Result { + if runtime_id.is_empty() || !runtime_id.chars().all(is_safe_id_char) { + return Err(format!( + "unsafe runtime id for a log filename: {runtime_id}" + )); + } + Ok(format!("install-{runtime_id}.log")) +} + +/// Characters allowed in a runtime id used as a filename. Excludes `/`, `\`, +/// `:` and `.`, so no id can traverse or escape the logs directory. +fn is_safe_id_char(c: char) -> bool { + c.is_ascii_alphanumeric() || c == '-' || c == '_' +} + +pub fn managed_agent_log_path(app: &AppHandle, pubkey: &str) -> Result { + Ok(managed_agents_logs_dir(app)?.join(format!("{pubkey}.log"))) +} + +/// Pair-scoped log path for a managed runtime. The relay URL never appears in +/// the filename; the suffix is a hash of the canonical URL. +pub fn managed_agent_runtime_log_path( + app: &AppHandle, + key: &ManagedAgentRuntimeKey, +) -> Result { + Ok(managed_agents_logs_dir(app)?.join(format!("{}.log", key.runtime_id()))) +} + +/// Log path to surface for an agent whose runtime is not tracked in memory: +/// the most recently written of its pair-scoped logs, falling back to the +/// legacy single-runtime path when the agent has not run since harnesses +/// became per (agent, relay) pair. +pub fn latest_managed_agent_log_path(app: &AppHandle, pubkey: &str) -> Result { + match newest_agent_log_in_dir(&managed_agents_logs_dir(app)?, pubkey) { + Some(path) => Ok(path), + None => managed_agent_log_path(app, pubkey), + } +} + +/// Newest log in `dir` belonging to `pubkey` — either a pair-scoped +/// `{pubkey}__{relay_hash}.log` or the legacy `{pubkey}.log`. Ties break +/// toward the higher filename so the choice is deterministic. +fn newest_agent_log_in_dir(dir: &Path, pubkey: &str) -> Option { + let legacy_name = format!("{pubkey}.log"); + let pair_prefix = format!("{pubkey}__"); + fs::read_dir(dir) + .ok()? + .flatten() + .filter_map(|entry| { + let name = entry.file_name(); + let matches = name.to_str().is_some_and(|name| { + name == legacy_name || (name.starts_with(&pair_prefix) && name.ends_with(".log")) + }); + if !matches { + return None; + } + let modified = entry.metadata().ok()?.modified().ok()?; + Some((modified, name, entry.path())) + }) + .max_by(|left, right| left.0.cmp(&right.0).then_with(|| left.1.cmp(&right.1))) + .map(|(_, _, path)| path) +} + +/// Maximum log file size before rotation (10 MB). +const MAX_LOG_FILE_SIZE: u64 = 10 * 1024 * 1024; + +/// If `path` exceeds [`MAX_LOG_FILE_SIZE`], rotate it to `.1`. +fn maybe_rotate_log(path: &Path) { + let size = match fs::metadata(path) { + Ok(m) => m.len(), + Err(_) => return, + }; + if size <= MAX_LOG_FILE_SIZE { + return; + } + let mut rotated = path.as_os_str().to_owned(); + rotated.push(".1"); + let _ = fs::rename(path, &rotated); +} + +pub(crate) fn open_log_file(path: &Path) -> Result { + maybe_rotate_log(path); + OpenOptions::new() + .create(true) + .append(true) + .open(path) + .map_err(|error| format!("failed to open log file {}: {error}", path.display())) +} + +/// Start a new install-log session at `path`: keep the previous run as +/// `.1` and return a freshly created, empty current file. +/// +/// Rotating per *run* rather than by size is what bounds this file. A run +/// writes one record per executed attempt, each capped by the log-scale +/// capture, so one run's file is bounded by steps × attempts × cap and the +/// history on disk is bounded at two runs. Size-triggered rotation could not +/// promise either: it never replaced an existing `.1`, and on Windows — +/// where rename does not replace its destination — it stopped working +/// altogether once `.1` existed, leaving the current file to grow. +/// +/// The old `.1` is therefore *removed* before the rename rather than renamed +/// over. Every step is best-effort: a rotation that fails must not cost the +/// user the install, so the session continues with a truncated current file. +pub(crate) fn start_install_log_session(path: &Path) -> Result { + if path.exists() { + let mut previous = path.as_os_str().to_owned(); + previous.push(".1"); + let previous = PathBuf::from(previous); + let _ = fs::remove_file(&previous); + let _ = fs::rename(path, &previous); + } + open_install_log(path, /* truncate */ true) +} + +/// Open an install log for appending one more record to the current session. +pub(crate) fn open_install_log_file(path: &Path) -> Result { + open_install_log(path, /* truncate */ false) +} + +/// Open an install log owner-only. +/// +/// The mode is set *in the create* rather than chmod'd afterwards, so the file +/// is never briefly group/world-readable. Install output can carry registry +/// tokens and proxy credentials echoed by a failing installer, so the window +/// matters even though it is short. An existing file's mode is left as-is — +/// `OpenOptions::mode` only applies on creation, and silently re-tightening a +/// file the user relaxed is not this function's call to make. +fn open_install_log(path: &Path, truncate: bool) -> Result { + let mut options = OpenOptions::new(); + options.create(true); + if truncate { + options.write(true).truncate(true); + } else { + options.append(true); + } + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt; + options.mode(0o600); + } + options + .open(path) + .map_err(|error| format!("failed to open log file {}: {error}", path.display())) +} + +pub(crate) fn append_log_marker(path: &Path, message: &str) -> Result<(), String> { + let mut file = open_log_file(path)?; + writeln!(file, "{message}").map_err(|error| format!("failed to write log marker: {error}")) +} + +fn agent_pids_dir(app: &AppHandle) -> Result { + let dir = managed_agents_base_dir(app)?.join("agent-pids"); + fs::create_dir_all(&dir) + .map_err(|error| format!("failed to create agent-pids dir: {error}"))?; + Ok(dir) +} + +/// Persist a pair-scoped runtime receipt atomically. Callers must register the +/// process in memory in the same runtime transition; on write failure they must +/// terminate the child before releasing that transition. +pub fn write_agent_runtime_receipt( + app: &AppHandle, + receipt: &ManagedAgentRuntimeReceipt, +) -> Result<(), String> { + let path = agent_pids_dir(app)?.join(format!("{}.json", receipt.key.runtime_id())); + let payload = serde_json::to_vec(receipt) + .map_err(|error| format!("failed to serialize runtime receipt: {error}"))?; + atomic_write_json_restricted(&path, &payload) +} + +pub fn remove_agent_runtime_receipt(app: &AppHandle, key: &ManagedAgentRuntimeKey) { + if let Ok(dir) = agent_pids_dir(app) { + let _ = fs::remove_file(dir.join(format!("{}.json", key.runtime_id()))); + } +} + +pub fn remove_agent_runtime_receipt_path(path: &Path) { + let _ = fs::remove_file(path); +} + +pub fn read_all_agent_runtime_receipts( + app: &AppHandle, +) -> Vec<(PathBuf, ManagedAgentRuntimeReceipt)> { + let Ok(dir) = agent_pids_dir(app) else { + return Vec::new(); + }; + let Ok(entries) = fs::read_dir(dir) else { + return Vec::new(); + }; + entries + .flatten() + .filter(|entry| entry.path().extension().is_some_and(|ext| ext == "json")) + .filter_map(|entry| { + let path = entry.path(); + let bytes = fs::read(&path).ok()?; + serde_json::from_slice(&bytes) + .ok() + .map(|receipt| (path, receipt)) + }) + .collect() +} + +/// Remove the PID file for an agent (e.g. on normal stop). +pub fn remove_agent_pid_file(app: &AppHandle, pubkey: &str) { + if let Ok(dir) = agent_pids_dir(app) { + let _ = fs::remove_file(dir.join(format!("{pubkey}.pid"))); + } +} + +/// Read all PID files from `agent-pids/`, returning `(pubkey, pid)` pairs. +pub fn read_all_agent_pid_files(app: &AppHandle) -> Vec<(String, u32)> { + let Ok(dir) = agent_pids_dir(app) else { + return Vec::new(); + }; + let Ok(entries) = fs::read_dir(&dir) else { + return Vec::new(); + }; + entries + .flatten() + .filter_map(|entry| { + let name = entry.file_name(); + let name = name.to_str()?; + let pubkey = name.strip_suffix(".pid")?; + let pid: u32 = fs::read_to_string(entry.path()).ok()?.trim().parse().ok()?; + Some((pubkey.to_string(), pid)) + }) + .collect() +} + +pub fn read_log_tail(path: &Path, max_lines: usize) -> Result { + if !path.exists() { + return Ok(String::new()); + } + + let mut file = File::open(path) + .map_err(|error| format!("failed to read log file {}: {error}", path.display()))?; + + let file_len = file + .seek(SeekFrom::End(0)) + .map_err(|error| format!("failed to seek log file: {error}"))?; + + if file_len == 0 { + return Ok(String::new()); + } + + // Read backward in chunks to find enough newlines. + const CHUNK_SIZE: u64 = 8 * 1024; + let mut buf = Vec::new(); + let mut remaining = file_len; + let mut newline_count: usize = 0; + // We need max_lines + 1 newlines to delimit max_lines lines (the trailing + // newline of the last line counts as one). + let target_newlines = max_lines + 1; + + while remaining > 0 && newline_count < target_newlines { + let chunk = remaining.min(CHUNK_SIZE); + remaining -= chunk; + file.seek(SeekFrom::Start(remaining)) + .map_err(|error| format!("failed to seek log file: {error}"))?; + + let mut tmp = vec![0u8; chunk as usize]; + file.read_exact(&mut tmp) + .map_err(|error| format!("failed to read log chunk: {error}"))?; + + // Prepend this chunk so buf always has the tail of the file. + tmp.append(&mut buf); + buf = tmp; + + newline_count = bytecount_newlines(&buf); + } + + // Strip ANSI escapes here (not in the harness) so the desktop log view + // renders cleanly while terminals and other tools still get the colors + // buzz-acp emits. + let cleaned = strip_ansi_escapes::strip_str(String::from_utf8_lossy(&buf)); + let lines: Vec<&str> = cleaned.lines().collect(); + let start = lines.len().saturating_sub(max_lines); + Ok(lines[start..].join("\n")) +} + +fn bytecount_newlines(buf: &[u8]) -> usize { + buf.iter().filter(|&&b| b == b'\n').count() +} + +/// A meaningful error recovered from an exited agent's log tail. +pub struct AgentLogError { + /// The full log line, wrapped as `Agent reported error…` for display. + pub message: String, + /// JSON-RPC error code parsed from the line's `(code N)` marker, or a + /// synthetic code for known bare prefixes. `None` for legacy-format + /// lines that carry no code (or when the code fails to parse as i64). + pub code: Option, +} + +pub fn meaningful_agent_error_from_log(path: &Path) -> Option { + let tail = read_log_tail(path, 200).ok()?; + tail.lines().rev().map(str::trim).find_map(|line| { + // New format: "Agent reported error (code -32002): ..." + if let Some(rest) = line.strip_prefix("Agent reported error (code ") { + if let Some(paren_end) = rest.find("): ") { + let code = rest[..paren_end].parse::().ok(); + return Some(AgentLogError { + message: line.to_string(), + code, + }); + } + } + // Legacy format (older buzz-acp builds): "Agent reported error: ..." + if line.starts_with("Agent reported error:") { + return Some(AgentLogError { + message: line.to_string(), + code: None, + }); + } + // Bare prefixes emitted by older agent binaries whose Display still leaks + // unwrapped errors. Promote these so they surface instead of the generic + // "harness exited with status N" fallback. + if line.starts_with("llm auth:") { + return Some(AgentLogError { + message: format!("Agent reported error: {line}"), + code: Some(-32001), + }); + } + if line.starts_with("llm model not found:") { + return Some(AgentLogError { + message: format!("Agent reported error: {line}"), + code: Some(-32002), + }); + } + None + }) +} + +#[cfg(test)] +#[path = "logs_tests.rs"] +mod tests; diff --git a/desktop/src-tauri/src/managed_agents/storage/logs_tests.rs b/desktop/src-tauri/src/managed_agents/storage/logs_tests.rs new file mode 100644 index 0000000000..cff48e067b --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/storage/logs_tests.rs @@ -0,0 +1,302 @@ +//! Unit tests for `managed_agents/storage/logs.rs`. +//! +//! Kept in a sibling file so `logs.rs` stays closer to the 1000-line gate; +//! `#[path]`-included from there. + +use std::fs::File; +use std::io::Write as _; +use std::path::Path; + +use tempfile::NamedTempFile; + +fn write_log(content: &str) -> NamedTempFile { + let mut file = NamedTempFile::new().expect("temp log"); + file.write_all(content.as_bytes()).expect("write log"); + file +} +#[test] +fn meaningful_agent_error_from_log_promotes_wrapped_llm_auth() { + let file = + write_log("noise\nAgent reported error (code -32001): llm auth: 401 unauthorized: ...\n"); + let result = super::meaningful_agent_error_from_log(file.path()).unwrap(); + assert!(result.message.contains("llm auth")); + assert_eq!(result.code, Some(-32001)); +} + +#[test] +fn meaningful_agent_error_from_log_promotes_unwrapped_llm_auth() { + let file = write_log("noise\nllm auth: denied\n"); + let result = super::meaningful_agent_error_from_log(file.path()).unwrap(); + assert_eq!(result.message, "Agent reported error: llm auth: denied"); + assert_eq!(result.code, Some(-32001)); +} + +#[test] +fn meaningful_agent_error_from_log_promotes_bare_model_not_found() { + let file = write_log("noise\nllm model not found: (some-model) 404\n"); + let result = super::meaningful_agent_error_from_log(file.path()).unwrap(); + assert_eq!( + result.message, + "Agent reported error: llm model not found: (some-model) 404" + ); + assert_eq!(result.code, Some(-32002)); +} + +#[test] +fn meaningful_agent_error_from_log_promotes_legacy_format() { + let file = write_log("noise\nAgent reported error: llm: 500 internal\n"); + let result = super::meaningful_agent_error_from_log(file.path()).unwrap(); + assert_eq!(result.message, "Agent reported error: llm: 500 internal"); + assert_eq!(result.code, None); +} + +#[test] +fn meaningful_agent_error_from_log_does_not_promote_midline_auth_text() { + let file = write_log("noise before llm auth: denied\n"); + assert!(super::meaningful_agent_error_from_log(file.path()).is_none()); +} + +#[test] +fn strips_ansi_from_typical_tracing_line() { + let input = "\x1b[2m2026-05-27T15:16:32\x1b[0m \x1b[32m INFO\x1b[0m \x1b[2mbuzz_acp\x1b[0m\x1b[2m:\x1b[0m starting"; + assert_eq!( + strip_ansi_escapes::strip_str(input), + "2026-05-27T15:16:32 INFO buzz_acp: starting" + ); +} +// ── harness-log selection tests ──────────────────────────────────────── + +const PUBKEY_A: &str = "aa11223344556677889900aabbccddeeff00112233445566778899aabbccddee"; +const PUBKEY_B: &str = "bb11223344556677889900aabbccddeeff00112233445566778899aabbccddee"; + +/// Write `name` into `dir` and stamp it `age_secs` before now, so selection +/// order is asserted against explicit mtimes rather than write order. +fn write_log_in(dir: &Path, name: &str, age_secs: u64) { + let path = dir.join(name); + let file = File::create(&path).expect("create log"); + file.set_modified(std::time::SystemTime::now() - std::time::Duration::from_secs(age_secs)) + .expect("stamp mtime"); +} + +#[test] +fn newest_agent_log_prefers_pair_scoped_when_it_is_freshest() { + let dir = tempfile::tempdir().expect("temp dir"); + write_log_in(dir.path(), &format!("{PUBKEY_A}.log"), 600); + write_log_in(dir.path(), &format!("{PUBKEY_A}__cafe.log"), 5); + + assert_eq!( + super::newest_agent_log_in_dir(dir.path(), PUBKEY_A), + Some(dir.path().join(format!("{PUBKEY_A}__cafe.log"))) + ); +} + +#[test] +fn newest_agent_log_prefers_legacy_when_it_is_freshest() { + let dir = tempfile::tempdir().expect("temp dir"); + write_log_in(dir.path(), &format!("{PUBKEY_A}.log"), 5); + write_log_in(dir.path(), &format!("{PUBKEY_A}__cafe.log"), 600); + + assert_eq!( + super::newest_agent_log_in_dir(dir.path(), PUBKEY_A), + Some(dir.path().join(format!("{PUBKEY_A}.log"))), + "mtime decides, not the filename shape" + ); +} + +#[test] +fn newest_agent_log_finds_sole_pair_scoped_log() { + let dir = tempfile::tempdir().expect("temp dir"); + write_log_in(dir.path(), &format!("{PUBKEY_A}__cafe.log"), 5); + + assert_eq!( + super::newest_agent_log_in_dir(dir.path(), PUBKEY_A), + Some(dir.path().join(format!("{PUBKEY_A}__cafe.log"))) + ); +} + +#[test] +fn newest_agent_log_picks_freshest_of_several_relays() { + let dir = tempfile::tempdir().expect("temp dir"); + write_log_in(dir.path(), &format!("{PUBKEY_A}__aaa.log"), 900); + write_log_in(dir.path(), &format!("{PUBKEY_A}__bbb.log"), 5); + write_log_in(dir.path(), &format!("{PUBKEY_A}__ccc.log"), 300); + + assert_eq!( + super::newest_agent_log_in_dir(dir.path(), PUBKEY_A), + Some(dir.path().join(format!("{PUBKEY_A}__bbb.log"))) + ); +} + +#[test] +fn newest_agent_log_ignores_other_agents_and_non_log_files() { + let dir = tempfile::tempdir().expect("temp dir"); + write_log_in(dir.path(), &format!("{PUBKEY_B}__cafe.log"), 1); + write_log_in(dir.path(), &format!("{PUBKEY_A}__cafe.log.gz"), 2); + write_log_in(dir.path(), &format!("{PUBKEY_A}__cafe.log"), 600); + + assert_eq!( + super::newest_agent_log_in_dir(dir.path(), PUBKEY_A), + Some(dir.path().join(format!("{PUBKEY_A}__cafe.log"))), + "a fresher log belonging to another agent must never be selected" + ); +} + +#[test] +fn newest_agent_log_is_none_when_agent_has_no_logs() { + let dir = tempfile::tempdir().expect("temp dir"); + write_log_in(dir.path(), &format!("{PUBKEY_B}.log"), 1); + + assert_eq!(super::newest_agent_log_in_dir(dir.path(), PUBKEY_A), None); +} + +#[test] +fn newest_agent_log_is_none_when_dir_is_missing() { + let dir = tempfile::tempdir().expect("temp dir"); + let missing = dir.path().join("absent"); + + assert_eq!(super::newest_agent_log_in_dir(&missing, PUBKEY_A), None); +} + +#[test] +fn newest_agent_log_breaks_mtime_ties_deterministically() { + let dir = tempfile::tempdir().expect("temp dir"); + write_log_in(dir.path(), &format!("{PUBKEY_A}__aaa.log"), 60); + write_log_in(dir.path(), &format!("{PUBKEY_A}__bbb.log"), 60); + + assert_eq!( + super::newest_agent_log_in_dir(dir.path(), PUBKEY_A), + Some(dir.path().join(format!("{PUBKEY_A}__bbb.log"))), + "equal mtimes must resolve to the same file on every read_dir order" + ); +} +// ── install logs ───────────────────────────────────────────────────────────── + +/// Install output can carry registry tokens and proxy credentials a failing +/// installer echoed, and the file is written unattended. `0o600` must come from +/// the create itself: a post-write `chmod` leaves a window where the umask +/// decides, and a crash inside it leaves the log readable to other local users. +#[cfg(unix)] +#[test] +fn install_log_is_created_owner_only_without_post_write_chmod() { + use std::os::unix::fs::PermissionsExt; + + let dir = tempfile::tempdir().expect("temp dir"); + let path = dir.path().join("install-goose.log"); + + let mut file = super::open_install_log_file(&path).expect("open install log"); + file.write_all(b"npm ERR!\n").expect("write"); + + let mode = std::fs::metadata(&path) + .expect("metadata") + .permissions() + .mode() + & 0o777; + assert_eq!(mode, 0o600, "install logs must be owner-only"); +} + +/// A run starts a new current file and keeps the previous run as `.1`, so the +/// two runs are never mixed and the history on disk stays bounded at two. +#[test] +fn install_log_session_keeps_the_previous_run_as_dot_one() { + let dir = tempfile::tempdir().expect("temp dir"); + let path = dir.path().join("install-goose.log"); + + let mut first = super::start_install_log_session(&path).expect("first session"); + first.write_all(b"run-one\n").expect("write"); + let mut second = super::start_install_log_session(&path).expect("second session"); + second.write_all(b"run-two\n").expect("write"); + + assert_eq!( + std::fs::read_to_string(&path).expect("read current"), + "run-two\n", + "the current file must hold only the newest run" + ); + assert_eq!( + std::fs::read_to_string(dir.path().join("install-goose.log.1")).expect("read .1"), + "run-one\n", + "the previous run must be preserved as .1" + ); +} + +/// The third run must still rotate when `.1` already exists. Windows `rename` +/// does not replace its destination, so a rename-only rotation silently stops +/// working here and leaves the current file to grow across every later run — +/// the old `.1` is removed first precisely so this cannot happen. Runs on the +/// Windows target too: this is the path that fails there. +#[test] +fn install_log_session_replaces_an_existing_dot_one() { + let dir = tempfile::tempdir().expect("temp dir"); + let path = dir.path().join("install-goose.log"); + let rotated = dir.path().join("install-goose.log.1"); + // Seed the state a rename-only rotation cannot get out of: both files exist. + std::fs::write(&path, b"previous-run\n").expect("seed current"); + std::fs::write(&rotated, b"ancient-run\n").expect("seed .1"); + + let mut file = super::start_install_log_session(&path).expect("session"); + file.write_all(b"fresh-run\n").expect("write"); + + assert_eq!( + std::fs::read_to_string(&path).expect("read current"), + "fresh-run\n", + "the current file must restart even when .1 was already present" + ); + assert_eq!( + std::fs::read_to_string(&rotated).expect("read .1"), + "previous-run\n", + ".1 must be replaced by the run that just ended, not kept" + ); +} + +/// Records written after the session starts append to it — a run's later +/// records must not erase its earlier ones. +#[test] +fn install_log_appends_within_a_session() { + let dir = tempfile::tempdir().expect("temp dir"); + let path = dir.path().join("install-goose.log"); + + let mut session = super::start_install_log_session(&path).expect("session"); + session.write_all(b"header\n").expect("write"); + for record in ["first\n", "second\n"] { + let mut file = super::open_install_log_file(&path).expect("open install log"); + file.write_all(record.as_bytes()).expect("write"); + } + + assert_eq!( + std::fs::read_to_string(&path).expect("read back"), + "header\nfirst\nsecond\n" + ); +} + +/// A runtime id becomes part of a filename. Ids reach this from user-defined +/// custom harnesses as well as the catalog, so anything that could traverse or +/// escape the logs directory is rejected rather than sanitized — a rejected id +/// simply means no log, while a silently rewritten one could collide with +/// another runtime's log. +#[test] +fn install_log_filename_rejects_ids_that_would_escape_the_logs_dir() { + for id in [ + "../../etc/passwd", + "goose/../../evil", + "sub/dir", + "back\\slash", + "with.dot", + "", + ] { + assert!( + super::install_log_filename(id).is_err(), + "id {id:?} must not be accepted as a filename component" + ); + } +} + +/// Ordinary catalog and custom-harness ids are accepted — the guard must not +/// reject the ids it exists to serve. +#[test] +fn install_log_filename_accepts_ordinary_runtime_ids() { + for id in ["goose", "claude-code", "buzz_agent", "codex2"] { + assert_eq!( + super::install_log_filename(id).expect("id must be usable in a log filename"), + format!("install-{id}.log") + ); + } +} diff --git a/desktop/src-tauri/src/managed_agents/storage_tests.rs b/desktop/src-tauri/src/managed_agents/storage_tests.rs index 8a15eafdc4..ccd0bbef59 100644 --- a/desktop/src-tauri/src/managed_agents/storage_tests.rs +++ b/desktop/src-tauri/src/managed_agents/storage_tests.rs @@ -5,11 +5,6 @@ use std::cell::RefCell; use std::collections::HashMap; -use std::fs::File; -use std::io::Write as _; -use std::path::Path; - -use tempfile::NamedTempFile; use super::{ agent_keyring_name, hydrate_keys_with, migrate_inline_key, persist_agent_keys_with, @@ -336,12 +331,6 @@ fn persist_agent_keys_writes_once_per_record_with_inline_key() { assert!(records[1].private_key_nsec.is_empty()); } -fn write_log(content: &str) -> NamedTempFile { - let mut file = NamedTempFile::new().expect("temp log"); - file.write_all(content.as_bytes()).expect("write log"); - file -} - /// The keyringless fallback write must land `0o600` from the write itself — /// not a post-write `chmod` — so a crash in the umask window can never leave /// plaintext agent nsecs world-readable (Wes storage.rs:239, SECURITY.md:90). @@ -368,163 +357,6 @@ fn restricted_write_lands_owner_only_without_post_write_chmod() { ); } -#[test] -fn meaningful_agent_error_from_log_promotes_wrapped_llm_auth() { - let file = - write_log("noise\nAgent reported error (code -32001): llm auth: 401 unauthorized: ...\n"); - let result = super::meaningful_agent_error_from_log(file.path()).unwrap(); - assert!(result.message.contains("llm auth")); - assert_eq!(result.code, Some(-32001)); -} - -#[test] -fn meaningful_agent_error_from_log_promotes_unwrapped_llm_auth() { - let file = write_log("noise\nllm auth: denied\n"); - let result = super::meaningful_agent_error_from_log(file.path()).unwrap(); - assert_eq!(result.message, "Agent reported error: llm auth: denied"); - assert_eq!(result.code, Some(-32001)); -} - -#[test] -fn meaningful_agent_error_from_log_promotes_bare_model_not_found() { - let file = write_log("noise\nllm model not found: (some-model) 404\n"); - let result = super::meaningful_agent_error_from_log(file.path()).unwrap(); - assert_eq!( - result.message, - "Agent reported error: llm model not found: (some-model) 404" - ); - assert_eq!(result.code, Some(-32002)); -} - -#[test] -fn meaningful_agent_error_from_log_promotes_legacy_format() { - let file = write_log("noise\nAgent reported error: llm: 500 internal\n"); - let result = super::meaningful_agent_error_from_log(file.path()).unwrap(); - assert_eq!(result.message, "Agent reported error: llm: 500 internal"); - assert_eq!(result.code, None); -} - -#[test] -fn meaningful_agent_error_from_log_does_not_promote_midline_auth_text() { - let file = write_log("noise before llm auth: denied\n"); - assert!(super::meaningful_agent_error_from_log(file.path()).is_none()); -} - -#[test] -fn strips_ansi_from_typical_tracing_line() { - let input = "\x1b[2m2026-05-27T15:16:32\x1b[0m \x1b[32m INFO\x1b[0m \x1b[2mbuzz_acp\x1b[0m\x1b[2m:\x1b[0m starting"; - assert_eq!( - strip_ansi_escapes::strip_str(input), - "2026-05-27T15:16:32 INFO buzz_acp: starting" - ); -} - -// ── harness-log selection tests ──────────────────────────────────────── - -const PUBKEY_A: &str = "aa11223344556677889900aabbccddeeff00112233445566778899aabbccddee"; -const PUBKEY_B: &str = "bb11223344556677889900aabbccddeeff00112233445566778899aabbccddee"; - -/// Write `name` into `dir` and stamp it `age_secs` before now, so selection -/// order is asserted against explicit mtimes rather than write order. -fn write_log_in(dir: &Path, name: &str, age_secs: u64) { - let path = dir.join(name); - let file = File::create(&path).expect("create log"); - file.set_modified(std::time::SystemTime::now() - std::time::Duration::from_secs(age_secs)) - .expect("stamp mtime"); -} - -#[test] -fn newest_agent_log_prefers_pair_scoped_when_it_is_freshest() { - let dir = tempfile::tempdir().expect("temp dir"); - write_log_in(dir.path(), &format!("{PUBKEY_A}.log"), 600); - write_log_in(dir.path(), &format!("{PUBKEY_A}__cafe.log"), 5); - - assert_eq!( - super::newest_agent_log_in_dir(dir.path(), PUBKEY_A), - Some(dir.path().join(format!("{PUBKEY_A}__cafe.log"))) - ); -} - -#[test] -fn newest_agent_log_prefers_legacy_when_it_is_freshest() { - let dir = tempfile::tempdir().expect("temp dir"); - write_log_in(dir.path(), &format!("{PUBKEY_A}.log"), 5); - write_log_in(dir.path(), &format!("{PUBKEY_A}__cafe.log"), 600); - - assert_eq!( - super::newest_agent_log_in_dir(dir.path(), PUBKEY_A), - Some(dir.path().join(format!("{PUBKEY_A}.log"))), - "mtime decides, not the filename shape" - ); -} - -#[test] -fn newest_agent_log_finds_sole_pair_scoped_log() { - let dir = tempfile::tempdir().expect("temp dir"); - write_log_in(dir.path(), &format!("{PUBKEY_A}__cafe.log"), 5); - - assert_eq!( - super::newest_agent_log_in_dir(dir.path(), PUBKEY_A), - Some(dir.path().join(format!("{PUBKEY_A}__cafe.log"))) - ); -} - -#[test] -fn newest_agent_log_picks_freshest_of_several_relays() { - let dir = tempfile::tempdir().expect("temp dir"); - write_log_in(dir.path(), &format!("{PUBKEY_A}__aaa.log"), 900); - write_log_in(dir.path(), &format!("{PUBKEY_A}__bbb.log"), 5); - write_log_in(dir.path(), &format!("{PUBKEY_A}__ccc.log"), 300); - - assert_eq!( - super::newest_agent_log_in_dir(dir.path(), PUBKEY_A), - Some(dir.path().join(format!("{PUBKEY_A}__bbb.log"))) - ); -} - -#[test] -fn newest_agent_log_ignores_other_agents_and_non_log_files() { - let dir = tempfile::tempdir().expect("temp dir"); - write_log_in(dir.path(), &format!("{PUBKEY_B}__cafe.log"), 1); - write_log_in(dir.path(), &format!("{PUBKEY_A}__cafe.log.gz"), 2); - write_log_in(dir.path(), &format!("{PUBKEY_A}__cafe.log"), 600); - - assert_eq!( - super::newest_agent_log_in_dir(dir.path(), PUBKEY_A), - Some(dir.path().join(format!("{PUBKEY_A}__cafe.log"))), - "a fresher log belonging to another agent must never be selected" - ); -} - -#[test] -fn newest_agent_log_is_none_when_agent_has_no_logs() { - let dir = tempfile::tempdir().expect("temp dir"); - write_log_in(dir.path(), &format!("{PUBKEY_B}.log"), 1); - - assert_eq!(super::newest_agent_log_in_dir(dir.path(), PUBKEY_A), None); -} - -#[test] -fn newest_agent_log_is_none_when_dir_is_missing() { - let dir = tempfile::tempdir().expect("temp dir"); - let missing = dir.path().join("absent"); - - assert_eq!(super::newest_agent_log_in_dir(&missing, PUBKEY_A), None); -} - -#[test] -fn newest_agent_log_breaks_mtime_ties_deterministically() { - let dir = tempfile::tempdir().expect("temp dir"); - write_log_in(dir.path(), &format!("{PUBKEY_A}__aaa.log"), 60); - write_log_in(dir.path(), &format!("{PUBKEY_A}__bbb.log"), 60); - - assert_eq!( - super::newest_agent_log_in_dir(dir.path(), PUBKEY_A), - Some(dir.path().join(format!("{PUBKEY_A}__bbb.log"))), - "equal mtimes must resolve to the same file on every read_dir order" - ); -} - // ── keyring-dev-migration tests ──────────────────────────────────────── #[test] @@ -722,138 +554,6 @@ fn try_delete_agent_key_returns_result() { let _: fn(&str) -> Result<(), String> = super::try_delete_agent_key; } -// ── install logs ───────────────────────────────────────────────────────────── - -/// Install output can carry registry tokens and proxy credentials a failing -/// installer echoed, and the file is written unattended. `0o600` must come from -/// the create itself: a post-write `chmod` leaves a window where the umask -/// decides, and a crash inside it leaves the log readable to other local users. -#[cfg(unix)] -#[test] -fn install_log_is_created_owner_only_without_post_write_chmod() { - use std::os::unix::fs::PermissionsExt; - - let dir = tempfile::tempdir().expect("temp dir"); - let path = dir.path().join("install-goose.log"); - - let mut file = super::open_install_log_file(&path).expect("open install log"); - file.write_all(b"npm ERR!\n").expect("write"); - - let mode = std::fs::metadata(&path) - .expect("metadata") - .permissions() - .mode() - & 0o777; - assert_eq!(mode, 0o600, "install logs must be owner-only"); -} - -/// A run starts a new current file and keeps the previous run as `.1`, so the -/// two runs are never mixed and the history on disk stays bounded at two. -#[test] -fn install_log_session_keeps_the_previous_run_as_dot_one() { - let dir = tempfile::tempdir().expect("temp dir"); - let path = dir.path().join("install-goose.log"); - - let mut first = super::start_install_log_session(&path).expect("first session"); - first.write_all(b"run-one\n").expect("write"); - let mut second = super::start_install_log_session(&path).expect("second session"); - second.write_all(b"run-two\n").expect("write"); - - assert_eq!( - std::fs::read_to_string(&path).expect("read current"), - "run-two\n", - "the current file must hold only the newest run" - ); - assert_eq!( - std::fs::read_to_string(dir.path().join("install-goose.log.1")).expect("read .1"), - "run-one\n", - "the previous run must be preserved as .1" - ); -} - -/// The third run must still rotate when `.1` already exists. Windows `rename` -/// does not replace its destination, so a rename-only rotation silently stops -/// working here and leaves the current file to grow across every later run — -/// the old `.1` is removed first precisely so this cannot happen. Runs on the -/// Windows target too: this is the path that fails there. -#[test] -fn install_log_session_replaces_an_existing_dot_one() { - let dir = tempfile::tempdir().expect("temp dir"); - let path = dir.path().join("install-goose.log"); - let rotated = dir.path().join("install-goose.log.1"); - // Seed the state a rename-only rotation cannot get out of: both files exist. - std::fs::write(&path, b"previous-run\n").expect("seed current"); - std::fs::write(&rotated, b"ancient-run\n").expect("seed .1"); - - let mut file = super::start_install_log_session(&path).expect("session"); - file.write_all(b"fresh-run\n").expect("write"); - - assert_eq!( - std::fs::read_to_string(&path).expect("read current"), - "fresh-run\n", - "the current file must restart even when .1 was already present" - ); - assert_eq!( - std::fs::read_to_string(&rotated).expect("read .1"), - "previous-run\n", - ".1 must be replaced by the run that just ended, not kept" - ); -} - -/// Records written after the session starts append to it — a run's later -/// records must not erase its earlier ones. -#[test] -fn install_log_appends_within_a_session() { - let dir = tempfile::tempdir().expect("temp dir"); - let path = dir.path().join("install-goose.log"); - - let mut session = super::start_install_log_session(&path).expect("session"); - session.write_all(b"header\n").expect("write"); - for record in ["first\n", "second\n"] { - let mut file = super::open_install_log_file(&path).expect("open install log"); - file.write_all(record.as_bytes()).expect("write"); - } - - assert_eq!( - std::fs::read_to_string(&path).expect("read back"), - "header\nfirst\nsecond\n" - ); -} - -/// A runtime id becomes part of a filename. Ids reach this from user-defined -/// custom harnesses as well as the catalog, so anything that could traverse or -/// escape the logs directory is rejected rather than sanitized — a rejected id -/// simply means no log, while a silently rewritten one could collide with -/// another runtime's log. -#[test] -fn install_log_filename_rejects_ids_that_would_escape_the_logs_dir() { - for id in [ - "../../etc/passwd", - "goose/../../evil", - "sub/dir", - "back\\slash", - "with.dot", - "", - ] { - assert!( - super::install_log_filename(id).is_err(), - "id {id:?} must not be accepted as a filename component" - ); - } -} - -/// Ordinary catalog and custom-harness ids are accepted — the guard must not -/// reject the ids it exists to serve. -#[test] -fn install_log_filename_accepts_ordinary_runtime_ids() { - for id in ["goose", "claude-code", "buzz_agent", "codex2"] { - assert_eq!( - super::install_log_filename(id).expect("id must be usable in a log filename"), - format!("install-{id}.log") - ); - } -} - /// Regression test: after secret extraction, the serialized store must not /// contain any secret-shaped values. This is the grep-empty criterion from /// the v5 spec acceptance criteria, exercised at the type level.