From ccddf76476af4d843b0515f6897789af6d3a7526 Mon Sep 17 00:00:00 2001 From: gustav-fff <286169375+gustav-fff@users.noreply.github.com> Date: Tue, 25 Aug 2026 10:04:38 -0700 Subject: [PATCH 1/3] fix: default fff-mcp log path to XDG state dir (#820) Per-session log files were written to the root of ~/.cache, cluttering it with up to DEFAULT_RETAIN_RUNS files and making rotation read_dir the whole cache directory on every startup. Default now resolves to $XDG_STATE_HOME/fff/fff_mcp.log (~/.local/state/fff/ fallback), and %LOCALAPPDATA%\fff\ on Windows. Closes #820 --- crates/fff-mcp/src/main.rs | 45 ++++++++++++++++++++++++++++++++------ 1 file changed, 38 insertions(+), 7 deletions(-) diff --git a/crates/fff-mcp/src/main.rs b/crates/fff-mcp/src/main.rs index f092f5e17..53607cd64 100644 --- a/crates/fff-mcp/src/main.rs +++ b/crates/fff-mcp/src/main.rs @@ -117,6 +117,7 @@ pub(crate) struct Args { /// Path-shape hint for per-session log files. /// Each fff-mcp startup writes a fresh sibling file `++.` + /// Defaults to `$XDG_STATE_HOME/fff/fff_mcp.log` (`~/.local/state/fff/fff_mcp.log`). #[arg(long = "log-file")] log_file: Option, @@ -213,16 +214,33 @@ fn resolve_defaults(args: &mut Args) { } if args.log_file.is_none() { - let home = dirs_home(); - let is_windows = cfg!(target_os = "windows"); - args.log_file = Some(if is_windows { - format!("{}\\AppData\\Local\\fff_mcp.log", home) - } else { - format!("{}/.cache/fff_mcp.log", home) - }); + args.log_file = Some(default_log_file()); } } +// Own subdirectory: log rotation read_dir's the parent on every startup, and the +// per-session `++.log` files otherwise pile up in its root. +#[cfg(not(windows))] +fn default_log_file() -> String { + let base = + absolute_env("XDG_STATE_HOME").unwrap_or_else(|| format!("{}/.local/state", dirs_home())); + format!("{}/fff/fff_mcp.log", base) +} + +#[cfg(windows)] +fn default_log_file() -> String { + let base = + absolute_env("LOCALAPPDATA").unwrap_or_else(|| format!("{}\\AppData\\Local", dirs_home())); + format!("{}\\fff\\fff_mcp.log", base) +} + +// XDG spec: a non-absolute value must be ignored as if unset. +fn absolute_env(key: &str) -> Option { + std::env::var(key) + .ok() + .filter(|value| std::path::Path::new(value).is_absolute()) +} + fn dirs_home() -> String { std::env::var("HOME") .or_else(|_| std::env::var("USERPROFILE")) @@ -447,3 +465,16 @@ fn watchdog_interval() -> Duration { } Duration::from_secs(60) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn default_log_file_lives_in_own_dir() { + let path = std::path::PathBuf::from(default_log_file()); + assert!(path.is_absolute(), "{}", path.display()); + assert_eq!(path.file_name().unwrap(), "fff_mcp.log"); + assert_eq!(path.parent().unwrap().file_name().unwrap(), "fff"); + } +} From c16c389b80a6f27863bcf8951090d25b82551d71 Mon Sep 17 00:00:00 2001 From: gustav-fff <286169375+gustav-fff@users.noreply.github.com> Date: Wed, 26 Aug 2026 16:49:33 -0700 Subject: [PATCH 2/3] refactor(mcp): build default log path with Path::join Address review: default_log_file() returns PathBuf and joins components via the Path API instead of platform-specific format! strings. Args::log_file is now Option. dirs_home() also rejects a non-absolute HOME/USERPROFILE so the default stays absolute, and the --log-file help documents the Windows default. --- crates/fff-mcp/src/healthcheck.rs | 5 ++-- crates/fff-mcp/src/main.rs | 47 ++++++++++++++++++------------- 2 files changed, 30 insertions(+), 22 deletions(-) diff --git a/crates/fff-mcp/src/healthcheck.rs b/crates/fff-mcp/src/healthcheck.rs index cc7db9e2a..02dbad192 100644 --- a/crates/fff-mcp/src/healthcheck.rs +++ b/crates/fff-mcp/src/healthcheck.rs @@ -92,14 +92,15 @@ pub fn run_healthcheck(args: &Args) -> Result<(), Box> { // 5. Log path hint (per-session files written next to this path) if let Some(ref log_path) = args.log_file { - let parent_ok = std::path::Path::new(log_path) + let parent_ok = log_path .parent() .is_some_and(|p| p.is_dir() || p.parent().is_some()); + let log_path = log_path.to_string_lossy(); all_ok &= check( "Log path", parent_ok, if parent_ok { - log_path + &log_path } else { "parent directory does not exist" }, diff --git a/crates/fff-mcp/src/main.rs b/crates/fff-mcp/src/main.rs index 53607cd64..8a94b68b7 100644 --- a/crates/fff-mcp/src/main.rs +++ b/crates/fff-mcp/src/main.rs @@ -6,6 +6,7 @@ mod parent; mod server; mod update_check; +use std::path::{Path, PathBuf}; use std::time::{Duration, SystemTime}; use clap::Parser; @@ -117,9 +118,10 @@ pub(crate) struct Args { /// Path-shape hint for per-session log files. /// Each fff-mcp startup writes a fresh sibling file `++.` - /// Defaults to `$XDG_STATE_HOME/fff/fff_mcp.log` (`~/.local/state/fff/fff_mcp.log`). + /// Defaults to `$XDG_STATE_HOME/fff/fff_mcp.log` (`~/.local/state/fff/fff_mcp.log`), + /// on Windows to `%LOCALAPPDATA%\fff\fff_mcp.log` (`~\AppData\Local\fff\fff_mcp.log`). #[arg(long = "log-file")] - log_file: Option, + log_file: Option, /// Log level (e.g. trace, debug, info, warn, error). #[arg(long = "log-level")] @@ -208,7 +210,7 @@ fn resolve_defaults(args: &mut Args) { .into_iter() .flatten() { - if let Some(parent) = std::path::Path::new(path).parent() { + if let Some(parent) = Path::new(path).parent() { let _ = std::fs::create_dir_all(parent); } } @@ -221,30 +223,31 @@ fn resolve_defaults(args: &mut Args) { // Own subdirectory: log rotation read_dir's the parent on every startup, and the // per-session `++.log` files otherwise pile up in its root. #[cfg(not(windows))] -fn default_log_file() -> String { +fn default_log_file() -> PathBuf { let base = - absolute_env("XDG_STATE_HOME").unwrap_or_else(|| format!("{}/.local/state", dirs_home())); - format!("{}/fff/fff_mcp.log", base) + absolute_env("XDG_STATE_HOME").unwrap_or_else(|| dirs_home().join(".local").join("state")); + base.join("fff").join("fff_mcp.log") } #[cfg(windows)] -fn default_log_file() -> String { +fn default_log_file() -> PathBuf { let base = - absolute_env("LOCALAPPDATA").unwrap_or_else(|| format!("{}\\AppData\\Local", dirs_home())); - format!("{}\\fff\\fff_mcp.log", base) + absolute_env("LOCALAPPDATA").unwrap_or_else(|| dirs_home().join("AppData").join("Local")); + base.join("fff").join("fff_mcp.log") } // XDG spec: a non-absolute value must be ignored as if unset. -fn absolute_env(key: &str) -> Option { - std::env::var(key) - .ok() - .filter(|value| std::path::Path::new(value).is_absolute()) +fn absolute_env(key: &str) -> Option { + std::env::var_os(key) + .map(PathBuf::from) + .filter(|value| value.is_absolute()) } -fn dirs_home() -> String { - std::env::var("HOME") - .or_else(|_| std::env::var("USERPROFILE")) - .unwrap_or_else(|_| "/tmp".to_string()) +// Relative homes would break the absolute-path contract of the default above. +fn dirs_home() -> PathBuf { + absolute_env("HOME") + .or_else(|| absolute_env("USERPROFILE")) + .unwrap_or_else(std::env::temp_dir) } #[tokio::main] @@ -256,8 +259,12 @@ async fn main() -> Result<(), Box> { return healthcheck::run_healthcheck(&args); } - let log_file = args.log_file.as_deref().unwrap_or(""); - if let Err(e) = fff::log::init_tracing(log_file, args.log_level.as_deref(), None) { + let log_file = args + .log_file + .as_deref() + .unwrap_or(Path::new("")) + .to_string_lossy(); + if let Err(e) = fff::log::init_tracing(&log_file, args.log_level.as_deref(), None) { eprintln!("Warning: Failed to init tracing: {}", e); } @@ -472,7 +479,7 @@ mod tests { #[test] fn default_log_file_lives_in_own_dir() { - let path = std::path::PathBuf::from(default_log_file()); + let path = default_log_file(); assert!(path.is_absolute(), "{}", path.display()); assert_eq!(path.file_name().unwrap(), "fff_mcp.log"); assert_eq!(path.parent().unwrap().file_name().unwrap(), "fff"); From 7e86b5c917e6585bf279515d1a763728ba6bf284 Mon Sep 17 00:00:00 2001 From: gustav-fff <286169375+gustav-fff@users.noreply.github.com> Date: Sat, 29 Aug 2026 14:55:25 -0700 Subject: [PATCH 3/3] fix(mcp): keep default log path absolute and reject non-utf8 --log-file dirs_home() now filters a relative TMPDIR and anchors at an absolute root so the default log path can never resolve under cwd. init_tracing keeps its &str API; a non-UTF-8 --log-file is rejected with a warning instead of being silently retargeted by to_string_lossy. --- crates/fff-mcp/src/main.rs | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/crates/fff-mcp/src/main.rs b/crates/fff-mcp/src/main.rs index 8a94b68b7..3b13f9658 100644 --- a/crates/fff-mcp/src/main.rs +++ b/crates/fff-mcp/src/main.rs @@ -243,13 +243,23 @@ fn absolute_env(key: &str) -> Option { .filter(|value| value.is_absolute()) } -// Relative homes would break the absolute-path contract of the default above. +// Relative homes would break the absolute-path contract of the default above, +// and TMPDIR can be relative too, so every fallback is filtered for absoluteness. fn dirs_home() -> PathBuf { absolute_env("HOME") .or_else(|| absolute_env("USERPROFILE")) - .unwrap_or_else(std::env::temp_dir) + .or_else(|| { + let tmp = std::env::temp_dir(); + tmp.is_absolute().then_some(tmp) + }) + .unwrap_or_else(|| PathBuf::from(FALLBACK_ROOT)) } +#[cfg(not(windows))] +const FALLBACK_ROOT: &str = "/tmp"; +#[cfg(windows)] +const FALLBACK_ROOT: &str = "C:\\Windows\\Temp"; + #[tokio::main] async fn main() -> Result<(), Box> { let mut args = Args::parse(); @@ -259,13 +269,19 @@ async fn main() -> Result<(), Box> { return healthcheck::run_healthcheck(&args); } - let log_file = args - .log_file - .as_deref() - .unwrap_or(Path::new("")) - .to_string_lossy(); - if let Err(e) = fff::log::init_tracing(&log_file, args.log_level.as_deref(), None) { - eprintln!("Warning: Failed to init tracing: {}", e); + // init_tracing takes &str (stable across fff-c/-python/-nvim), so reject a + // non-UTF-8 --log-file rather than let to_string_lossy retarget the file. + let log_file = args.log_file.as_deref().unwrap_or(Path::new("")); + match log_file.to_str() { + Some(log_file) => { + if let Err(e) = fff::log::init_tracing(log_file, args.log_level.as_deref(), None) { + eprintln!("Warning: Failed to init tracing: {}", e); + } + } + None => eprintln!( + "Warning: --log-file is not valid UTF-8, file logging disabled: {}", + log_file.display() + ), } let base_path = args.base_path.unwrap_or_else(|| {