scriptoriumadmin/fabro
1
0
Fork 0
mirror of https://github.com/fabro-sh/fabro.git synced 2026-08-28 05:27:41 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

fix(cli): unify server state and logging under storage

Browse source 
Route server-owned logs to <storage>/logs/server.log from the start of
tracing, remove legacy home/config ownership paths, and fail fast when
a running legacy daemon is detected instead of silently proceeding.

This also adds the missing sink-resolution, truncate/append,
concurrency, legacy-config, and uninstall regression coverage for the
home/storage cleanup plan.
This commit is contained in:
Bryan Helmkamp 2026-04-19 16:34:27 -04:00
parent b849738a5b
commit 858e8e1270
No known key found for this signature in database
29 changed files with 1241 additions and 428 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
docs/administration/server-configuration.mdx
View file

@ -10,7 +10,7 @@ description: "Server-owned settings.toml sections, CLI overrides, and environmen
On a same-machine setup, the CLI and server share one `settings.toml`. On a remote deployment, the server machine has its own `settings.toml`, and the client machine keeps a separate local `settings.toml` for CLI-only values such as `[cli.target]`.
<Note>
Legacy `server.toml`, `user.toml`, and `cli.toml` are ignored with a warning. Rename them to `settings.toml`.
Fabro only reads `settings.toml`. Older `server.toml`, `user.toml`, and `cli.toml` filenames are no longer part of the supported config surface.
</Note>
### Which sections are server-owned

2
docs/administration/troubleshooting.mdx
View file

@ -14,7 +14,7 @@ fabro doctor --server https://fabro.example.com/api/v1
```
It checks:
- Local user config and legacy `~/.fabro/.env` warnings
- Local user config and storage directory health
- Server-reported LLM provider connectivity
- GitHub App, sandbox, and Brave Search credentials
- Server authentication and crypto configuration

2
docs/reference/user-configuration.mdx
View file

@ -8,7 +8,7 @@ Fabro loads machine defaults from `~/.fabro/settings.toml`. The file is optional
On a same-machine setup, the CLI and server both read this file. On a remote setup, each machine has its own `settings.toml` and reads the sections relevant to that process.
<Note>
Legacy `cli.toml`, `user.toml`, and `server.toml` are ignored with a warning. Rename them to `settings.toml`.
Fabro only reads `settings.toml`. Older `cli.toml`, `user.toml`, and `server.toml` filenames are no longer part of the supported config surface.
</Note>
## File location

124
lib/crates/fabro-cli/src/commands/doctor.rs
View file

@ -2,11 +2,7 @@ use std::path::{Path, PathBuf};
use anyhow::Result;
use fabro_api::types as api_types;
use fabro_config::legacy_env;
use fabro_config::user::{
active_settings_path, legacy_old_user_config_path, legacy_server_config_path,
legacy_user_config_path,
};
use fabro_config::user::active_settings_path;
use fabro_types::settings::CliSettings;
use fabro_types::settings::cli::{CliLayer, OutputFormat};
pub(crate) use fabro_util::check_report::{
@ -22,12 +18,9 @@ use crate::command_context::CommandContext;
use crate::shared::print_json_pretty;
use crate::user_config;
pub(crate) fn check_config(
settings_path: Option<PathBuf>,
legacy_paths: &[PathBuf],
) -> CheckResult {
match (settings_path, legacy_paths.is_empty()) {
(Some(path), true) => {
pub(crate) fn check_config(settings_path: Option<PathBuf>) -> CheckResult {
match settings_path {
Some(path) => {
let display = contract_tilde(&path);
CheckResult {
name: "Configuration".to_string(),
@ -40,44 +33,7 @@ pub(crate) fn check_config(
remediation: None,
}
}
(Some(path), false) => {
let display = contract_tilde(&path);
CheckResult {
name: "Configuration".to_string(),
status: CheckStatus::Warning,
summary: display.display().to_string(),
details: std::iter::once(CheckDetail::new(format!(
"Loaded from {}",
display.display()
)))
.chain(legacy_paths.iter().map(|legacy| {
let legacy_display = contract_tilde(legacy);
CheckDetail::new(format!(
"Ignoring legacy config file {}",
legacy_display.display()
))
}))
.collect(),
remediation: Some("Delete or rename legacy config files".to_string()),
}
}
(None, false) => CheckResult {
name: "Configuration".to_string(),
status: CheckStatus::Warning,
summary: "legacy config files ignored".to_string(),
details: legacy_paths
.iter()
.map(|legacy| {
CheckDetail::new(format!("Found legacy config file {}", legacy.display()))
})
.chain(std::iter::once(CheckDetail::new(
"Rename one to ~/.fabro/settings.toml or create a new settings.toml"
.to_string(),
)))
.collect(),
remediation: Some("Create ~/.fabro/settings.toml".to_string()),
},
(None, true) => CheckResult {
None => CheckResult {
name: "Configuration".to_string(),
status: CheckStatus::Warning,
summary: "no settings config file found".to_string(),
@ -89,22 +45,6 @@ pub(crate) fn check_config(
}
}
fn check_legacy_env(path: Option<PathBuf>) -> Option<CheckResult> {
path.map(|path| {
let display = contract_tilde(&path);
CheckResult {
name: "Legacy .env".to_string(),
status: CheckStatus::Warning,
summary: "legacy secrets file detected".to_string(),
details: vec![CheckDetail::new(format!(
"{} is no longer read by fabro",
display.display()
))],
remediation: Some("Re-enter credentials with `fabro provider login`.".to_string()),
}
})
}
#[derive(Debug, Clone, PartialEq, Eq)]
struct StorageDirStatus {
path: PathBuf,
@ -284,37 +224,20 @@ pub(crate) async fn run_doctor(
};
let settings_config_path = active_settings_path(None);
let legacy_config_paths = [
legacy_user_config_path(),
legacy_old_user_config_path(),
legacy_server_config_path(),
]
.into_iter()
.flatten()
.filter(|path| path.exists())
.collect::<Vec<_>>();
let legacy_env_path = {
let p = legacy_env::legacy_env_file_path();
p.exists().then_some(p)
};
let settings = user_config::load_settings().unwrap_or_default();
let storage_dir_path = user_config::storage_dir(&settings)
.unwrap_or_else(|_| fabro_util::Home::from_env().storage_dir());
let storage_dir_path =
user_config::storage_dir(&settings).unwrap_or_else(|_| user_config::default_storage_dir());
let storage_dir = probe_storage_dir(&storage_dir_path);
let mut local_checks = vec![
let local_checks = vec![
check_config(
settings_config_path
.exists()
.then_some(settings_config_path),
&legacy_config_paths,
),
check_storage_dir(&storage_dir),
];
if let Some(legacy_env_check) = check_legacy_env(legacy_env_path) {
local_checks.push(legacy_env_check);
}
let mut report = CheckReport {
title: "Fabro Doctor".to_string(),
@ -470,45 +393,18 @@ mod tests {
#[test]
fn check_config_pass_with_path() {
let result = check_config(Some(PathBuf::from("/home/user/.fabro/settings.toml")), &[]);
let result = check_config(Some(PathBuf::from("/home/user/.fabro/settings.toml")));
assert_eq!(result.status, CheckStatus::Pass);
assert!(result.summary.contains(".fabro/settings.toml"));
}
#[test]
fn check_config_warning_without_path() {
let result = check_config(None, &[]);
let result = check_config(None);
assert_eq!(result.status, CheckStatus::Warning);
assert!(result.remediation.is_some());
}
#[test]
fn check_config_warning_for_legacy_only_path() {
let result = check_config(None, &[PathBuf::from("/home/user/.fabro/cli.toml")]);
assert_eq!(result.status, CheckStatus::Warning);
assert!(result.summary.contains("legacy"));
}
#[test]
fn check_legacy_env_warning_when_present() {
let result = check_legacy_env(Some(PathBuf::from("/home/user/.fabro/.env")));
assert_eq!(
result.as_ref().map(|check| check.status),
Some(CheckStatus::Warning)
);
assert!(
result
.as_ref()
.is_some_and(|check| check.summary.contains("legacy secrets file"))
);
}
#[test]
fn check_legacy_env_is_omitted_when_absent() {
let result = check_legacy_env(None);
assert!(result.is_none());
}
// -- check_storage_dir --
#[test]

30
lib/crates/fabro-cli/src/commands/install.rs
View file

@ -16,8 +16,8 @@ use dialoguer::theme::ColorfulTheme;
use dialoguer::{MultiSelect, Select};
use fabro_api::types::{CreateSecretRequest, SecretType as ApiSecretType};
use fabro_auth::{AuthCredential, AuthMethod, codex_oauth_config, credential_id_for};
use fabro_config::user::{SETTINGS_CONFIG_FILENAME, legacy_default_storage_root};
use fabro_config::{ResolveError, Storage, envfile, legacy_env};
use fabro_config::user::{SETTINGS_CONFIG_FILENAME, default_storage_dir};
use fabro_config::{ResolveError, Storage, envfile};
use fabro_install::{
InstallListenConfig, generate_jwt_keypair, merge_server_settings as merge_server_settings_impl,
write_github_app_settings, write_token_settings,
@ -1166,7 +1166,9 @@ async fn persist_install_outputs(
settings_write,
server_was_running,
|path| Box::pin(server_client::connect_api_client(path)),
|path, timeout| Box::pin(stop::stop_server(path, timeout)),
|path, timeout| {
Box::pin(async move { stop::stop_server(path, timeout).await.unwrap_or(false) })
},
)
.await
}
@ -1355,7 +1357,9 @@ async fn restart_server_after_install(
restart_server_after_install_with(
storage_dir,
config_path,
|path, timeout| Box::pin(stop::stop_server(path, timeout)),
|path, timeout| {
Box::pin(async move { stop::stop_server(path, timeout).await.unwrap_or(false) })
},
|storage_dir, config_path| {
Box::pin(start::ensure_server_running_for_storage(
storage_dir,
@ -1478,9 +1482,9 @@ async fn run_install_github_inner(
let storage_dir = user_config::storage_dir(&parsed_settings).unwrap_or_else(|_| {
args.storage_dir
.clone_path()
.unwrap_or_else(|| legacy_default_storage_root().join("storage"))
.unwrap_or_else(default_storage_dir)
});
let server_was_running = record::active_server_record(&storage_dir).is_some();
let server_was_running = record::active_server_record(&storage_dir)?.is_some();
let mut doc: toml::Value = toml::from_str(&existing_config_contents)
.context("failed to parse existing settings.toml")?;
@ -1623,7 +1627,7 @@ async fn run_install_inner(
let emoji = console::Emoji("⚒️ ", "");
let cli_settings = user_config::load_settings_with_storage_dir(args.storage_dir.as_deref())?;
let storage_dir = user_config::storage_dir(&cli_settings)?;
let server_was_running = record::active_server_record(&storage_dir).is_some();
let server_was_running = record::active_server_record(&storage_dir)?.is_some();
let fabro_dir = fabro_util::Home::from_env().root().to_path_buf();
let config_path = fabro_dir.join(SETTINGS_CONFIG_FILENAME);
let existing_config_contents = std::fs::read_to_string(&config_path).ok();
@ -1656,18 +1660,6 @@ async fn run_install_inner(
std::fs::create_dir_all(&fabro_dir)
.with_context(|| format!("creating fabro home directory {}", fabro_dir.display()))?;
{
let env_path = legacy_env::legacy_env_file_path();
if env_path.exists() {
fabro_util::printerr!(
printer,
" Warning: {} is no longer read by fabro server. This install will persist runtime secrets in server.env and workflow-visible credentials in the vault instead.",
env_path.display()
);
fabro_util::printerr!(printer, "");
}
}
let facts = InstallFacts {
codex_detected: detect_binary_on_path("codex").await,
};

12
lib/crates/fabro-cli/src/commands/provider/login.rs
View file

@ -1,7 +1,6 @@
use anyhow::Result;
use fabro_api::types;
use fabro_auth::credential_id_for;
use fabro_config::legacy_env;
use fabro_types::settings::CliSettings;
use fabro_types::settings::cli::CliLayer;
use fabro_util::printer::Printer;
@ -36,17 +35,6 @@ pub(super) async fn login_command(
let credential_id = credential_id_for(&credential).map_err(anyhow::Error::msg)?;
let value = serde_json::to_string(&credential)?;
{
let path = legacy_env::legacy_env_file_path();
if path.exists() {
fabro_util::printerr!(
printer,
" Warning: {} is no longer read by fabro server. Re-enter credentials with `fabro provider login`.",
path.display()
);
}
}
server
.api()
.create_secret()

14
lib/crates/fabro-cli/src/commands/server/foreground.rs
View file

@ -15,7 +15,7 @@ pub(crate) async fn execute(
record_path: PathBuf,
mut serve_args: ServeArgs,
bind: BindRequest,
storage_dir: Option<PathBuf>,
storage_dir: PathBuf,
styles: &'static Styles,
printer: Printer,
) -> Result<()> {
@ -35,22 +35,14 @@ pub(crate) async fn execute(
None
};
let log_path = storage_dir.as_ref().map_or_else(
|| {
record_path.parent().map_or_else(
|| PathBuf::from("server.log"),
|parent| parent.join("server.log"),
)
},
|dir| Storage::new(dir).server_state().log_path(),
);
let log_path = Storage::new(&storage_dir).server_state().log_path();
let dev_token_path = std::env::var_os("FABRO_DEV_TOKEN_PATH").map(PathBuf::from);
let pid = std::process::id();
Box::pin(serve::serve_command(
serve_args,
styles,
storage_dir,
Some(storage_dir),
move |resolved_bind| {
record::write_server_record(&record_path, &record::ServerRecord {
pid,

20
lib/crates/fabro-cli/src/commands/server/mod.rs
View file

@ -9,7 +9,7 @@ use std::time::Duration;
use anyhow::Result;
use base64::Engine as _;
use base64::engine::general_purpose::URL_SAFE_NO_PAD;
use fabro_config::user::{FABRO_CONFIG_ENV, active_settings_path, legacy_default_storage_root};
use fabro_config::user::{FABRO_CONFIG_ENV, active_settings_path, default_storage_dir};
use fabro_server::bind::{self, Bind, BindRequest};
use fabro_server::install::{self, InstallAppState};
use fabro_server::serve::{self, ServeArgs};
@ -27,6 +27,7 @@ use crate::user_config;
pub(crate) async fn dispatch(
command: ServerCommand,
_globals: &GlobalArgs,
foreground_log_bootstrap: Option<start::ForegroundServerLogBootstrap>,
printer: Printer,
) -> Result<()> {
match command {
@ -62,6 +63,7 @@ pub(crate) async fn dispatch(
foreground,
serve_args,
storage_dir,
foreground_log_bootstrap,
styles,
printer,
))
@ -73,8 +75,7 @@ pub(crate) async fn dispatch(
}) => {
let settings = user_config::load_settings_with_storage_dir(storage_dir.as_deref())?;
let storage_dir = user_config::storage_dir(&settings)?;
stop::execute(&storage_dir, Duration::from_secs(timeout), printer).await;
Ok(())
stop::execute(&storage_dir, Duration::from_secs(timeout), printer).await
}
ServerCommand::Restart(ServerRestartArgs {
storage_dir,
@ -87,7 +88,7 @@ pub(crate) async fn dispatch(
storage_dir.as_deref(),
&serve_args,
)? {
stop::stop_server(&bootstrap.storage_dir, Duration::from_secs(timeout)).await;
stop::stop_server(&bootstrap.storage_dir, Duration::from_secs(timeout)).await?;
if serve_args.no_web {
fabro_util::printerr!(
printer,
@ -102,7 +103,7 @@ pub(crate) async fn dispatch(
storage_dir.as_deref(),
)?;
let storage_dir = user_config::storage_dir(&settings)?;
stop::stop_server(&storage_dir, Duration::from_secs(timeout)).await;
stop::stop_server(&storage_dir, Duration::from_secs(timeout)).await?;
let bind_addr =
serve::resolve_bind_request_from_settings(&settings, serve_args.bind.as_deref())?;
let styles: &'static Styles = Box::leak(Box::new(Styles::detect_stderr()));
@ -111,6 +112,7 @@ pub(crate) async fn dispatch(
foreground,
serve_args,
storage_dir,
foreground_log_bootstrap,
styles,
printer,
))
@ -136,6 +138,7 @@ pub(crate) async fn dispatch(
.clone()
.unwrap_or_else(|| user_config::active_settings_path(None)),
);
let storage_dir = user_config::storage_dir(&settings)?;
let bind_addr =
serve::resolve_bind_request_from_settings(&settings, serve_args.bind.as_deref())?;
let styles: &'static Styles = Box::leak(Box::new(Styles::detect_stderr()));
@ -146,7 +149,7 @@ pub(crate) async fn dispatch(
..serve_args
},
bind_addr,
storage_dir.clone_path(),
storage_dir,
styles,
printer,
))
@ -181,10 +184,7 @@ fn maybe_install_bootstrap(
None => default_install_bind_request(),
};
let storage_dir = storage_dir.map_or_else(
|| legacy_default_storage_root().join("storage"),
std::path::Path::to_path_buf,
);
let storage_dir = storage_dir.map_or_else(default_storage_dir, std::path::Path::to_path_buf);
Ok(Some(InstallBootstrap {
bind_request,

40
lib/crates/fabro-cli/src/commands/server/record.rs
View file

@ -1,10 +1,10 @@
use std::path::{Path, PathBuf};
use anyhow::{Context, Result};
use anyhow::{Context, Result, bail};
use chrono::{DateTime, Utc};
use fabro_config::Storage;
use fabro_config::user::legacy_default_storage_root;
use fabro_server::bind::Bind;
use fabro_util::Home;
use serde::{Deserialize, Serialize};
#[derive(Debug, Clone, Serialize, Deserialize)]
@ -50,9 +50,8 @@ fn server_record_path(storage_dir: &Path) -> PathBuf {
}
fn legacy_record_path(storage_dir: &Path) -> Option<PathBuf> {
let default_storage_dir = legacy_default_storage_root().join("storage");
if storage_dir == default_storage_dir {
Some(server_record_path(&legacy_default_storage_root()))
if storage_dir == fabro_config::user::default_storage_dir() {
Some(Home::from_env().root().join("server.json"))
} else {
None
}
@ -71,14 +70,29 @@ fn active_server_record_at_path(path: PathBuf) -> Option<ActiveServerRecord> {
}
}
pub(crate) fn active_server_record_details(storage_dir: &Path) -> Option<ActiveServerRecord> {
pub(crate) fn active_server_record_details(
storage_dir: &Path,
) -> Result<Option<ActiveServerRecord>> {
let primary_path = server_record_path(storage_dir);
active_server_record_at_path(primary_path)
.or_else(|| legacy_record_path(storage_dir).and_then(active_server_record_at_path))
if let Some(active) = active_server_record_at_path(primary_path.clone()) {
return Ok(Some(active));
}
if let Some(legacy_path) = legacy_record_path(storage_dir) {
if active_server_record_at_path(legacy_path.clone()).is_some() {
bail!(
"Legacy server record {} is still active while current storage record {} is missing.\nStop the old daemon with a legacy Fabro CLI or manually clear the stale daemon before retrying.",
legacy_path.display(),
primary_path.display()
);
}
}
Ok(None)
}
pub(crate) fn active_server_record(storage_dir: &Path) -> Option<ServerRecord> {
active_server_record_details(storage_dir).map(|active| active.record)
pub(crate) fn active_server_record(storage_dir: &Path) -> Result<Option<ServerRecord>> {
Ok(active_server_record_details(storage_dir)?.map(|active| active.record))
}
#[cfg(unix)]
@ -111,7 +125,7 @@ mod tests {
ServerRecord {
pid: std::process::id(),
bind,
log_path: PathBuf::from("/tmp/server.log"),
log_path: PathBuf::from("/tmp/storage/logs/server.log"),
dev_token_path: None,
started_at: Utc::now(),
}
@ -132,7 +146,7 @@ mod tests {
#[test]
fn active_server_record_returns_none_when_no_file() {
let dir = tempfile::tempdir().unwrap();
assert!(active_server_record(dir.path()).is_none());
assert!(active_server_record(dir.path()).unwrap().is_none());
}
#[test]
@ -143,7 +157,7 @@ mod tests {
record.pid = u32::MAX; // definitely not alive
write_server_record(&path, &record).unwrap();
assert!(active_server_record(dir.path()).is_none());
assert!(active_server_record(dir.path()).unwrap().is_none());
assert!(!path.exists()); // lazy cleanup removed file
}
}

52
lib/crates/fabro-cli/src/commands/server/start.rs
View file

@ -17,11 +17,16 @@ use tokio::time;
use super::record;
pub(crate) struct ForegroundServerLogBootstrap {
_lock_file: std::fs::File,
}
pub(crate) async fn execute(
bind: BindRequest,
foreground: bool,
mut serve_args: ServeArgs,
storage_dir: PathBuf,
foreground_log_bootstrap: Option<ForegroundServerLogBootstrap>,
styles: &'static Styles,
printer: Printer,
) -> Result<()> {
@ -32,6 +37,8 @@ pub(crate) async fn execute(
bind,
serve_args,
storage_dir,
foreground_log_bootstrap
.context("internal error: missing foreground server log bootstrap")?,
styles,
printer,
))
@ -49,6 +56,31 @@ pub(crate) async fn execute(
}
}
pub(crate) async fn prepare_foreground_server_log(
storage_dir: &Path,
) -> Result<ForegroundServerLogBootstrap> {
let lock_file = acquire_lock(storage_dir).await?;
if let Some(existing) = record::active_server_record(storage_dir)? {
bail!(
"Server already running (pid {}) on {}",
existing.pid,
existing.bind
);
}
let log_path = Storage::new(storage_dir).server_state().log_path();
if let Some(parent) = log_path.parent() {
std::fs::create_dir_all(parent)
.with_context(|| format!("creating log directory {}", parent.display()))?;
}
std::fs::File::create(&log_path)
.with_context(|| format!("creating server log file {}", log_path.display()))?;
Ok(ForegroundServerLogBootstrap {
_lock_file: lock_file,
})
}
pub(crate) async fn ensure_server_running_for_storage(
storage_dir: &Path,
config_path: &Path,
@ -80,7 +112,7 @@ async fn ensure_server_running_with_bind(
config_path: &Path,
storage_dir: &Path,
) -> Result<Bind> {
if let Some(existing) = record::active_server_record(storage_dir) {
if let Some(existing) = record::active_server_record(storage_dir)? {
if bind_request
.as_ref()
.is_none_or(|requested| bind_matches_request(&existing.bind, requested))
@ -124,7 +156,7 @@ async fn ensure_server_running_with_bind(
)
.await
{
Ok(()) => record::active_server_record(storage_dir)
Ok(()) => record::active_server_record(storage_dir)?
.map(|server| server.bind)
.ok_or_else(|| {
anyhow!(
@ -133,7 +165,7 @@ async fn ensure_server_running_with_bind(
)
}),
Err(err) => {
if let Some(existing) = record::active_server_record(storage_dir) {
if let Some(existing) = record::active_server_record(storage_dir)? {
Ok(existing.bind)
} else {
Err(err)
@ -258,6 +290,7 @@ async fn execute_foreground(
bind: BindRequest,
serve_args: ServeArgs,
storage_dir: PathBuf,
_log_bootstrap: ForegroundServerLogBootstrap,
styles: &'static Styles,
printer: Printer,
) -> Result<()> {
@ -282,17 +315,6 @@ async fn execute_foreground(
},
);
let lock_file = acquire_lock(&storage_dir).await?;
let _lock_file = lock_file; // keep alive for the duration
if let Some(existing) = record::active_server_record(&storage_dir) {
bail!(
"Server already running (pid {}) on {}",
existing.pid,
existing.bind
);
}
let server_state = Storage::new(&storage_dir).server_state();
let record_path = server_state.record_path();
let log_path = server_state.log_path();
@ -344,7 +366,7 @@ async fn execute_daemon(
let lock_file = acquire_lock(storage_dir).await?;
let _lock_file = lock_file; // keep alive until function returns
if let Some(existing) = record::active_server_record(storage_dir) {
if let Some(existing) = record::active_server_record(storage_dir)? {
if announce {
bail!(
"Server already running (pid {}) on {}",

2
lib/crates/fabro-cli/src/commands/server/status.rs
View file

@ -7,7 +7,7 @@ use fabro_util::printer::Printer;
use super::record;
pub(crate) fn execute(storage_dir: &Path, json: bool, printer: Printer) -> Result<()> {
let Some(record) = record::active_server_record(storage_dir) else {
let Some(record) = record::active_server_record(storage_dir)? else {
if json {
fabro_util::printout!(printer, r#"{{"status":"stopped"}}"#);
} else {

14
lib/crates/fabro-cli/src/commands/server/stop.rs
View file

@ -1,15 +1,16 @@
use std::path::Path;
use std::time::Duration;
use anyhow::Result;
use fabro_server::bind::Bind;
use fabro_util::printer::Printer;
use tokio::time;
use super::record;
pub(crate) async fn stop_server(storage_dir: &Path, timeout: Duration) -> bool {
let Some(active) = record::active_server_record_details(storage_dir) else {
return false;
pub(crate) async fn stop_server(storage_dir: &Path, timeout: Duration) -> Result<bool> {
let Some(active) = record::active_server_record_details(storage_dir)? else {
return Ok(false);
};
let record = active.record;
@ -36,14 +37,15 @@ pub(crate) async fn stop_server(storage_dir: &Path, timeout: Duration) -> bool {
let _ = std::fs::remove_file(path);
}
true
Ok(true)
}
pub(crate) async fn execute(storage_dir: &Path, timeout: Duration, printer: Printer) {
if !stop_server(storage_dir, timeout).await {
pub(crate) async fn execute(storage_dir: &Path, timeout: Duration, printer: Printer) -> Result<()> {
if !stop_server(storage_dir, timeout).await? {
fabro_util::printerr!(printer, "Server is not running");
std::process::exit(1);
}
fabro_util::printerr!(printer, "Server stopped");
Ok(())
}

23
lib/crates/fabro-cli/src/commands/uninstall.rs
View file

@ -48,11 +48,14 @@ pub(crate) async fn run_uninstall(
}
let storage_dir = user_config::load_settings().map_or_else(
|_| home.storage_dir(),
|settings| user_config::storage_dir(&settings).unwrap_or_else(|_| home.storage_dir()),
|_| user_config::default_storage_dir(),
|settings| {
user_config::storage_dir(&settings)
.unwrap_or_else(|_| user_config::default_storage_dir())
},
);
let inventory = build_inventory(&home_root, &storage_dir);
let inventory = build_inventory(&home_root, &storage_dir)?;
if !args.yes {
if json {
@ -73,13 +76,13 @@ pub(crate) async fn run_uninstall(
execute_uninstall(&inventory, json, printer).await
}
fn build_inventory(home_root: &Path, storage_dir: &Path) -> Inventory {
fn build_inventory(home_root: &Path, storage_dir: &Path) -> Result<Inventory> {
let home_size = dir_size(home_root);
let server_running = server::record::active_server_record_details(storage_dir).is_some();
let server_running = server::record::active_server_record_details(storage_dir)?.is_some();
let shell_configs = find_shell_configs_with_sentinel();
let (binary_path, binary_is_managed) = resolve_binary(home_root);
Inventory {
Ok(Inventory {
home_root: home_root.to_path_buf(),
storage_dir: storage_dir.to_path_buf(),
home_exists: true,
@ -88,7 +91,7 @@ fn build_inventory(home_root: &Path, storage_dir: &Path) -> Inventory {
shell_configs,
binary_path,
binary_is_managed,
}
})
}
fn dir_size(path: &Path) -> u64 {
@ -252,7 +255,7 @@ async fn execute_uninstall(inventory: &Inventory, json: bool, printer: Printer)
// Unit 3a: Server stop
if inventory.server_running {
server::stop::execute(&inventory.storage_dir, Duration::from_secs(5), printer).await;
server::stop::execute(&inventory.storage_dir, Duration::from_secs(5), printer).await?;
result.server_stopped = true;
}
@ -730,7 +733,7 @@ mod tests {
create_fabro_home(&home_root);
let storage_dir = home_root.join("storage");
let inv = build_inventory(&home_root, &storage_dir);
let inv = build_inventory(&home_root, &storage_dir).unwrap();
assert!(inv.home_exists);
assert!(inv.home_size > 0);
@ -745,7 +748,7 @@ mod tests {
let home_root = tmp.path().join("fake-fabro-home");
create_fabro_home(&home_root);
let inv = build_inventory(&home_root, &home_root.join("storage"));
let inv = build_inventory(&home_root, &home_root.join("storage")).unwrap();
// shell_configs depends on the actual user's shell config files,
// but we verify the field is populated (even if empty in CI/test)
assert!(inv.shell_configs.is_empty() || !inv.shell_configs.is_empty());

104
lib/crates/fabro-cli/src/logging.rs
View file

@ -1,18 +1,26 @@
use std::fs::{File, OpenOptions};
use std::path::Path;
use anyhow::{Context, Result};
use fabro_util::run_log;
use tracing_appender::rolling;
use tracing_subscriber::fmt::writer::MakeWriter;
use tracing_subscriber::layer::SubscriberExt;
use tracing_subscriber::util::SubscriberInitExt;
use tracing_subscriber::{EnvFilter, fmt};
const LOG_RETENTION_DAYS: u32 = 7;
#[derive(Clone, Debug, PartialEq, Eq)]
pub(crate) enum InternalLogSink {
Cli,
Server { path: std::path::PathBuf },
}
pub(crate) fn init_tracing(
debug: bool,
config_log_level: Option<&str>,
log_prefix: &str,
sink: &InternalLogSink,
) -> Result<()> {
let default_level = if debug {
"debug"
@ -22,37 +30,28 @@ pub(crate) fn init_tracing(
let filter =
EnvFilter::try_from_env("FABRO_LOG").unwrap_or_else(|_| EnvFilter::new(default_level));
let log_dir = fabro_util::Home::from_env().logs_dir();
match sink {
InternalLogSink::Cli => {
let log_dir = fabro_util::Home::from_env().logs_dir();
std::fs::create_dir_all(&log_dir)
.with_context(|| format!("Failed to create log directory: {}", log_dir.display()))?;
std::fs::create_dir_all(&log_dir).with_context(|| {
format!("Failed to create log directory: {}", log_dir.display())
})?;
let file_appender = rolling::RollingFileAppender::builder()
.rotation(rolling::Rotation::DAILY)
.filename_prefix(log_prefix)
.filename_suffix("log")
.build(&log_dir)
.with_context(|| "Failed to create log file appender")?;
let file_appender = rolling::RollingFileAppender::builder()
.rotation(rolling::Rotation::DAILY)
.filename_prefix("cli")
.filename_suffix("log")
.build(&log_dir)
.with_context(|| "Failed to create log file appender")?;
cleanup_old_logs(&log_dir, log_prefix, LOG_RETENTION_DAYS);
let run_log_writer = run_log::init();
tracing_subscriber::registry()
.with(filter)
.with(
fmt::layer()
.with_writer(file_appender)
.with_target(true)
.with_ansi(false),
)
.with(
fmt::layer()
.with_writer(run_log_writer)
.with_target(true)
.with_ansi(false),
)
.init();
cleanup_old_logs(&log_dir, "cli", LOG_RETENTION_DAYS);
init_subscriber(filter, file_appender);
}
InternalLogSink::Server { path } => {
init_subscriber(filter, FixedFileAppender::open(path)?);
}
}
Ok(())
}
@ -88,3 +87,50 @@ fn cleanup_old_logs(log_dir: &Path, prefix: &str, max_age_days: u32) {
}
}
}
fn init_subscriber<W>(filter: EnvFilter, file_writer: W)
where
W: for<'writer> MakeWriter<'writer> + Send + Sync + 'static,
{
let run_log_writer = run_log::init();
tracing_subscriber::registry()
.with(filter)
.with(
fmt::layer()
.with_writer(file_writer)
.with_target(true)
.with_ansi(false),
)
.with(
fmt::layer()
.with_writer(run_log_writer)
.with_target(true)
.with_ansi(false),
)
.init();
}
struct FixedFileAppender {
file: File,
}
impl FixedFileAppender {
fn open(path: &Path) -> Result<Self> {
let file = OpenOptions::new()
.append(true)
.open(path)
.with_context(|| format!("Failed to open server log file: {}", path.display()))?;
Ok(Self { file })
}
}
impl<'writer> MakeWriter<'writer> for FixedFileAppender {
type Writer = File;
fn make_writer(&'writer self) -> Self::Writer {
self.file
.try_clone()
.expect("fixed log file handle should be cloneable")
}
}

324
lib/crates/fabro-cli/src/main.rs
View file

@ -25,7 +25,6 @@ use args::{
};
use clap::{CommandFactory, Parser};
use fabro_config::merge::combine_files;
use fabro_config::user::load_settings_config;
use fabro_telemetry::{git, panic as tel_panic, sanitize, sender};
use fabro_types::settings::SettingsLayer;
use fabro_types::settings::cli::OutputVerbosity;
@ -144,6 +143,10 @@ async fn main_inner() -> (String, Result<()>) {
let cli_layer = global_args_cli_layer(&globals);
let process_local_json = globals.json;
let command_name = command.name().to_string();
let pre_tracing_bootstrap = match pre_tracing_bootstrap(command.as_ref()).await {
Ok(bootstrap) => bootstrap,
Err(err) => return (command_name, Err(err)),
};
let user_settings = match user_config::load_settings() {
Ok(settings) => settings,
@ -159,35 +162,15 @@ async fn main_inner() -> (String, Result<()>) {
};
let printer = printer_from_verbosity(cli_settings.output.verbosity);
let config_log_level = if let Commands::Server(ServerNamespace {
command:
ServerCommand::Start(args::ServerStartArgs {
serve_args: args, ..
})
| ServerCommand::Serve(args::ServerServeArgs {
serve_args: args, ..
}),
}) = command.as_ref()
{
match load_settings_config(args.config.as_deref()) {
Ok(layer) => layer
.server
.as_ref()
.and_then(|server| server.logging.as_ref())
.and_then(|logging| logging.level.clone()),
Err(err) => return (command_name, Err(err.into())),
}
} else {
cli_settings.logging.level.clone()
let config_log_level = match &pre_tracing_bootstrap.sink {
logging::InternalLogSink::Cli => cli_settings.logging.level.clone(),
logging::InternalLogSink::Server { .. } => pre_tracing_bootstrap.config_log_level.clone(),
};
let log_prefix = if command_name == "server start" || command_name == "server __serve" {
"server"
} else {
"cli"
};
if let Err(err) = logging::init_tracing(globals.debug, config_log_level.as_deref(), log_prefix)
{
if let Err(err) = logging::init_tracing(
globals.debug,
config_log_level.as_deref(),
&pre_tracing_bootstrap.sink,
) {
fabro_util::printerr!(
bootstrap_printer,
"Warning: failed to initialize logging: {err:#}"
@ -195,6 +178,7 @@ async fn main_inner() -> (String, Result<()>) {
}
debug!(command = %command_name, "CLI command started");
let foreground_server_log_bootstrap = pre_tracing_bootstrap.foreground_server_log_bootstrap;
let upgrade_handle = if matches!(
command.as_ref(),
@ -254,7 +238,13 @@ async fn main_inner() -> (String, Result<()>) {
commands::model::execute(command, &cli_settings, &cli_layer, printer).await?;
}
Commands::Server(ns) => {
Box::pin(commands::server::dispatch(ns.command, &globals, printer)).await?;
Box::pin(commands::server::dispatch(
ns.command,
&globals,
foreground_server_log_bootstrap,
printer,
))
.await?;
}
Commands::Doctor(args) => {
let verbose =
@ -409,6 +399,98 @@ async fn main_inner() -> (String, Result<()>) {
(command_name, result)
}
struct PreTracingBootstrap {
sink: logging::InternalLogSink,
config_log_level: Option<String>,
foreground_server_log_bootstrap: Option<commands::server::start::ForegroundServerLogBootstrap>,
}
impl PreTracingBootstrap {
fn cli() -> Self {
Self {
sink: logging::InternalLogSink::Cli,
config_log_level: None,
foreground_server_log_bootstrap: None,
}
}
}
async fn pre_tracing_bootstrap(command: &Commands) -> Result<PreTracingBootstrap> {
match command {
Commands::Server(ServerNamespace {
command: ServerCommand::Start(args),
}) if args.foreground => {
prepare_foreground_server_bootstrap(
args.serve_args.config.as_deref(),
args.storage_dir.as_deref(),
)
.await
}
Commands::Server(ServerNamespace {
command: ServerCommand::Restart(args),
}) if args.foreground => {
prepare_foreground_server_bootstrap(
args.serve_args.config.as_deref(),
args.storage_dir.as_deref(),
)
.await
}
Commands::Server(ServerNamespace {
command: ServerCommand::Serve(args),
}) => prepare_server_sink_bootstrap(
args.serve_args.config.as_deref(),
args.storage_dir.as_deref(),
),
_ => Ok(PreTracingBootstrap::cli()),
}
}
async fn prepare_foreground_server_bootstrap(
config_path: Option<&std::path::Path>,
storage_dir: Option<&std::path::Path>,
) -> Result<PreTracingBootstrap> {
let settings =
user_config::load_settings_with_config_and_storage_dir(config_path, storage_dir)?;
let storage_dir = user_config::storage_dir(&settings)?;
let storage = fabro_config::Storage::new(&storage_dir);
let foreground_server_log_bootstrap =
commands::server::start::prepare_foreground_server_log(&storage_dir).await?;
Ok(PreTracingBootstrap {
sink: logging::InternalLogSink::Server {
path: storage.server_state().log_path(),
},
config_log_level: server_config_log_level(&settings),
foreground_server_log_bootstrap: Some(foreground_server_log_bootstrap),
})
}
fn prepare_server_sink_bootstrap(
config_path: Option<&std::path::Path>,
storage_dir: Option<&std::path::Path>,
) -> Result<PreTracingBootstrap> {
let settings =
user_config::load_settings_with_config_and_storage_dir(config_path, storage_dir)?;
let storage_dir = user_config::storage_dir(&settings)?;
let storage = fabro_config::Storage::new(&storage_dir);
Ok(PreTracingBootstrap {
sink: logging::InternalLogSink::Server {
path: storage.server_state().log_path(),
},
config_log_level: server_config_log_level(&settings),
foreground_server_log_bootstrap: None,
})
}
fn server_config_log_level(settings: &SettingsLayer) -> Option<String> {
settings
.server
.as_ref()
.and_then(|server| server.logging.as_ref())
.and_then(|logging| logging.level.clone())
}
#[cfg(test)]
mod tests {
use args::{
@ -418,6 +500,37 @@ mod tests {
use super::*;
fn runtime() -> tokio::runtime::Runtime {
tokio::runtime::Runtime::new().expect("runtime should build")
}
fn write_test_settings(path: &std::path::Path) {
std::fs::write(
path,
r#"
_version = 1
[server.logging]
level = "warn"
"#,
)
.unwrap();
}
#[test]
fn pre_tracing_bootstrap_uses_cli_sink_for_normal_cli_command() {
let cli = Cli::try_parse_from(["fabro", "uninstall"]).expect("should parse");
let command = cli.command.as_deref().unwrap();
let bootstrap = runtime()
.block_on(pre_tracing_bootstrap(command))
.expect("bootstrap should resolve");
assert_eq!(bootstrap.sink, logging::InternalLogSink::Cli);
assert!(bootstrap.config_log_level.is_none());
assert!(bootstrap.foreground_server_log_bootstrap.is_none());
}
#[test]
fn parse_provider_login_openai() {
let cli = Cli::try_parse_from(["fabro", "provider", "login", "--provider", "openai"])
@ -523,6 +636,157 @@ mod tests {
}
}
#[test]
fn pre_tracing_bootstrap_uses_server_sink_for_server_start_foreground() {
let storage_dir = tempfile::tempdir().unwrap();
let config_dir = tempfile::tempdir().unwrap();
let config_path = config_dir.path().join("settings.toml");
write_test_settings(&config_path);
let cli = Cli::try_parse_from([
"fabro",
"server",
"start",
"--foreground",
"--storage-dir",
storage_dir.path().to_str().unwrap(),
"--config",
config_path.to_str().unwrap(),
])
.expect("should parse");
let command = cli.command.as_deref().unwrap();
let bootstrap = runtime()
.block_on(pre_tracing_bootstrap(command))
.expect("bootstrap should resolve");
assert_eq!(bootstrap.sink, logging::InternalLogSink::Server {
path: storage_dir.path().join("logs").join("server.log"),
});
assert_eq!(bootstrap.config_log_level.as_deref(), Some("warn"));
assert!(bootstrap.foreground_server_log_bootstrap.is_some());
}
#[test]
fn pre_tracing_bootstrap_uses_server_sink_for_server_restart_foreground() {
let storage_dir = tempfile::tempdir().unwrap();
let config_dir = tempfile::tempdir().unwrap();
let config_path = config_dir.path().join("settings.toml");
write_test_settings(&config_path);
let cli = Cli::try_parse_from([
"fabro",
"server",
"restart",
"--foreground",
"--storage-dir",
storage_dir.path().to_str().unwrap(),
"--config",
config_path.to_str().unwrap(),
])
.expect("should parse");
let command = cli.command.as_deref().unwrap();
let bootstrap = runtime()
.block_on(pre_tracing_bootstrap(command))
.expect("bootstrap should resolve");
assert_eq!(bootstrap.sink, logging::InternalLogSink::Server {
path: storage_dir.path().join("logs").join("server.log"),
});
assert_eq!(bootstrap.config_log_level.as_deref(), Some("warn"));
assert!(bootstrap.foreground_server_log_bootstrap.is_some());
}
#[test]
fn pre_tracing_bootstrap_uses_server_sink_for_server_serve() {
let storage_dir = tempfile::tempdir().unwrap();
let config_dir = tempfile::tempdir().unwrap();
let config_path = config_dir.path().join("settings.toml");
write_test_settings(&config_path);
let record_path = storage_dir.path().join("server.json");
let cli = Cli::try_parse_from([
"fabro",
"server",
"__serve",
"--storage-dir",
storage_dir.path().to_str().unwrap(),
"--record-path",
record_path.to_str().unwrap(),
"--config",
config_path.to_str().unwrap(),
])
.expect("should parse");
let command = cli.command.as_deref().unwrap();
let bootstrap = runtime()
.block_on(pre_tracing_bootstrap(command))
.expect("bootstrap should resolve");
assert_eq!(bootstrap.sink, logging::InternalLogSink::Server {
path: storage_dir.path().join("logs").join("server.log"),
});
assert_eq!(bootstrap.config_log_level.as_deref(), Some("warn"));
assert!(bootstrap.foreground_server_log_bootstrap.is_none());
}
#[test]
fn pre_tracing_bootstrap_uses_cli_sink_for_server_start_daemon_wrapper() {
let storage_dir = tempfile::tempdir().unwrap();
let config_dir = tempfile::tempdir().unwrap();
let config_path = config_dir.path().join("settings.toml");
write_test_settings(&config_path);
let cli = Cli::try_parse_from([
"fabro",
"server",
"start",
"--storage-dir",
storage_dir.path().to_str().unwrap(),
"--config",
config_path.to_str().unwrap(),
])
.expect("should parse");
let command = cli.command.as_deref().unwrap();
let bootstrap = runtime()
.block_on(pre_tracing_bootstrap(command))
.expect("bootstrap should resolve");
assert_eq!(bootstrap.sink, logging::InternalLogSink::Cli);
assert!(bootstrap.config_log_level.is_none());
assert!(bootstrap.foreground_server_log_bootstrap.is_none());
}
#[test]
fn pre_tracing_bootstrap_uses_cli_sink_for_server_restart_daemon_wrapper() {
let storage_dir = tempfile::tempdir().unwrap();
let config_dir = tempfile::tempdir().unwrap();
let config_path = config_dir.path().join("settings.toml");
write_test_settings(&config_path);
let cli = Cli::try_parse_from([
"fabro",
"server",
"restart",
"--storage-dir",
storage_dir.path().to_str().unwrap(),
"--config",
config_path.to_str().unwrap(),
])
.expect("should parse");
let command = cli.command.as_deref().unwrap();
let bootstrap = runtime()
.block_on(pre_tracing_bootstrap(command))
.expect("bootstrap should resolve");
assert_eq!(bootstrap.sink, logging::InternalLogSink::Cli);
assert!(bootstrap.config_log_level.is_none());
assert!(bootstrap.foreground_server_log_bootstrap.is_none());
}
#[test]
fn parse_provider_login_missing_provider_flag() {
let result = Cli::try_parse_from(["fabro", "provider", "login"]);

6
lib/crates/fabro-cli/src/server_client.rs
View file

@ -231,7 +231,7 @@ fn remote_dev_token_auth_for_target<'a>(
}
fn remote_url_matches_active_local_tcp_server(api_url: &str, storage_dir: &Path) -> bool {
let Some(record) = record::active_server_record(storage_dir) else {
let Ok(Some(record)) = record::active_server_record(storage_dir) else {
return false;
};
let Bind::Tcp(bind_addr) = record.bind else {
@ -1190,7 +1190,9 @@ mod tests {
record::write_server_record(&record_path, &record::ServerRecord {
pid: std::process::id(),
bind: Bind::Unix(temp_home.path().join("fabro.sock")),
log_path: storage.path().join("server.log"),
log_path: fabro_config::Storage::new(storage.path())
.server_state()
.log_path(),
dev_token_path: Some(token_path),
started_at: chrono::Utc::now(),
})

99
lib/crates/fabro-cli/tests/it/cmd/config.rs
View file

@ -641,7 +641,7 @@ fn settings_missing_run_config_errors() {
}
#[test]
fn settings_legacy_cli_config_warns_and_ignores_it() {
fn settings_legacy_cli_config_is_silently_ignored() {
let context = test_context!();
let project = tempfile::tempdir().unwrap();
@ -663,9 +663,91 @@ name = "legacy-model"
.arg("--local")
.current_dir(project.path())
.assert()
.success()
.stderr(predicate::str::contains("ignoring legacy config file"))
.stderr(predicate::str::contains("Rename it to"));
.success();
assert!(
assert.get_output().stderr.is_empty(),
"settings should not warn about legacy config files: {}",
String::from_utf8_lossy(&assert.get_output().stderr)
);
let cfg = parse_settings(&assert.get_output().stdout);
assert_eq!(cfg["cli"]["output"]["verbosity"].as_str(), Some("normal"));
assert!(
cfg["run"]["model"].get("name").is_none(),
"resolved dense settings should omit an unset run.model.name"
);
}
#[test]
fn settings_legacy_user_config_is_silently_ignored() {
let context = test_context!();
let project = tempfile::tempdir().unwrap();
context.write_home(
".fabro/user.toml",
r#"
_version = 1
[cli.output]
verbosity = "verbose"
[run.model]
name = "legacy-model"
"#,
);
let assert = context
.settings()
.arg("--local")
.current_dir(project.path())
.assert()
.success();
assert!(
assert.get_output().stderr.is_empty(),
"settings should not warn about legacy config files: {}",
String::from_utf8_lossy(&assert.get_output().stderr)
);
let cfg = parse_settings(&assert.get_output().stdout);
assert_eq!(cfg["cli"]["output"]["verbosity"].as_str(), Some("normal"));
assert!(
cfg["run"]["model"].get("name").is_none(),
"resolved dense settings should omit an unset run.model.name"
);
}
#[test]
fn settings_legacy_server_config_is_silently_ignored() {
let context = test_context!();
let project = tempfile::tempdir().unwrap();
context.write_home(
".fabro/server.toml",
r#"
_version = 1
[cli.output]
verbosity = "verbose"
[run.model]
name = "legacy-model"
"#,
);
let assert = context
.settings()
.arg("--local")
.current_dir(project.path())
.assert()
.success();
assert!(
assert.get_output().stderr.is_empty(),
"settings should not warn about legacy config files: {}",
String::from_utf8_lossy(&assert.get_output().stderr)
);
let cfg = parse_settings(&assert.get_output().stdout);
assert_eq!(cfg["cli"]["output"]["verbosity"].as_str(), Some("normal"));
@ -697,8 +779,13 @@ shared = "legacy"
.arg("--local")
.current_dir(project.path())
.assert()
.success()
.stderr(predicate::str::contains("ignoring legacy config file"));
.success();
assert!(
assert.get_output().stderr.is_empty(),
"settings should not warn about legacy config files: {}",
String::from_utf8_lossy(&assert.get_output().stderr)
);
let cfg = parse_settings(&assert.get_output().stdout);
assert_eq!(run_model_name(&cfg), Some("project-model"));

424
lib/crates/fabro-cli/tests/it/cmd/server_start.rs
View file

@ -10,6 +10,59 @@ fn isolated_storage_dir() -> tempfile::TempDir {
root
}
fn server_log_files(logs_dir: &std::path::Path) -> Vec<std::path::PathBuf> {
let Ok(entries) = std::fs::read_dir(logs_dir) else {
return Vec::new();
};
entries
.flatten()
.map(|entry| entry.path())
.filter(|path| {
path.file_name()
.and_then(|name| name.to_str())
.is_some_and(|name| name.starts_with("server.") && name.ends_with(".log"))
})
.collect()
}
fn wait_for_path(path: &std::path::Path) {
let deadline = Instant::now() + Duration::from_secs(5);
while Instant::now() < deadline {
if path.exists() {
return;
}
std::thread::sleep(Duration::from_millis(50));
}
panic!("timed out waiting for {}", path.display());
}
fn wait_for_log_line(path: &std::path::Path, needle: &str) {
let deadline = Instant::now() + Duration::from_secs(5);
while Instant::now() < deadline {
if std::fs::read_to_string(path)
.ok()
.is_some_and(|contents| contents.contains(needle))
{
return;
}
std::thread::sleep(Duration::from_millis(50));
}
panic!("timed out waiting for {needle:?} in {}", path.display());
}
fn stop_pid(pid: u32) {
fabro_proc::sigterm(pid);
let deadline = Instant::now() + Duration::from_secs(5);
while Instant::now() < deadline {
if !fabro_proc::process_alive(pid) {
return;
}
std::thread::sleep(Duration::from_millis(50));
}
fabro_proc::sigkill(pid);
}
#[test]
fn help() {
let context = test_context!();
@ -151,6 +204,377 @@ fn start_without_default_settings_enters_install_mode_in_foreground() {
);
}
#[test]
#[expect(
clippy::disallowed_methods,
reason = "This sync integration test spawns the real foreground server process to verify log ownership."
)]
fn foreground_start_writes_tracing_to_storage_server_log() {
let home_dir = tempfile::tempdir_in("/tmp").unwrap();
let storage_root = isolated_storage_dir();
let storage_dir = storage_root.path().join("storage");
let socket_path = storage_root.path().join("foreground.sock");
let config_dir = tempfile::tempdir_in("/tmp").unwrap();
let config_path = config_dir.path().join("settings.toml");
std::fs::write(&config_path, "_version = 1\n").unwrap();
let storage_log_path = storage_dir.join("logs").join("server.log");
std::fs::create_dir_all(storage_log_path.parent().unwrap()).unwrap();
std::fs::write(&storage_log_path, "stale pre-start log entry\n").unwrap();
let mut cmd = std::process::Command::new(env!("CARGO_BIN_EXE_fabro"));
apply_test_isolation(&mut cmd, home_dir.path());
cmd.args(["server", "start", "--foreground"])
.arg("--storage-dir")
.arg(&storage_dir)
.arg("--bind")
.arg(&socket_path)
.arg("--config")
.arg(&config_path)
.stdin(Stdio::null())
.stdout(Stdio::null())
.stderr(Stdio::piped());
let mut child = cmd.spawn().expect("server start should spawn");
let record_path = storage_dir.join("server.json");
let deadline = Instant::now() + Duration::from_secs(5);
while Instant::now() < deadline {
if record_path.exists() {
break;
}
if let Some(status) = child.try_wait().expect("server start should poll") {
let output = child
.wait_with_output()
.expect("server start output should be readable");
panic!(
"foreground server exited before writing server.json with status {status}:\nstderr:\n{}",
String::from_utf8_lossy(&output.stderr)
);
}
std::thread::sleep(Duration::from_millis(50));
}
assert!(
record_path.exists(),
"expected foreground start to create server.json"
);
wait_for_log_line(&storage_log_path, "API server started");
let stop_output = {
let mut stop = std::process::Command::new(env!("CARGO_BIN_EXE_fabro"));
apply_test_isolation(&mut stop, home_dir.path());
stop.args(["server", "stop"])
.arg("--storage-dir")
.arg(&storage_dir)
.output()
.expect("server stop should run")
};
assert!(
stop_output.status.success(),
"server stop should succeed:\nstdout:\n{}\nstderr:\n{}",
String::from_utf8_lossy(&stop_output.stdout),
String::from_utf8_lossy(&stop_output.stderr)
);
let output = child
.wait_with_output()
.expect("server start output should be readable");
wait_for_log_line(
&storage_log_path,
"Shutdown signal received, stopping server",
);
let storage_log = std::fs::read_to_string(&storage_log_path).unwrap_or_default();
assert!(
storage_log.contains("API server started"),
"expected {} to contain server tracing, got:\n{}\nforeground stderr:\n{}",
storage_log_path.display(),
storage_log,
String::from_utf8_lossy(&output.stderr)
);
assert!(
storage_log.contains("Shutdown signal received, stopping server"),
"expected {} to contain shutdown tracing, got:\n{}",
storage_log_path.display(),
storage_log
);
assert!(
!storage_log.contains("stale pre-start log entry"),
"expected startup to truncate stale log contents, got:\n{}",
storage_log
);
assert!(
storage_log.find("API server started")
< storage_log.find("Shutdown signal received, stopping server"),
"expected shutdown trace to append after startup trace, got:\n{}",
storage_log
);
let home_server_logs = server_log_files(&home_dir.path().join(".fabro").join("logs"));
assert!(
home_server_logs.is_empty(),
"expected foreground server start to avoid home server logs, found: {home_server_logs:?}"
);
}
#[test]
fn daemon_start_writes_tracing_to_storage_server_log() {
let context = test_context!();
let storage_root = isolated_storage_dir();
let storage_dir = storage_root.path().join("storage");
let socket_path = storage_root.path().join("daemon.sock");
let config_dir = tempfile::tempdir_in("/tmp").unwrap();
let config_path = config_dir.path().join("settings.toml");
std::fs::write(&config_path, "_version = 1\n").unwrap();
let storage_log_path = storage_dir.join("logs").join("server.log");
std::fs::create_dir_all(storage_log_path.parent().unwrap()).unwrap();
std::fs::write(&storage_log_path, "stale pre-start log entry\n").unwrap();
context
.command()
.args(["server", "start"])
.arg("--storage-dir")
.arg(&storage_dir)
.arg("--bind")
.arg(&socket_path)
.arg("--config")
.arg(&config_path)
.assert()
.success();
wait_for_log_line(&storage_log_path, "API server started");
context
.command()
.args(["server", "stop"])
.arg("--storage-dir")
.arg(&storage_dir)
.assert()
.success();
wait_for_log_line(
&storage_log_path,
"Shutdown signal received, stopping server",
);
let storage_log = std::fs::read_to_string(&storage_log_path).unwrap_or_default();
assert!(
storage_log.contains("API server started"),
"expected {} to contain startup tracing, got:\n{}",
storage_log_path.display(),
storage_log
);
assert!(
storage_log.contains("Shutdown signal received, stopping server"),
"expected {} to contain shutdown tracing, got:\n{}",
storage_log_path.display(),
storage_log
);
assert!(
!storage_log.contains("stale pre-start log entry"),
"expected startup to truncate stale log contents, got:\n{}",
storage_log
);
assert!(
storage_log.find("API server started")
< storage_log.find("Shutdown signal received, stopping server"),
"expected shutdown trace to append after startup trace, got:\n{}",
storage_log
);
let home_server_logs = server_log_files(&context.home_dir.join(".fabro").join("logs"));
assert!(
home_server_logs.is_empty(),
"expected daemonized server start to avoid home server logs, found: {home_server_logs:?}"
);
}
#[test]
#[expect(
clippy::disallowed_methods,
reason = "This integration test moves a live daemon record on disk to simulate an unsupported legacy daemon upgrade."
)]
fn start_errors_when_only_a_legacy_running_server_record_exists() {
let home_dir = tempfile::tempdir_in("/tmp").unwrap();
let fabro_home = home_dir.path().join(".fabro");
let storage_dir = fabro_home.join("storage");
let socket_path = home_dir.path().join("legacy.sock");
let config_dir = tempfile::tempdir_in("/tmp").unwrap();
let config_path = config_dir.path().join("settings.toml");
std::fs::write(&config_path, "_version = 1\n").unwrap();
let start_output = {
let mut start = std::process::Command::new(env!("CARGO_BIN_EXE_fabro"));
apply_test_isolation(&mut start, home_dir.path());
start
.args(["server", "start", "--bind"])
.arg(&socket_path)
.arg("--config")
.arg(&config_path)
.output()
.expect("server start should run")
};
assert!(
start_output.status.success(),
"server start should succeed:\nstdout:\n{}\nstderr:\n{}",
String::from_utf8_lossy(&start_output.stdout),
String::from_utf8_lossy(&start_output.stderr)
);
let current_record = storage_dir.join("server.json");
wait_for_path(&current_record);
let legacy_record = fabro_home.join("server.json");
std::fs::rename(&current_record, &legacy_record).unwrap();
let retry_output = {
let mut retry = std::process::Command::new(env!("CARGO_BIN_EXE_fabro"));
apply_test_isolation(&mut retry, home_dir.path());
retry
.args(["server", "start", "--bind"])
.arg(home_dir.path().join("new.sock"))
.arg("--config")
.arg(&config_path)
.output()
.expect("server start retry should run")
};
let pid = serde_json::from_str::<serde_json::Value>(
&std::fs::read_to_string(&legacy_record).unwrap(),
)
.unwrap()["pid"]
.as_u64()
.unwrap() as u32;
stop_pid(pid);
let _ = std::fs::remove_file(&legacy_record);
let _ = std::fs::remove_file(&socket_path);
assert!(
!retry_output.status.success(),
"server start should fail when only the legacy record exists"
);
let stderr = String::from_utf8_lossy(&retry_output.stderr);
assert!(
stderr.contains(&legacy_record.display().to_string()),
"expected stderr to mention the legacy record path, got:\n{stderr}"
);
assert!(
stderr.contains(&current_record.display().to_string()),
"expected stderr to mention the current record path, got:\n{stderr}"
);
assert!(
stderr.contains("legacy Fabro CLI"),
"expected stderr to instruct manual cleanup, got:\n{stderr}"
);
}
#[test]
#[expect(
clippy::disallowed_methods,
reason = "This sync integration test starts two real foreground server processes to verify lock ownership protects log truncation."
)]
fn concurrent_foreground_start_does_not_retruncate_storage_server_log() {
let home_dir = tempfile::tempdir_in("/tmp").unwrap();
let storage_root = isolated_storage_dir();
let storage_dir = storage_root.path().join("storage");
let first_socket_path = storage_root.path().join("foreground-first.sock");
let second_socket_path = storage_root.path().join("foreground-second.sock");
let config_dir = tempfile::tempdir_in("/tmp").unwrap();
let config_path = config_dir.path().join("settings.toml");
std::fs::write(&config_path, "_version = 1\n").unwrap();
let storage_log_path = storage_dir.join("logs").join("server.log");
std::fs::create_dir_all(storage_log_path.parent().unwrap()).unwrap();
std::fs::write(&storage_log_path, "stale pre-start log entry\n").unwrap();
let mut first = std::process::Command::new(env!("CARGO_BIN_EXE_fabro"));
apply_test_isolation(&mut first, home_dir.path());
first
.args(["server", "start", "--foreground"])
.arg("--storage-dir")
.arg(&storage_dir)
.arg("--bind")
.arg(&first_socket_path)
.arg("--config")
.arg(&config_path)
.stdin(Stdio::null())
.stdout(Stdio::null())
.stderr(Stdio::piped());
let first_child = first.spawn().expect("first foreground start should spawn");
wait_for_path(&storage_dir.join("server.json"));
wait_for_log_line(&storage_log_path, "API server started");
let marker = "marker-after-first-start\n";
{
use std::io::Write as _;
let mut file = std::fs::OpenOptions::new()
.append(true)
.open(&storage_log_path)
.unwrap();
file.write_all(marker.as_bytes()).unwrap();
}
let second_output = {
let mut second = std::process::Command::new(env!("CARGO_BIN_EXE_fabro"));
apply_test_isolation(&mut second, home_dir.path());
second
.args(["server", "start", "--foreground"])
.arg("--storage-dir")
.arg(&storage_dir)
.arg("--bind")
.arg(&second_socket_path)
.arg("--config")
.arg(&config_path)
.output()
.expect("second foreground start should run")
};
assert!(
!second_output.status.success(),
"second foreground start should fail:\nstdout:\n{}\nstderr:\n{}",
String::from_utf8_lossy(&second_output.stdout),
String::from_utf8_lossy(&second_output.stderr)
);
let second_stderr = String::from_utf8_lossy(&second_output.stderr);
assert!(
second_stderr.contains("timed out waiting for server lock"),
"expected lock timeout failure, got:\n{second_stderr}"
);
let storage_log = std::fs::read_to_string(&storage_log_path).unwrap_or_default();
assert!(
storage_log.contains(marker.trim_end()),
"expected second start to avoid retruncating the log, got:\n{}",
storage_log
);
assert!(
!storage_log.contains("stale pre-start log entry"),
"expected the first start to truncate stale log contents, got:\n{}",
storage_log
);
let stop_output = {
let mut stop = std::process::Command::new(env!("CARGO_BIN_EXE_fabro"));
apply_test_isolation(&mut stop, home_dir.path());
stop.args(["server", "stop", "--timeout", "0"])
.arg("--storage-dir")
.arg(&storage_dir)
.output()
.expect("server stop should run")
};
assert!(
stop_output.status.success(),
"server stop should succeed:\nstdout:\n{}\nstderr:\n{}",
String::from_utf8_lossy(&stop_output.stdout),
String::from_utf8_lossy(&stop_output.stderr)
);
let _ = first_child
.wait_with_output()
.expect("first child should exit");
}
#[test]
#[expect(
clippy::disallowed_methods,

98
lib/crates/fabro-cli/tests/it/cmd/server_status.rs
View file

@ -6,6 +6,29 @@ fn isolated_storage_dir() -> tempfile::TempDir {
root
}
fn wait_for_path(path: &std::path::Path) {
let deadline = std::time::Instant::now() + std::time::Duration::from_secs(5);
while std::time::Instant::now() < deadline {
if path.exists() {
return;
}
std::thread::sleep(std::time::Duration::from_millis(50));
}
panic!("timed out waiting for {}", path.display());
}
fn stop_pid(pid: u32) {
fabro_proc::sigterm(pid);
let deadline = std::time::Instant::now() + std::time::Duration::from_secs(5);
while std::time::Instant::now() < deadline {
if !fabro_proc::process_alive(pid) {
return;
}
std::thread::sleep(std::time::Duration::from_millis(50));
}
fabro_proc::sigkill(pid);
}
#[test]
fn help() {
let context = test_context!();
@ -47,3 +70,78 @@ fn status_when_not_running() {
Server is not running
");
}
#[test]
#[expect(
clippy::disallowed_methods,
reason = "This integration test moves a live daemon record on disk to simulate an unsupported legacy daemon upgrade."
)]
fn status_errors_when_only_a_legacy_running_server_record_exists() {
let home_dir = tempfile::tempdir_in("/tmp").unwrap();
let fabro_home = home_dir.path().join(".fabro");
let storage_dir = fabro_home.join("storage");
let socket_path = home_dir.path().join("legacy.sock");
let config_dir = tempfile::tempdir_in("/tmp").unwrap();
let config_path = config_dir.path().join("settings.toml");
std::fs::write(&config_path, "_version = 1\n").unwrap();
let start_output = {
let mut start = std::process::Command::new(env!("CARGO_BIN_EXE_fabro"));
fabro_test::apply_test_isolation(&mut start, home_dir.path());
start
.args(["server", "start", "--bind"])
.arg(&socket_path)
.arg("--config")
.arg(&config_path)
.output()
.expect("server start should run")
};
assert!(
start_output.status.success(),
"server start should succeed:\nstdout:\n{}\nstderr:\n{}",
String::from_utf8_lossy(&start_output.stdout),
String::from_utf8_lossy(&start_output.stderr)
);
let current_record = storage_dir.join("server.json");
wait_for_path(&current_record);
let legacy_record = fabro_home.join("server.json");
std::fs::rename(&current_record, &legacy_record).unwrap();
let status_output = {
let mut status = std::process::Command::new(env!("CARGO_BIN_EXE_fabro"));
fabro_test::apply_test_isolation(&mut status, home_dir.path());
status
.args(["server", "status"])
.output()
.expect("server status should run")
};
let pid = serde_json::from_str::<serde_json::Value>(
&std::fs::read_to_string(&legacy_record).unwrap(),
)
.unwrap()["pid"]
.as_u64()
.unwrap() as u32;
stop_pid(pid);
let _ = std::fs::remove_file(&legacy_record);
let _ = std::fs::remove_file(&socket_path);
assert!(
!status_output.status.success(),
"server status should fail when only the legacy record exists"
);
let stderr = String::from_utf8_lossy(&status_output.stderr);
assert!(
stderr.contains(&legacy_record.display().to_string()),
"expected stderr to mention the legacy record path, got:\n{stderr}"
);
assert!(
stderr.contains(&current_record.display().to_string()),
"expected stderr to mention the current record path, got:\n{stderr}"
);
assert!(
stderr.contains("legacy Fabro CLI"),
"expected stderr to instruct manual cleanup, got:\n{stderr}"
);
}

105
lib/crates/fabro-cli/tests/it/cmd/uninstall.rs
View file

@ -1,4 +1,5 @@
use std::fs;
use std::time::{Duration, Instant};
use fabro_test::{fabro_snapshot, test_context};
use serde_json::Value;
@ -37,6 +38,29 @@ fn command_with_no_fabro_home(context: &fabro_test::TestContext) -> assert_cmd::
cmd
}
fn wait_for_path(path: &std::path::Path) {
let deadline = Instant::now() + Duration::from_secs(5);
while Instant::now() < deadline {
if path.exists() {
return;
}
std::thread::sleep(Duration::from_millis(50));
}
panic!("timed out waiting for {}", path.display());
}
fn stop_pid(pid: u32) {
fabro_proc::sigterm(pid);
let deadline = Instant::now() + Duration::from_secs(5);
while Instant::now() < deadline {
if !fabro_proc::process_alive(pid) {
return;
}
std::thread::sleep(Duration::from_millis(50));
}
fabro_proc::sigkill(pid);
}
#[test]
fn not_installed_prints_message() {
let context = test_context!();
@ -164,3 +188,84 @@ fn not_installed_json() {
assert_eq!(value["status"].as_str(), Some("not_installed"));
}
#[test]
#[expect(
clippy::disallowed_methods,
reason = "This integration test moves a live daemon record on disk to simulate an unsupported legacy daemon upgrade."
)]
fn uninstall_yes_fails_when_only_a_legacy_running_server_record_exists() {
let home_dir = tempfile::tempdir_in("/tmp").unwrap();
let fabro_home = home_dir.path().join(".fabro");
let storage_dir = fabro_home.join("storage");
let socket_path = home_dir.path().join("legacy.sock");
let config_dir = tempfile::tempdir_in("/tmp").unwrap();
let config_path = config_dir.path().join("settings.toml");
std::fs::write(&config_path, "_version = 1\n").unwrap();
std::fs::create_dir_all(&fabro_home).unwrap();
std::fs::write(fabro_home.join("settings.toml"), "_version = 1\n").unwrap();
let start_output = {
let mut start = std::process::Command::new(env!("CARGO_BIN_EXE_fabro"));
fabro_test::apply_test_isolation(&mut start, home_dir.path());
start
.args(["server", "start", "--bind"])
.arg(&socket_path)
.arg("--config")
.arg(&config_path)
.output()
.expect("server start should run")
};
assert!(
start_output.status.success(),
"server start should succeed:\nstdout:\n{}\nstderr:\n{}",
String::from_utf8_lossy(&start_output.stdout),
String::from_utf8_lossy(&start_output.stderr)
);
let current_record = storage_dir.join("server.json");
wait_for_path(&current_record);
let legacy_record = fabro_home.join("server.json");
std::fs::rename(&current_record, &legacy_record).unwrap();
let pid = serde_json::from_str::<serde_json::Value>(
&std::fs::read_to_string(&legacy_record).unwrap(),
)
.unwrap()["pid"]
.as_u64()
.unwrap() as u32;
let uninstall_output = {
let mut uninstall = std::process::Command::new(env!("CARGO_BIN_EXE_fabro"));
fabro_test::apply_test_isolation(&mut uninstall, home_dir.path());
uninstall
.args(["uninstall", "--yes"])
.output()
.expect("uninstall should run")
};
stop_pid(pid);
let _ = std::fs::remove_file(&legacy_record);
let _ = std::fs::remove_file(&socket_path);
assert!(
!uninstall_output.status.success(),
"uninstall --yes should fail when only the legacy record exists"
);
let stderr = String::from_utf8_lossy(&uninstall_output.stderr);
assert!(
stderr.contains(&legacy_record.display().to_string()),
"expected stderr to mention the legacy record path, got:\n{stderr}"
);
assert!(
stderr.contains(&current_record.display().to_string()),
"expected stderr to mention the current record path, got:\n{stderr}"
);
assert!(
stderr.contains("legacy Fabro CLI"),
"expected stderr to instruct manual cleanup, got:\n{stderr}"
);
assert!(
fabro_home.exists(),
"uninstall should not remove ~/.fabro when the legacy daemon detector fires"
);
}

12
lib/crates/fabro-config/src/legacy_env.rs
View file

@ -1,12 +0,0 @@
//! Helpers for the deprecated `~/.fabro/.env` file.
//!
//! Fabro no longer reads `.env` automatically. The only remaining use for this
//! module is detecting the old file so CLI commands can print migration
//! warnings.
use std::path::PathBuf;
/// Return the path to the legacy `~/.fabro/.env` file.
pub fn legacy_env_file_path() -> PathBuf {
crate::Home::from_env().root().join(".env")
}

1
lib/crates/fabro-config/src/lib.rs
View file

@ -6,7 +6,6 @@ pub mod effective_settings;
pub mod envfile;
pub mod error;
pub mod home;
pub mod legacy_env;
pub mod load;
pub mod merge;
pub mod parse;

3
lib/crates/fabro-config/src/resolve/server.rs
View file

@ -14,6 +14,7 @@ use fabro_types::settings::server::{
use fabro_util::Home;
use super::{ResolveError, default_interp, parse_socket_addr, require_interp};
use crate::user::default_storage_dir;
pub fn resolve_server(layer: &ServerLayer, errors: &mut Vec<ResolveError>) -> ServerSettings {
let storage = resolve_storage(layer.storage.as_ref());
@ -57,7 +58,7 @@ fn resolve_storage(layer: Option<&ServerStorageLayer>) -> ServerStorageSettings
ServerStorageSettings {
root: layer
.and_then(|storage| storage.root.clone())
.unwrap_or_else(|| default_interp(Home::from_env().storage_dir())),
.unwrap_or_else(|| default_interp(default_storage_dir())),
}
}

9
lib/crates/fabro-config/src/storage.rs
View file

@ -39,6 +39,11 @@ impl Storage {
self.root.join("cache")
}
#[must_use]
pub fn slatedb_cache_dir(&self) -> PathBuf {
self.cache_dir().join("slatedb")
}
#[must_use]
pub fn secrets_path(&self) -> PathBuf {
self.root
@ -173,6 +178,10 @@ mod tests {
storage.cache_dir(),
std::path::Path::new("/tmp/fabro-data/cache")
);
assert_eq!(
storage.slatedb_cache_dir(),
std::path::Path::new("/tmp/fabro-data/cache/slatedb")
);
assert_eq!(
storage.secrets_path(),
std::path::Path::new("/tmp/fabro-data/vaults/default/secrets.json")

107
lib/crates/fabro-config/src/user.rs
View file

@ -4,9 +4,7 @@
//! `~/.fabro/settings.toml` file. Runtime types that used to be
//! re-exported from here live in `fabro_types::settings::user` now.
use std::collections::HashSet;
use std::path::{Path, PathBuf};
use std::sync::{Mutex, OnceLock};
use fabro_types::settings::SettingsLayer;
@ -15,23 +13,18 @@ use crate::home::Home;
use crate::load::load_settings_path;
pub const SETTINGS_CONFIG_FILENAME: &str = "settings.toml";
pub const LEGACY_USER_CONFIG_FILENAME: &str = "cli.toml";
pub const LEGACY_OLD_USER_CONFIG_FILENAME: &str = "user.toml";
pub const LEGACY_SERVER_CONFIG_FILENAME: &str = "server.toml";
pub const FABRO_CONFIG_ENV: &str = "FABRO_CONFIG";
static WARNED_LEGACY_USER_CONFIGS: OnceLock<Mutex<HashSet<PathBuf>>> = OnceLock::new();
pub fn default_settings_path() -> PathBuf {
Home::from_env().user_config()
}
pub fn default_socket_path() -> PathBuf {
Home::from_env().root().join("fabro.sock")
pub fn default_storage_dir() -> PathBuf {
Home::from_env().root().join("storage")
}
pub fn legacy_default_storage_root() -> PathBuf {
Home::from_env().root().to_path_buf()
pub fn default_socket_path() -> PathBuf {
Home::from_env().root().join("fabro.sock")
}
pub fn active_settings_path(path: Option<&Path>) -> PathBuf {
@ -47,37 +40,9 @@ fn active_settings_path_with_lookup(
.unwrap_or_else(default_settings_path)
}
pub fn legacy_user_config_path() -> Option<PathBuf> {
Some(Home::from_env().root().join(LEGACY_USER_CONFIG_FILENAME))
}
pub fn legacy_old_user_config_path() -> Option<PathBuf> {
Some(
Home::from_env()
.root()
.join(LEGACY_OLD_USER_CONFIG_FILENAME),
)
}
pub fn legacy_server_config_path() -> Option<PathBuf> {
Some(Home::from_env().root().join(LEGACY_SERVER_CONFIG_FILENAME))
}
fn warned_legacy_user_configs() -> &'static Mutex<HashSet<PathBuf>> {
WARNED_LEGACY_USER_CONFIGS.get_or_init(|| Mutex::new(HashSet::new()))
}
fn should_warn_about_legacy_user_config(path: &Path) -> bool {
warned_legacy_user_configs()
.lock()
.expect("legacy user config warning lock poisoned")
.insert(path.to_path_buf())
}
/// Load settings config from an explicit path or `~/.fabro/settings.toml`,
/// returning defaults if the default file doesn't exist. An explicit path that
/// doesn't exist is an error.
#[allow(clippy::print_stderr)]
pub fn load_settings_config(path: Option<&Path>) -> Result<SettingsLayer> {
if let Some(explicit) = path
.map(Path::to_path_buf)
@ -86,25 +51,7 @@ pub fn load_settings_config(path: Option<&Path>) -> Result<SettingsLayer> {
return load_v2_layer_from_path(&explicit);
}
for legacy_path in [
legacy_user_config_path(),
legacy_old_user_config_path(),
legacy_server_config_path(),
]
.into_iter()
.flatten()
{
if legacy_path.is_file() && should_warn_about_legacy_user_config(&legacy_path) {
let target = default_settings_path();
eprintln!(
"Warning: ignoring legacy config file {}. Rename it to {}.",
legacy_path.display(),
target.display()
);
}
}
let default = Home::from_env().root().join(SETTINGS_CONFIG_FILENAME);
let default = default_settings_path();
if default.is_file() {
load_v2_layer_from_path(&default)
} else {
@ -119,23 +66,10 @@ fn load_v2_layer_from_path(path: &Path) -> Result<SettingsLayer> {
#[cfg(test)]
mod tests {
use super::{
LEGACY_OLD_USER_CONFIG_FILENAME, LEGACY_SERVER_CONFIG_FILENAME,
LEGACY_USER_CONFIG_FILENAME, SETTINGS_CONFIG_FILENAME, active_settings_path_with_lookup,
default_settings_path, default_socket_path, legacy_old_user_config_path,
legacy_server_config_path, legacy_user_config_path, should_warn_about_legacy_user_config,
SETTINGS_CONFIG_FILENAME, active_settings_path_with_lookup, default_settings_path,
default_socket_path, default_storage_dir,
};
#[test]
fn should_warn_about_legacy_user_config_once_per_path() {
let dir = tempfile::tempdir().unwrap();
let first = dir.path().join("cli.toml");
let second = dir.path().join("other-cli.toml");
assert!(should_warn_about_legacy_user_config(&first));
assert!(!should_warn_about_legacy_user_config(&first));
assert!(should_warn_about_legacy_user_config(&second));
}
#[test]
fn settings_paths_use_expected_filenames() {
let home = dirs::home_dir().unwrap();
@ -144,33 +78,8 @@ mod tests {
default_settings_path(),
home.join(".fabro").join(SETTINGS_CONFIG_FILENAME)
);
assert_eq!(default_storage_dir(), home.join(".fabro/storage"));
assert_eq!(default_socket_path(), home.join(".fabro/fabro.sock"));
assert_eq!(
legacy_user_config_path(),
Some(home.join(".fabro").join(LEGACY_USER_CONFIG_FILENAME))
);
assert_eq!(
legacy_old_user_config_path(),
Some(home.join(".fabro").join(LEGACY_OLD_USER_CONFIG_FILENAME))
);
assert_eq!(
legacy_server_config_path(),
Some(home.join(".fabro").join(LEGACY_SERVER_CONFIG_FILENAME))
);
}
#[test]
fn should_warn_once_per_legacy_path_even_with_multiple_filenames() {
let dir = tempfile::tempdir().unwrap();
let user = dir.path().join("user.toml");
let server = dir.path().join("server.toml");
let cli = dir.path().join("cli.toml");
assert!(should_warn_about_legacy_user_config(&user));
assert!(!should_warn_about_legacy_user_config(&user));
assert!(should_warn_about_legacy_user_config(&server));
assert!(!should_warn_about_legacy_user_config(&server));
assert!(should_warn_about_legacy_user_config(&cli));
}
#[test]

9
lib/crates/fabro-config/tests/resolve_server.rs
View file

@ -1,4 +1,5 @@
use fabro_config::parse_settings_layer;
use fabro_config::user::default_storage_dir;
use fabro_types::settings::server::{
GithubIntegrationStrategy, IpAllowEntry, ObjectStoreSettings, ServerListenSettings,
};
@ -16,7 +17,7 @@ fn resolves_server_defaults_from_empty_settings() {
assert_eq!(
settings.storage.root.as_source(),
Home::from_env().storage_dir().to_string_lossy()
default_storage_dir().to_string_lossy()
);
assert!(settings.web.enabled);
assert_eq!(settings.web.url.as_source(), "http://localhost:3000");
@ -36,8 +37,7 @@ fn resolves_server_defaults_from_empty_settings() {
ObjectStoreSettings::Local { root } => {
assert_eq!(
root.as_source(),
Home::from_env()
.storage_dir()
default_storage_dir()
.join("objects")
.join("artifacts")
.to_string_lossy()
@ -51,8 +51,7 @@ fn resolves_server_defaults_from_empty_settings() {
ObjectStoreSettings::Local { root } => {
assert_eq!(
root.as_source(),
Home::from_env()
.storage_dir()
default_storage_dir()
.join("objects")
.join("slatedb")
.to_string_lossy()

2
lib/crates/fabro-server/src/serve.rs
View file

@ -354,7 +354,7 @@ where
let (object_store, slatedb_prefix, flush_interval, disk_cache) =
build_slatedb_store(&resolved_server_settings)?;
let cache_path = if disk_cache {
Some(data_dir.join("cache").join("slatedb"))
Some(storage.slatedb_cache_dir())
} else {
None
};

27
lib/crates/fabro-util/src/home.rs
View file

@ -44,26 +44,11 @@ impl Home {
self.root.join("settings.toml")
}
#[must_use]
pub fn server_config(&self) -> PathBuf {
self.root.join("settings.toml")
}
#[must_use]
pub fn certs_dir(&self) -> PathBuf {
self.root.join("certs")
}
#[must_use]
pub fn skills_dir(&self) -> PathBuf {
self.root.join("skills")
}
#[must_use]
pub fn storage_dir(&self) -> PathBuf {
self.root.join("storage")
}
#[must_use]
pub fn socket_path(&self) -> PathBuf {
self.root.join("fabro.sock")
@ -103,22 +88,10 @@ mod tests {
home.user_config(),
std::path::Path::new("/tmp/fabro-home/settings.toml")
);
assert_eq!(
home.server_config(),
std::path::Path::new("/tmp/fabro-home/settings.toml")
);
assert_eq!(
home.certs_dir(),
std::path::Path::new("/tmp/fabro-home/certs")
);
assert_eq!(
home.skills_dir(),
std::path::Path::new("/tmp/fabro-home/skills")
);
assert_eq!(
home.storage_dir(),
std::path::Path::new("/tmp/fabro-home/storage")
);
assert_eq!(
home.socket_path(),
std::path::Path::new("/tmp/fabro-home/fabro.sock")

2
lib/crates/fabro-workflow/src/run_lookup.rs
View file

@ -140,7 +140,7 @@ pub fn scratch_base(storage_dir: &Path) -> PathBuf {
}
pub fn default_scratch_base() -> PathBuf {
scratch_base(&fabro_util::Home::from_env().storage_dir())
scratch_base(&fabro_config::user::default_storage_dir())
}
fn scan_orphan_runs(base: &Path) -> Result<Vec<RunInfo>> {