From a1764506ce3bdc0cf5b36282154f5d8fd897305f Mon Sep 17 00:00:00 2001 From: Clay Loveless Date: Sun, 5 Apr 2026 11:16:53 -0400 Subject: [PATCH] fix(observability): use platform-correct log directories and atomic writes Logs were landing in XDG-style paths on macOS instead of ~/Library/Logs/. Replace the generic ProjectDirs fallback with platform_log_dir() that resolves correctly per OS: macOS ~/Library/Logs/, Linux $XDG_STATE_HOME, Windows LocalAppData. Also buffer each JSON log line into a single write() syscall so concurrent O_APPEND writers can't interleave partial lines. Accept service name as a caller parameter in from_env_with_overrides instead of hard-coding CARGO_PKG_NAME at compile time. Isolate integration tests from production log paths via env var override. Release-Note: Fix log files writing to wrong directory on macOS Release-Impact: medium --- .config/bito.yaml | 36 +++++++++++++++ .config/scrat.toml | 8 ++++ .repo.yml | 2 +- crates/bito-core/src/observability.rs | 60 +++++++++++++++++++++---- crates/bito/src/main.rs | 1 + crates/bito/tests/cli.rs | 7 ++- crates/bito/tests/config_integration.rs | 7 ++- 7 files changed, 110 insertions(+), 11 deletions(-) create mode 100644 .config/bito.yaml create mode 100644 .config/scrat.toml diff --git a/.config/bito.yaml b/.config/bito.yaml new file mode 100644 index 0000000..4ce2fb8 --- /dev/null +++ b/.config/bito.yaml @@ -0,0 +1,36 @@ +# .bito.yaml — generated by building-in-the-open plugin +# Edit freely. Re-run plugin setup to regenerate from defaults. + +dialect: en-us +max_grade: 12.0 +passive_max_percent: 15.0 +tokenizer: claude + +rules: + - paths: [".handoffs/*.md"] + checks: + completeness: + template: handoff + tokens: + budget: 2000 + + - paths: ["record/decisions/*.md"] + checks: + completeness: + template: adr + readability: + max_grade: 12.0 + + - paths: ["record/designs/*.md"] + checks: + completeness: + template: design-doc + readability: + max_grade: 12.0 + + - paths: ["docs/**/*.md", "README.md"] + checks: + readability: + max_grade: 12.0 + grammar: + passive_max: 15.0 diff --git a/.config/scrat.toml b/.config/scrat.toml new file mode 100644 index 0000000..ec73f68 --- /dev/null +++ b/.config/scrat.toml @@ -0,0 +1,8 @@ +[project] +type = "rust" + +[commands] +test = "just test" + +[ship] +no_publish = true diff --git a/.repo.yml b/.repo.yml index 6c3b66b..eb28056 100644 --- a/.repo.yml +++ b/.repo.yml @@ -1,5 +1,5 @@ # Changes here will be overwritten by Copier -_commit: v1.2.0 +_commit: v1.3.0 _src_path: gh:claylo/claylo-rs categories: - command-line-utilities diff --git a/crates/bito-core/src/observability.rs b/crates/bito-core/src/observability.rs index 7979ef3..4a8851d 100644 --- a/crates/bito-core/src/observability.rs +++ b/crates/bito-core/src/observability.rs @@ -29,9 +29,9 @@ pub struct ObservabilityConfig { impl ObservabilityConfig { /// Create config from environment variables with optional overrides. - pub fn from_env_with_overrides(log_dir: Option) -> Self { + pub fn from_env_with_overrides(service: &str, log_dir: Option) -> Self { Self { - service: env!("CARGO_PKG_NAME").to_string(), + service: service.to_string(), log_dir, } } @@ -193,9 +193,12 @@ where event.record(&mut visitor); map.extend(visitor.values); - let mut writer = self.writer.make_writer(); - if serde_json::to_writer(&mut writer, &Value::Object(map)).is_ok() { - let _ = writer.write_all(b"\n"); + // Buffer the entire line so it's written in a single write() syscall, + // which is atomic with O_APPEND for lines under PIPE_BUF (typically 4096). + if let Ok(mut buf) = serde_json::to_vec(&Value::Object(map)) { + buf.push(b'\n'); + let mut writer = self.writer.make_writer(); + let _ = writer.write_all(&buf); } } } @@ -354,9 +357,9 @@ fn resolve_log_target_with( candidates.push(PathBuf::from(DEFAULT_LOG_DIR_UNIX)); } - // Use XDG-compliant data directory for log storage - if let Some(proj_dirs) = directories::ProjectDirs::from("", "", service) { - candidates.push(proj_dirs.data_local_dir().join("logs")); + // Platform-appropriate log directory + if let Some(log_dir) = platform_log_dir(service) { + candidates.push(log_dir); } if let Ok(dir) = std::env::current_dir() { @@ -413,6 +416,26 @@ fn ensure_writable(dir: &Path, file_name: &str) -> Result<(), String> { Ok(()) } +/// Resolve the platform-appropriate log directory for a service. +/// +/// - macOS: `~/Library/Logs/{service}/` +/// - Linux/BSD: `$XDG_STATE_HOME/{service}/logs/` (default `~/.local/state/{service}/logs/`) +/// - Windows: `{LocalAppData}/{service}/logs/` (via `directories` crate) +fn platform_log_dir(service: &str) -> Option { + if cfg!(target_os = "macos") { + std::env::var_os("HOME").map(|home| PathBuf::from(home).join("Library/Logs").join(service)) + } else if cfg!(unix) { + let state_base = std::env::var_os("XDG_STATE_HOME") + .map(PathBuf::from) + .or_else(|| { + std::env::var_os("HOME").map(|home| PathBuf::from(home).join(".local/state")) + })?; + Some(state_base.join(service).join("logs")) + } else { + directories::ProjectDirs::from("", "", service).map(|p| p.data_local_dir().join("logs")) + } +} + // ============================================================================ // Tests // ============================================================================ @@ -496,6 +519,27 @@ mod tests { assert_eq!(&ts[10..11], "T", "date-time separator"); } + #[test] + fn platform_log_dir_contains_service_name() { + let dir = platform_log_dir("test-svc").expect("platform_log_dir should return Some"); + let path = dir.to_str().expect("path should be valid UTF-8"); + assert!( + path.contains("test-svc"), + "log dir should contain service name: {path}" + ); + } + + #[test] + #[cfg(target_os = "macos")] + fn platform_log_dir_uses_library_logs_on_macos() { + let dir = platform_log_dir("test-svc").unwrap(); + let path = dir.to_str().unwrap(); + assert!( + path.contains("Library/Logs"), + "macOS log dir should use ~/Library/Logs/: {path}" + ); + } + #[test] fn days_to_ymd_known_dates() { // 1970-01-01 = day 0 diff --git a/crates/bito/src/main.rs b/crates/bito/src/main.rs index 0af08cf..85b73e4 100644 --- a/crates/bito/src/main.rs +++ b/crates/bito/src/main.rs @@ -51,6 +51,7 @@ async fn main() -> anyhow::Result<()> { let (config, config_sources) = loader.load().context("failed to load configuration")?; let obs_config = observability::ObservabilityConfig::from_env_with_overrides( + env!("CARGO_PKG_NAME"), config .log_dir .as_ref() diff --git a/crates/bito/tests/cli.rs b/crates/bito/tests/cli.rs index 5dcf337..33dc170 100644 --- a/crates/bito/tests/cli.rs +++ b/crates/bito/tests/cli.rs @@ -12,7 +12,12 @@ use predicates::prelude::*; /// cargo build directories, but works correctly for standard project layouts. #[allow(deprecated)] fn cmd() -> Command { - Command::cargo_bin(env!("CARGO_PKG_NAME")).unwrap() + let mut cmd = Command::cargo_bin(env!("CARGO_PKG_NAME")).unwrap(); + // Route log output to a temp directory so tests don't write to production paths + let prefix = env!("CARGO_PKG_NAME").to_uppercase().replace('-', "_"); + let test_log_dir = std::env::temp_dir().join(format!("{}-test-logs", env!("CARGO_PKG_NAME"))); + cmd.env(format!("{prefix}_LOG_DIR"), test_log_dir); + cmd } // ============================================================================= diff --git a/crates/bito/tests/config_integration.rs b/crates/bito/tests/config_integration.rs index 6270a3e..f07416c 100644 --- a/crates/bito/tests/config_integration.rs +++ b/crates/bito/tests/config_integration.rs @@ -13,7 +13,12 @@ use tempfile::TempDir; /// Returns a Command configured to run our binary. #[allow(deprecated)] fn cmd() -> Command { - Command::cargo_bin(env!("CARGO_PKG_NAME")).unwrap() + let mut cmd = Command::cargo_bin(env!("CARGO_PKG_NAME")).unwrap(); + // Route log output to a temp directory so tests don't write to production paths + let prefix = env!("CARGO_PKG_NAME").to_uppercase().replace('-', "_"); + let test_log_dir = std::env::temp_dir().join(format!("{}-test-logs", env!("CARGO_PKG_NAME"))); + cmd.env(format!("{prefix}_LOG_DIR"), test_log_dir); + cmd } /// Run `info --json` from a directory and parse the JSON output.