mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-09-16 23:43:10 +00:00
fix(home): migrate all runtime consumers to use Home
Replaces every dirs::home_dir().join(".fabro") in production code with
Home::from_env() accessors, so FABRO_HOME is respected everywhere:
- fabro-config: project workflows dir, legacy .env path
- fabro-cli: logging dir, install root + cert defaults, upgrade check
state, workflow list, doctor/install/login legacy_env callers
- fabro-telemetry: tmp dir for spawn, anonymous ID file
- fabro-workflow: file resolver fallback in source resolution
- fabro-agent: skill discovery (session + default_skill_dirs API)
Remaining dirs::home_dir() calls are legitimate: tilde expansion,
display path shortening, non-.fabro paths (e.g. ~/.claude/skills).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
c8af7e0638
commit
7afcb5091a
15 changed files with 47 additions and 53 deletions
1
Cargo.lock
generated
1
Cargo.lock
generated
|
|
@ -1960,6 +1960,7 @@ dependencies = [
|
|||
"chrono",
|
||||
"dirs",
|
||||
"exec",
|
||||
"fabro-util",
|
||||
"fork",
|
||||
"git2",
|
||||
"insta",
|
||||
|
|
|
|||
|
|
@ -126,8 +126,9 @@ impl Session {
|
|||
let skill_dirs = if let Some(dirs) = &self.config.skill_dirs {
|
||||
dirs.clone()
|
||||
} else {
|
||||
let home = dirs::home_dir().map(|p| p.to_string_lossy().to_string());
|
||||
default_skill_dirs(home.as_deref(), self.config.git_root.as_deref())
|
||||
let skills_dir = fabro_util::Home::from_env().skills_dir();
|
||||
let skills_str = skills_dir.to_string_lossy().to_string();
|
||||
default_skill_dirs(Some(&skills_str), self.config.git_root.as_deref())
|
||||
};
|
||||
self.skills = discover_skills(self.sandbox.as_ref(), &skill_dirs).await;
|
||||
debug!(skill_count = self.skills.len(), "Skills discovered");
|
||||
|
|
|
|||
|
|
@ -205,11 +205,11 @@ pub fn format_skills_prompt_section(skills: &[Skill]) -> String {
|
|||
lines.join("\n")
|
||||
}
|
||||
|
||||
pub fn default_skill_dirs(home_dir: Option<&str>, git_root: Option<&str>) -> Vec<String> {
|
||||
pub fn default_skill_dirs(fabro_skills_dir: Option<&str>, git_root: Option<&str>) -> Vec<String> {
|
||||
let mut dirs = Vec::new();
|
||||
|
||||
if let Some(home) = home_dir {
|
||||
dirs.push(format!("{home}/.fabro/skills"));
|
||||
if let Some(skills_dir) = fabro_skills_dir {
|
||||
dirs.push(skills_dir.to_string());
|
||||
}
|
||||
|
||||
if let Some(root) = git_root {
|
||||
|
|
@ -517,7 +517,7 @@ name: trimmed
|
|||
|
||||
#[test]
|
||||
fn default_dirs_with_git_root() {
|
||||
let dirs = default_skill_dirs(Some("/home/user"), Some("/repo"));
|
||||
let dirs = default_skill_dirs(Some("/home/user/.fabro/skills"), Some("/repo"));
|
||||
assert_eq!(
|
||||
dirs,
|
||||
vec![
|
||||
|
|
@ -530,7 +530,7 @@ name: trimmed
|
|||
|
||||
#[test]
|
||||
fn default_dirs_without_git_root() {
|
||||
let dirs = default_skill_dirs(Some("/home/user"), None);
|
||||
let dirs = default_skill_dirs(Some("/home/user/.fabro/skills"), None);
|
||||
assert_eq!(dirs, vec!["/home/user/.fabro/skills"]);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -324,9 +324,10 @@ pub(crate) async fn run_doctor(
|
|||
.flatten()
|
||||
.filter(|path| path.exists())
|
||||
.collect::<Vec<_>>();
|
||||
let legacy_env_path = legacy_env::legacy_env_file_path()
|
||||
.ok()
|
||||
.filter(|path| path.exists());
|
||||
let legacy_env_path = {
|
||||
let p = legacy_env::legacy_env_file_path();
|
||||
p.exists().then_some(p)
|
||||
};
|
||||
|
||||
let mut report = CheckReport {
|
||||
title: "Fabro Doctor".to_string(),
|
||||
|
|
|
|||
|
|
@ -232,17 +232,18 @@ fn merge_server_settings(doc: &mut toml::Value, username: &str) -> Result<()> {
|
|||
);
|
||||
|
||||
let tls = ensure_table(api, "tls")?;
|
||||
let certs_dir = fabro_util::Home::from_env().certs_dir();
|
||||
tls.insert(
|
||||
"cert".to_string(),
|
||||
toml::Value::String("~/.fabro/certs/server.crt".to_string()),
|
||||
toml::Value::String(certs_dir.join("server.crt").to_string_lossy().to_string()),
|
||||
);
|
||||
tls.insert(
|
||||
"key".to_string(),
|
||||
toml::Value::String("~/.fabro/certs/server.key".to_string()),
|
||||
toml::Value::String(certs_dir.join("server.key").to_string_lossy().to_string()),
|
||||
);
|
||||
tls.insert(
|
||||
"ca".to_string(),
|
||||
toml::Value::String("~/.fabro/certs/ca.crt".to_string()),
|
||||
toml::Value::String(certs_dir.join("ca.crt").to_string_lossy().to_string()),
|
||||
);
|
||||
|
||||
Ok(())
|
||||
|
|
@ -582,12 +583,11 @@ pub(crate) async fn run_install(args: &InstallArgs, globals: &GlobalArgs) -> Res
|
|||
eprintln!(" {}", s.dim.apply_to("LLM providers and GitHub App."));
|
||||
eprintln!();
|
||||
|
||||
let fabro_dir = dirs::home_dir()
|
||||
.context("could not determine home directory")?
|
||||
.join(".fabro");
|
||||
let fabro_dir = fabro_util::Home::from_env().root().to_path_buf();
|
||||
std::fs::create_dir_all(&fabro_dir)?;
|
||||
|
||||
if let Ok(env_path) = legacy_env::legacy_env_file_path() {
|
||||
{
|
||||
let env_path = legacy_env::legacy_env_file_path();
|
||||
if env_path.exists() {
|
||||
eprintln!(
|
||||
" Warning: {} is no longer read by fabro server. This install will persist credentials in the server secret store instead.",
|
||||
|
|
@ -1043,13 +1043,13 @@ mod tests {
|
|||
|
||||
#[test]
|
||||
fn config_toml_has_tls_paths() {
|
||||
use std::path::PathBuf;
|
||||
let toml_str = format_config_toml("bob");
|
||||
let settings: fabro_types::Settings = toml::from_str(&toml_str).unwrap();
|
||||
let tls = settings.api.unwrap().tls.expect("tls should be set");
|
||||
assert_eq!(tls.cert, PathBuf::from("~/.fabro/certs/server.crt"));
|
||||
assert_eq!(tls.key, PathBuf::from("~/.fabro/certs/server.key"));
|
||||
assert_eq!(tls.ca, PathBuf::from("~/.fabro/certs/ca.crt"));
|
||||
let certs_dir = fabro_util::Home::from_env().certs_dir();
|
||||
assert_eq!(tls.cert, certs_dir.join("server.crt"));
|
||||
assert_eq!(tls.key, certs_dir.join("server.key"));
|
||||
assert_eq!(tls.ca, certs_dir.join("ca.crt"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
|
|||
|
|
@ -25,7 +25,8 @@ pub(super) async fn login_command(args: ProviderLoginArgs, globals: &GlobalArgs)
|
|||
vec![(env_var, key)]
|
||||
};
|
||||
|
||||
if let Ok(path) = legacy_env::legacy_env_file_path() {
|
||||
{
|
||||
let path = legacy_env::legacy_env_file_path();
|
||||
if path.exists() {
|
||||
eprintln!(
|
||||
" Warning: {} is no longer read by fabro server. Re-enter credentials with `fabro provider login` or `fabro secret set`.",
|
||||
|
|
|
|||
|
|
@ -384,10 +384,7 @@ pub(crate) fn spawn_upgrade_check(
|
|||
}
|
||||
|
||||
async fn check_and_print_notice() -> Result<()> {
|
||||
let Some(home) = dirs::home_dir() else {
|
||||
return Ok(());
|
||||
};
|
||||
let state_path = home.join(".fabro").join(LAST_CHECK_FILE);
|
||||
let state_path = fabro_util::Home::from_env().root().join(LAST_CHECK_FILE);
|
||||
|
||||
let current = Version::parse(env!("CARGO_PKG_VERSION"))?;
|
||||
|
||||
|
|
|
|||
|
|
@ -24,7 +24,7 @@ pub(super) fn list_command(_args: &WorkflowListArgs, globals: &GlobalArgs) -> Re
|
|||
|
||||
let fabro_root = resolve_fabro_root(&config_path, &config);
|
||||
let project_wf_dir = fabro_root.join("workflows");
|
||||
let user_wf_dir = dirs::home_dir().map(|h| h.join(".fabro").join("workflows"));
|
||||
let user_wf_dir = Some(fabro_util::Home::from_env().workflows_dir());
|
||||
|
||||
let workflows = list_workflows_detailed(Some(&project_wf_dir), user_wf_dir.as_deref());
|
||||
|
||||
|
|
|
|||
|
|
@ -16,8 +16,7 @@ pub(crate) fn init_tracing(
|
|||
let filter =
|
||||
EnvFilter::try_from_env("FABRO_LOG").unwrap_or_else(|_| EnvFilter::new(default_level));
|
||||
|
||||
let log_dir =
|
||||
dirs::home_dir().map_or_else(|| ".fabro/logs".into(), |h| h.join(".fabro").join("logs"));
|
||||
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()))?;
|
||||
|
|
|
|||
|
|
@ -6,10 +6,7 @@
|
|||
|
||||
use std::path::PathBuf;
|
||||
|
||||
use anyhow::{Context, Result};
|
||||
|
||||
/// Return the path to the legacy `~/.fabro/.env` file.
|
||||
pub fn legacy_env_file_path() -> Result<PathBuf> {
|
||||
let home = dirs::home_dir().context("could not determine home directory")?;
|
||||
Ok(home.join(".fabro").join(".env"))
|
||||
pub fn legacy_env_file_path() -> PathBuf {
|
||||
crate::Home::from_env().root().join(".env")
|
||||
}
|
||||
|
|
|
|||
|
|
@ -154,7 +154,7 @@ pub fn resolve_working_directory(settings: &Settings, caller_cwd: &Path) -> Path
|
|||
}
|
||||
|
||||
fn resolve_workflow_arg_from(arg: &Path, start_dir: &Path) -> anyhow::Result<PathBuf> {
|
||||
resolve_workflow_arg_impl(arg, start_dir, user_workflows_dir().as_deref())
|
||||
resolve_workflow_arg_impl(arg, start_dir, Some(&user_workflows_dir()))
|
||||
}
|
||||
|
||||
fn resolve_workflow_arg_impl(
|
||||
|
|
@ -237,8 +237,8 @@ fn resolve_user_workflow(user_workflows: Option<&Path>, name: &str, arg: &Path)
|
|||
}
|
||||
|
||||
/// Return the user-level workflows directory (`~/.fabro/workflows/`).
|
||||
fn user_workflows_dir() -> Option<PathBuf> {
|
||||
dirs::home_dir().map(|h| h.join(".fabro").join("workflows"))
|
||||
fn user_workflows_dir() -> PathBuf {
|
||||
crate::Home::from_env().workflows_dir()
|
||||
}
|
||||
|
||||
/// Metadata about a discovered workflow.
|
||||
|
|
|
|||
|
|
@ -18,6 +18,7 @@ base64.workspace = true
|
|||
chrono.workspace = true
|
||||
dirs.workspace = true
|
||||
exec.workspace = true
|
||||
fabro-util = { path = "../fabro-util" }
|
||||
fork.workspace = true
|
||||
git2.workspace = true
|
||||
mac_address.workspace = true
|
||||
|
|
|
|||
|
|
@ -1,11 +1,10 @@
|
|||
use anyhow::{Context, Result};
|
||||
use std::fs;
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::path::Path;
|
||||
use uuid::Uuid;
|
||||
|
||||
fn dot_id_path() -> Result<PathBuf> {
|
||||
let home = dirs::home_dir().context("could not determine home directory")?;
|
||||
Ok(home.join(".fabro").join(".id"))
|
||||
fn dot_id_path() -> std::path::PathBuf {
|
||||
fabro_util::Home::from_env().root().join(".id")
|
||||
}
|
||||
|
||||
fn read_existing_id(path: &Path) -> Option<String> {
|
||||
|
|
@ -16,7 +15,7 @@ fn read_existing_id(path: &Path) -> Option<String> {
|
|||
|
||||
/// Server: UUID persisted at ~/.fabro/.id
|
||||
pub fn load_or_create_server_id() -> Result<String> {
|
||||
let path = dot_id_path()?;
|
||||
let path = dot_id_path();
|
||||
|
||||
if let Some(id) = read_existing_id(&path) {
|
||||
return Ok(id);
|
||||
|
|
@ -37,7 +36,7 @@ pub fn load_or_create_server_id() -> Result<String> {
|
|||
|
||||
/// CLI: prefer existing ~/.fabro/.id, else MD5 of MAC address
|
||||
pub fn compute_cli_id() -> Result<String> {
|
||||
if let Some(id) = read_existing_id(&dot_id_path()?) {
|
||||
if let Some(id) = read_existing_id(&dot_id_path()) {
|
||||
return Ok(id);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -106,10 +106,7 @@ fn spawn_detached_windows(args: &[&str], env: &[(&str, &str)], env_remove: &[&st
|
|||
/// This is the shared pattern used by both analytics and panic senders.
|
||||
/// No-ops silently if the exe path can't be resolved or the temp file can't be written.
|
||||
pub fn spawn_fabro_subcommand(subcommand: &str, filename: &str, json: &[u8]) {
|
||||
let Some(home) = dirs::home_dir() else {
|
||||
return;
|
||||
};
|
||||
let tmp_dir = home.join(".fabro").join("tmp");
|
||||
let tmp_dir = fabro_util::Home::from_env().tmp_dir();
|
||||
if std::fs::create_dir_all(&tmp_dir).is_err() {
|
||||
return;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -102,9 +102,9 @@ pub(crate) fn resolve_workflow(request: ResolveWorkflowInput) -> anyhow::Result<
|
|||
workflow_toml_path: resolution.workflow_toml_path,
|
||||
dot_path: Some(resolution.dot_path.clone()),
|
||||
current_dir: Some(current_dir),
|
||||
file_resolver: Some(Arc::new(FilesystemFileResolver::new(
|
||||
dirs::home_dir().map(|home| home.join(".fabro")),
|
||||
))),
|
||||
file_resolver: Some(Arc::new(FilesystemFileResolver::new(Some(
|
||||
fabro_util::Home::from_env().root().to_path_buf(),
|
||||
)))),
|
||||
goal_override,
|
||||
working_directory,
|
||||
})
|
||||
|
|
@ -126,9 +126,9 @@ pub(crate) fn resolve_workflow(request: ResolveWorkflowInput) -> anyhow::Result<
|
|||
dot_path: None,
|
||||
current_dir: base_dir,
|
||||
file_resolver: has_base_dir.then(|| {
|
||||
Arc::new(FilesystemFileResolver::new(
|
||||
dirs::home_dir().map(|home| home.join(".fabro")),
|
||||
)) as Arc<dyn FileResolver>
|
||||
Arc::new(FilesystemFileResolver::new(Some(
|
||||
fabro_util::Home::from_env().root().to_path_buf(),
|
||||
))) as Arc<dyn FileResolver>
|
||||
}),
|
||||
goal_override,
|
||||
working_directory,
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue