diff --git a/crates/path-cli/src/cache.rs b/crates/path-cli/src/cache.rs index 2b2432e4..470f9f3c 100644 --- a/crates/path-cli/src/cache.rs +++ b/crates/path-cli/src/cache.rs @@ -9,7 +9,7 @@ use anyhow::{Context, Result, anyhow, bail}; use std::path::PathBuf; use toolpath::v1::Graph; -use crate::config::config_dir; +use std::path::Path; /// An entry surfaced by `list_cached`. #[derive(Debug, Clone)] @@ -21,16 +21,16 @@ pub(crate) struct CacheEntry { } /// The cache directory: `$CONFIG_DIR/documents/`. -pub(crate) fn cache_dir() -> Result { - Ok(config_dir()?.join(crate::config::DOCUMENTS_DIR_NAME)) +pub(crate) fn cache_dir(config_dir: &Path) -> PathBuf { + config_dir.join(crate::config::DOCUMENTS_DIR_NAME) } /// Path for a given cache id (does not check existence). -pub(crate) fn cache_path(id: &str) -> Result { +pub(crate) fn cache_path(config_dir: &Path, id: &str) -> Result { if id.is_empty() || id.contains('/') || id.contains('\\') || id.ends_with(".json") { bail!("invalid cache id: {id:?}"); } - Ok(cache_dir()?.join(format!("{id}.json"))) + Ok(cache_dir(config_dir).join(format!("{id}.json"))) } /// Write a toolpath document to the cache under `id`. Errors if the @@ -39,10 +39,15 @@ pub(crate) fn cache_path(id: &str) -> Result { /// Uses `O_CREAT | O_EXCL` (`create_new`) when `force == false` so the /// exists-check and the write are atomic — two concurrent `path import` /// invocations racing the same id can't silently stomp each other. -pub(crate) fn write_cached(id: &str, doc: &Graph, force: bool) -> Result { +pub(crate) fn write_cached( + config_dir: &Path, + id: &str, + doc: &Graph, + force: bool, +) -> Result { use std::io::Write; - let dir = cache_dir()?; + let dir = cache_dir(config_dir); std::fs::create_dir_all(&dir).with_context(|| format!("create {}", dir.display()))?; #[cfg(unix)] { @@ -50,7 +55,7 @@ pub(crate) fn write_cached(id: &str, doc: &Graph, force: bool) -> Result Result` string to a filesystem path. A ref is either a /// bare cache id (looks up `$CACHE_DIR/.json`) or a file path /// (contains `/` or `\\`, or ends with `.json`). -pub(crate) fn cache_ref(s: &str) -> Result { +pub(crate) fn cache_ref(config_dir: &Path, s: &str) -> Result { if s.contains('/') || s.contains('\\') || s.ends_with(".json") { let p = PathBuf::from(s); if !p.exists() { @@ -98,7 +103,7 @@ pub(crate) fn cache_ref(s: &str) -> Result { } return Ok(p); } - let p = cache_path(s)?; + let p = cache_path(config_dir, s)?; if !p.exists() { bail!( "cache entry {s} not found at {}; run `path cache ls` to see what's cached", @@ -108,8 +113,8 @@ pub(crate) fn cache_ref(s: &str) -> Result { Ok(p) } -pub(crate) fn list_cached() -> Result> { - let dir = cache_dir()?; +pub(crate) fn list_cached(config_dir: &Path) -> Result> { + let dir = cache_dir(config_dir); if !dir.exists() { return Ok(Vec::new()); } @@ -136,8 +141,8 @@ pub(crate) fn list_cached() -> Result> { Ok(out) } -pub(crate) fn remove_cached(id: &str) -> Result<()> { - let path = cache_path(id)?; +pub(crate) fn remove_cached(config_dir: &Path, id: &str) -> Result<()> { + let path = cache_path(config_dir, id)?; if !path.exists() { return Err(anyhow!("cache entry {id} not found")); } @@ -173,19 +178,11 @@ pub(crate) fn pathbase_cache_id(owner: &str, repo: &str, id: &str) -> String { #[cfg(test)] mod tests { use super::*; - use crate::config::{CONFIG_DIR_ENV, TEST_ENV_LOCK}; - fn with_cfg R, R>(f: F) -> R { - let temp = tempfile::tempdir().unwrap(); - let _g = TEST_ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); - unsafe { - std::env::set_var(CONFIG_DIR_ENV, temp.path()); - } - let result = f(temp.path()); - unsafe { - std::env::remove_var(CONFIG_DIR_ENV); - } - result + /// A config directory in a fresh tempdir. Dropping the `TempDir` + /// removes it. + fn config_dir_in_tempdir() -> tempfile::TempDir { + tempfile::tempdir().unwrap() } fn sample_doc() -> Graph { @@ -194,100 +191,94 @@ mod tests { #[test] fn write_and_read_cache_entry() { - with_cfg(|_| { - let doc = sample_doc(); - let p = write_cached("claude-abc", &doc, false).unwrap(); - assert!(p.exists()); - assert_eq!(p.file_name().unwrap(), "claude-abc.json"); - }); + let temp = config_dir_in_tempdir(); + let doc = sample_doc(); + let p = write_cached(temp.path(), "claude-abc", &doc, false).unwrap(); + assert!(p.exists()); + assert_eq!(p.file_name().unwrap(), "claude-abc.json"); } #[test] fn write_errors_if_exists_without_force() { - with_cfg(|_| { - let doc = sample_doc(); - write_cached("claude-abc", &doc, false).unwrap(); - let err = write_cached("claude-abc", &doc, false).unwrap_err(); - assert!(err.to_string().contains("already exists")); - }); + let temp = config_dir_in_tempdir(); + let doc = sample_doc(); + write_cached(temp.path(), "claude-abc", &doc, false).unwrap(); + let err = write_cached(temp.path(), "claude-abc", &doc, false).unwrap_err(); + assert!(err.to_string().contains("already exists")); } #[test] fn write_force_overwrites() { - with_cfg(|_| { - let doc = sample_doc(); - write_cached("claude-abc", &doc, false).unwrap(); - write_cached("claude-abc", &doc, true).unwrap(); - }); + let temp = config_dir_in_tempdir(); + let doc = sample_doc(); + write_cached(temp.path(), "claude-abc", &doc, false).unwrap(); + write_cached(temp.path(), "claude-abc", &doc, true).unwrap(); } #[test] fn cache_ref_finds_existing_cache_entry() { - with_cfg(|_| { - let doc = sample_doc(); - let p = write_cached("claude-abc", &doc, false).unwrap(); - let resolved = cache_ref("claude-abc").unwrap(); - assert_eq!(resolved, p); - }); + let temp = config_dir_in_tempdir(); + let doc = sample_doc(); + let p = write_cached(temp.path(), "claude-abc", &doc, false).unwrap(); + let resolved = cache_ref(temp.path(), "claude-abc").unwrap(); + assert_eq!(resolved, p); } #[test] fn cache_ref_returns_file_path_unchanged() { + let temp = config_dir_in_tempdir(); let tmp = tempfile::NamedTempFile::new().unwrap(); std::fs::write(tmp.path(), "{}").unwrap(); - let resolved = cache_ref(tmp.path().to_str().unwrap()).unwrap(); + let resolved = cache_ref(temp.path(), tmp.path().to_str().unwrap()).unwrap(); assert_eq!(resolved, tmp.path()); } #[test] fn cache_ref_errors_on_missing_id() { - with_cfg(|_| { - let err = cache_ref("does-not-exist").unwrap_err(); - assert!(err.to_string().contains("not found")); - }); + let temp = config_dir_in_tempdir(); + let err = cache_ref(temp.path(), "does-not-exist").unwrap_err(); + assert!(err.to_string().contains("not found")); } #[test] fn cache_path_rejects_slashes_and_json_suffix() { - assert!(cache_path("foo/bar").is_err()); - assert!(cache_path("foo.json").is_err()); - assert!(cache_path("").is_err()); + let temp = config_dir_in_tempdir(); + assert!(cache_path(temp.path(), "foo/bar").is_err()); + assert!(cache_path(temp.path(), "foo.json").is_err()); + assert!(cache_path(temp.path(), "").is_err()); } #[test] fn list_empty_when_dir_missing() { - with_cfg(|_| { - assert!(list_cached().unwrap().is_empty()); - }); + let temp = config_dir_in_tempdir(); + assert!(list_cached(temp.path()).unwrap().is_empty()); } #[test] fn list_and_remove_roundtrip() { - with_cfg(|_| { - let doc = sample_doc(); - write_cached("a", &doc, false).unwrap(); - write_cached("b", &doc, false).unwrap(); - let entries = list_cached().unwrap(); - assert_eq!(entries.len(), 2); - - remove_cached("a").unwrap(); - let entries = list_cached().unwrap(); - assert_eq!(entries.len(), 1); - assert_eq!(entries[0].id, "b"); - - assert!(remove_cached("a").is_err()); - }); + let temp = config_dir_in_tempdir(); + let doc = sample_doc(); + write_cached(temp.path(), "a", &doc, false).unwrap(); + write_cached(temp.path(), "b", &doc, false).unwrap(); + let entries = list_cached(temp.path()).unwrap(); + assert_eq!(entries.len(), 2); + + remove_cached(temp.path(), "a").unwrap(); + let entries = list_cached(temp.path()).unwrap(); + assert_eq!(entries.len(), 1); + assert_eq!(entries[0].id, "b"); + + assert!(remove_cached(temp.path(), "a").is_err()); } #[cfg(unix)] #[test] fn writes_file_with_0600() { use std::os::unix::fs::PermissionsExt; - with_cfg(|_| { - let p = write_cached("claude-abc", &sample_doc(), false).unwrap(); - let mode = std::fs::metadata(&p).unwrap().permissions().mode() & 0o777; - assert_eq!(mode, 0o600); - }); + let temp = config_dir_in_tempdir(); + let p = write_cached(temp.path(), "claude-abc", &sample_doc(), false).unwrap(); + let mode = std::fs::metadata(&p).unwrap().permissions().mode() & 0o777; + assert_eq!(mode, 0o600); } #[test] @@ -306,7 +297,8 @@ mod tests { #[test] fn make_id_result_survives_cache_path() { // Regression: make_id output must be accepted by cache_path. + let temp = config_dir_in_tempdir(); let id = make_id("pathbase", "trc_01H.json"); - assert!(cache_path(&id).is_ok()); + assert!(cache_path(temp.path(), &id).is_ok()); } } diff --git a/crates/path-cli/src/cmd_cache.rs b/crates/path-cli/src/cmd_cache.rs index 0561ab56..73212484 100644 --- a/crates/path-cli/src/cmd_cache.rs +++ b/crates/path-cli/src/cmd_cache.rs @@ -7,7 +7,7 @@ use anyhow::Result; use clap::Subcommand; #[cfg(not(target_os = "emscripten"))] -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use crate::cache::{list_cached, remove_cached}; use crate::config::Config; @@ -46,8 +46,8 @@ pub enum CacheOp { pub fn run(op: CacheOp, config: &Config) -> Result<()> { match op { - CacheOp::Ls => run_ls(), - CacheOp::Rm { id } => run_rm(&id, config), + CacheOp::Ls => run_ls(&config.config_dir()?), + CacheOp::Rm { id } => run_rm(&id, &config.config_dir()?), #[cfg(not(target_os = "emscripten"))] CacheOp::Sync { types, @@ -56,8 +56,8 @@ pub fn run(op: CacheOp, config: &Config) -> Result<()> { } } -fn run_ls() -> Result<()> { - let entries = list_cached()?; +fn run_ls(config_dir: &Path) -> Result<()> { + let entries = list_cached(config_dir)?; if entries.is_empty() { eprintln!("No cached documents. Run `path import ` to create one."); return Ok(()); @@ -68,16 +68,12 @@ fn run_ls() -> Result<()> { Ok(()) } -#[cfg_attr(target_os = "emscripten", expect(unused_variables))] -fn run_rm(id: &str, config: &Config) -> Result<()> { - remove_cached(id)?; +fn run_rm(id: &str, config_dir: &Path) -> Result<()> { + remove_cached(config_dir, id)?; // The artifact is still real — downgrade its manifest record to // "known, not cached" so the next sync can re-materialize it. #[cfg(not(target_os = "emscripten"))] - if let Err(e) = config - .config_dir() - .and_then(|dir| crate::sync::evict_cache_id(&dir, id)) - { + if let Err(e) = crate::sync::evict_cache_id(config_dir, id) { eprintln!("warning: sync manifest not updated: {e}"); } eprintln!("Removed {id}"); diff --git a/crates/path-cli/src/cmd_config.rs b/crates/path-cli/src/cmd_config.rs index a8e138ac..1eb9bbc8 100644 --- a/crates/path-cli/src/cmd_config.rs +++ b/crates/path-cli/src/cmd_config.rs @@ -12,7 +12,7 @@ use std::ffi::OsString; use std::io::Write; use std::path::Path; -use crate::config::{CONFIG_FILE_NAME, config_dir, home_dir, home_relative}; +use crate::config::{CONFIG_FILE_NAME, Config, home_relative}; #[derive(Subcommand, Debug)] pub enum ConfigOp { @@ -21,9 +21,9 @@ pub enum ConfigOp { Edit, } -pub fn run(op: ConfigOp) -> Result<()> { +pub fn run(op: ConfigOp, config: &Config) -> Result<()> { match op { - ConfigOp::Edit => edit(), + ConfigOp::Edit => edit(config), } } @@ -31,14 +31,14 @@ pub fn run(op: ConfigOp) -> Result<()> { /// behaves exactly like no file. const TEMPLATE: &str = "# Toolpath user configuration.\n# https://toolpath.net/cli/\n"; -fn edit() -> Result<()> { - let path = config_dir()?.join(CONFIG_FILE_NAME); +fn edit(config: &Config) -> Result<()> { + let path = config.config_dir()?.join(CONFIG_FILE_NAME); ensure_config_file(&path)?; let editor = resolve_editor(std::env::var_os("VISUAL"), std::env::var_os("EDITOR")); run_editor(&editor, &path)?; - let display = home_relative(&path, home_dir().as_deref()); + let display = home_relative(&path, config.home_dir().map(|p| p.as_path())); let text = std::fs::read_to_string(&path) .with_context(|| format!("failed to read {display} back after editing"))?; let rules = crate::share_config::validate_config_text(&text, &display).with_context(|| { diff --git a/crates/path-cli/src/cmd_export.rs b/crates/path-cli/src/cmd_export.rs index 50d1538a..3bb5d7d9 100644 --- a/crates/path-cli/src/cmd_export.rs +++ b/crates/path-cli/src/cmd_export.rs @@ -457,7 +457,7 @@ fn run_copilot( #[cfg(not(target_os = "emscripten"))] { - let path = load_path_doc(&input)?; + let path = load_path_doc(&config.config_dir()?, &input)?; match (project, output) { (Some(project_dir), None) => { let id = project_copilot(&path, &project_dir, config)?; @@ -670,7 +670,7 @@ fn run_claude( #[cfg(not(target_os = "emscripten"))] { - let path = load_path_doc(&input)?; + let path = load_path_doc(&config.config_dir()?, &input)?; let conversation = build_claude_conversation(&path)?; let jsonl = serialize_jsonl(&conversation)?; @@ -705,8 +705,8 @@ fn run_claude( } #[cfg(not(target_os = "emscripten"))] -fn load_path_doc(input: &str) -> Result { - let file = cache_ref(input)?; +fn load_path_doc(config_dir: &std::path::Path, input: &str) -> Result { + let file = cache_ref(config_dir, input)?; let json = std::fs::read_to_string(&file) .with_context(|| format!("Failed to read {}", file.display()))?; let doc = toolpath::v1::Graph::from_json(&json) @@ -798,7 +798,7 @@ fn run_gemini( }; let project_path = project_dir.to_string_lossy().to_string(); - let conversation = build_gemini_conversation(&input, &project_path)?; + let conversation = build_gemini_conversation(&config.config_dir()?, &input, &project_path)?; match (project, output) { (Some(_), None) => write_into_gemini_project(&conversation, &project_path, config)?, @@ -812,12 +812,13 @@ fn run_gemini( #[cfg(not(target_os = "emscripten"))] fn build_gemini_conversation( + config_dir: &std::path::Path, input: &str, project_path: &str, ) -> Result { use toolpath_convo::ConversationProjector; - let path = load_path_doc(input)?; + let path = load_path_doc(config_dir, input)?; let view = toolpath_convo::extract_conversation(&path); // The projector bakes `projectHash` and `directories` into the @@ -1028,7 +1029,7 @@ fn run_pi( }; let cwd_str = project_dir.to_string_lossy().to_string(); - let session = build_pi_session(&input, &cwd_str)?; + let session = build_pi_session(&config.config_dir()?, &input, &cwd_str)?; match (project, output) { (Some(_), None) => write_into_pi_project(&session, &cwd_str, config)?, @@ -1041,10 +1042,14 @@ fn run_pi( } #[cfg(not(target_os = "emscripten"))] -fn build_pi_session(input: &str, cwd: &str) -> Result { +fn build_pi_session( + config_dir: &std::path::Path, + input: &str, + cwd: &str, +) -> Result { use toolpath_convo::ConversationProjector; - let path = load_path_doc(input)?; + let path = load_path_doc(config_dir, input)?; let view = toolpath_convo::extract_conversation(&path); let projector = toolpath_pi::project::PiProjector::new().with_cwd(cwd.to_string()); @@ -1179,7 +1184,7 @@ fn run_codex( }; let cwd_str = project_dir.to_string_lossy().to_string(); - let session = build_codex_session(&input, &cwd_str)?; + let session = build_codex_session(&config.config_dir()?, &input, &cwd_str)?; match (project, output) { (Some(_), None) => write_into_codex_project(&session, config)?, @@ -1192,10 +1197,14 @@ fn run_codex( } #[cfg(not(target_os = "emscripten"))] -fn build_codex_session(input: &str, cwd: &str) -> Result { +fn build_codex_session( + config_dir: &std::path::Path, + input: &str, + cwd: &str, +) -> Result { use toolpath_convo::ConversationProjector; - let path = load_path_doc(input)?; + let path = load_path_doc(config_dir, input)?; let view = toolpath_convo::extract_conversation(&path); let projector = toolpath_codex::project::CodexProjector::new().with_cwd(cwd.to_string()); @@ -1438,7 +1447,7 @@ fn run_opencode( #[cfg(not(target_os = "emscripten"))] { - let path = load_path_doc(&input)?; + let path = load_path_doc(&config.config_dir()?, &input)?; match (project, output) { (Some(project_dir), None) => { let session = build_opencode_session(&path, Some(&project_dir))?; @@ -1684,7 +1693,7 @@ fn run_cursor( #[cfg(not(target_os = "emscripten"))] { - let path = load_path_doc(&input)?; + let path = load_path_doc(&config.config_dir()?, &input)?; match (project, output) { (Some(project_dir), None) => { let session = build_cursor_session(&path, Some(&project_dir), config)?; @@ -1959,7 +1968,7 @@ fn run_pathbase(args: PathbaseExportArgs, config: &Config) -> Result<()> { { use crate::cmd_pathbase::preflight_auth; - let file = cache_ref(&args.input)?; + let file = cache_ref(&config.config_dir()?, &args.input)?; let body = std::fs::read_to_string(&file) .with_context(|| format!("Failed to read {}", file.display()))?; let upload = PathbaseUploadArgs { @@ -2229,7 +2238,7 @@ mod tests { None, Some(output_path.clone()), false, - &Config::default(), + &config_with_home(temp.path()), ) .unwrap(); @@ -2278,7 +2287,7 @@ mod tests { None, None, false, - &Config::default(), + &config_with_home(temp.path()), ) .unwrap_err(); assert!(err.to_string().contains("single-path graph")); @@ -2294,7 +2303,7 @@ mod tests { None, None, false, - &Config::default(), + &config_with_home(temp.path()), ) .unwrap_err(); assert!(err.to_string().contains("parse") || err.to_string().contains("Failed")); @@ -2439,7 +2448,7 @@ mod tests { input_path.to_string_lossy().to_string(), Some(project), None, - &Config::default(), + &config_with_home(temp.path()), ) .expect_err("should reject multi-path graph"); assert!(err.to_string().contains("single-path graph")); @@ -2504,7 +2513,7 @@ mod tests { input_path.to_string_lossy().to_string(), None, Some(out_path.clone()), - &Config::default(), + &config_with_home(temp.path()), ) .expect("export gemini --output"); @@ -2670,7 +2679,7 @@ mod tests { input_path.to_string_lossy().to_string(), Some(project), None, - &Config::default(), + &config_with_home(temp.path()), ) .expect_err("should reject empty graph"); assert!(err.to_string().contains("single-path")); @@ -2726,7 +2735,7 @@ mod tests { input_path.to_string_lossy().to_string(), None, Some(out_path.clone()), - &Config::default(), + &config_with_home(temp.path()), ) .expect("export pi --output"); @@ -2792,7 +2801,7 @@ mod tests { input_path.to_string_lossy().to_string(), None, Some(out_path.clone()), - &Config::default(), + &config_with_home(temp.path()), ) .expect("export codex --output"); @@ -2934,7 +2943,7 @@ mod tests { input_path.to_string_lossy().to_string(), Some(project), None, - &Config::default(), + &config_with_home(temp.path()), ) .expect_err("should reject empty graph"); assert!(err.to_string().contains("single-path")); @@ -3059,7 +3068,7 @@ mod tests { input_path.to_string_lossy().to_string(), None, Some(out_path.clone()), - &Config::default(), + &config_with_home(temp.path()), ) .unwrap(); @@ -3161,7 +3170,7 @@ mod tests { input_path.to_string_lossy().to_string(), None, None, - &Config::default(), + &config_with_home(temp.path()), ) .unwrap_err(); assert!(err.to_string().contains("single-path")); diff --git a/crates/path-cli/src/cmd_import.rs b/crates/path-cli/src/cmd_import.rs index 10a35015..abffcfe7 100644 --- a/crates/path-cli/src/cmd_import.rs +++ b/crates/path-cli/src/cmd_import.rs @@ -226,26 +226,30 @@ fn emit( }; println!("{}", json); } else { + let config_dir = config.config_dir()?; // The implicit sync in `path query` fills the cache under // these same IDs; re-importing an artifact whose record is // still fresh is a no-op, not an exists-error. #[cfg(not(target_os = "emscripten"))] if !force && let Some(stub) = &d.provenance - && crate::sync::record_is_current(config, stub, &d.cache_id) + && crate::sync::record_is_current(&config_dir, stub, &d.cache_id) { - println!("{}", crate::cache::cache_path(&d.cache_id)?.display()); + println!( + "{}", + crate::cache::cache_path(&config_dir, &d.cache_id)?.display() + ); eprintln!( "{} is already up to date (pass --force to re-derive)", d.cache_id ); continue; } - let path = write_cached(&d.cache_id, &d.doc, force)?; + let path = write_cached(&config_dir, &d.cache_id, &d.doc, force)?; println!("{}", path.display()); #[cfg(not(target_os = "emscripten"))] if let Some(stub) = &d.provenance - && let Err(e) = crate::sync::record_artifact(config, stub, &d.cache_id) + && let Err(e) = crate::sync::record_artifact(&config_dir, stub, &d.cache_id) { eprintln!("warning: sync manifest not updated: {e}"); } diff --git a/crates/path-cli/src/cmd_query.rs b/crates/path-cli/src/cmd_query.rs index 9ff5470b..8fad5159 100644 --- a/crates/path-cli/src/cmd_query.rs +++ b/crates/path-cli/src/cmd_query.rs @@ -119,7 +119,9 @@ pub fn run(args: QueryArgs, pretty: bool, config: &Config) -> Result<()> { // pretty on a TTY or when the global `--pretty` flag is set. let compact = args.compact || (!pretty && !std::io::stdout().is_terminal()); + let config_dir = config.config_dir()?; crate::query::run( + &config_dir, &scope, &args.filter, compact, diff --git a/crates/path-cli/src/cmd_resume.rs b/crates/path-cli/src/cmd_resume.rs index 6521eb72..f8e977d0 100644 --- a/crates/path-cli/src/cmd_resume.rs +++ b/crates/path-cli/src/cmd_resume.rs @@ -231,7 +231,7 @@ pub(crate) fn resolve_input( let cache_id = crate::cache::pathbase_cache_id(&ref_.owner, &ref_.repo, &ref_.id); if !args.force && !args.no_cache - && let Ok(cache_path) = crate::cache::cache_path(&cache_id) + && let Ok(cache_path) = crate::cache::cache_path(&config.config_dir()?, &cache_id) && cache_path.exists() { let json = std::fs::read_to_string(&cache_path) @@ -245,7 +245,12 @@ pub(crate) fn resolve_input( // force=true here: we either short-circuited above // (cache miss) or the user explicitly passed --force, // and either way we want the new bytes to land. - crate::cache::write_cached(&derived.cache_id, &derived.doc, true)?; + crate::cache::write_cached( + &config.config_dir()?, + &derived.cache_id, + &derived.doc, + true, + )?; eprintln!("Resolved {} → {}", raw, derived.cache_id); } derived.doc @@ -257,13 +262,16 @@ pub(crate) fn resolve_input( .map_err(|e| anyhow::anyhow!("not a valid toolpath document: {}", e))? } Shape::CacheId(id) => { - let file = crate::cache::cache_ref(id).map_err(|e| { - anyhow::anyhow!( - "couldn't resolve `{}` as a URL, file path, or cache id: {}", - raw, - e - ) - })?; + let file = config + .config_dir() + .and_then(|dir| crate::cache::cache_ref(&dir, id)) + .map_err(|e| { + anyhow::anyhow!( + "couldn't resolve `{}` as a URL, file path, or cache id: {}", + raw, + e + ) + })?; let json = std::fs::read_to_string(&file) .with_context(|| format!("read {}", file.display()))?; Graph::from_json(&json) diff --git a/crates/path-cli/src/cmd_share.rs b/crates/path-cli/src/cmd_share.rs index 8c377496..871288d8 100644 --- a/crates/path-cli/src/cmd_share.rs +++ b/crates/path-cli/src/cmd_share.rs @@ -781,20 +781,21 @@ fn share_explicit( ), (false, _) => None, }; + let config_dir = config.config_dir()?; // Fast path: when the manifest shows this exact source state is // already in the cache, upload the cached doc instead of re-deriving // — a derive would reproduce it byte-for-byte anyway. if !args.no_cache && let Some(cache_id) = crate::sync::fresh_cache_id( - config, + &config_dir, &providers::harness_bundle(config), harness, project.as_deref(), session, ) { - let doc_path = crate::cache::cache_path(&cache_id)?; + let doc_path = crate::cache::cache_path(&config_dir, &cache_id)?; let body = std::fs::read_to_string(&doc_path) .with_context(|| format!("Failed to read {}", doc_path.display()))?; eprintln!( @@ -829,9 +830,9 @@ fn share_explicit( // the upload uses the fresh body, not the cache. Always // overwrite so cache and upload agree (use `--no-cache` to skip // the cache write entirely). - let path = crate::cache::write_cached(&derived.cache_id, &derived.doc, true)?; + let path = crate::cache::write_cached(&config_dir, &derived.cache_id, &derived.doc, true)?; if let Some(stub) = &derived.provenance - && let Err(e) = crate::sync::record_artifact(config, stub, &derived.cache_id) + && let Err(e) = crate::sync::record_artifact(&config_dir, stub, &derived.cache_id) { eprintln!("warning: sync manifest not updated: {e}"); } diff --git a/crates/path-cli/src/config.rs b/crates/path-cli/src/config.rs index 3e0f51cd..c5fa3383 100644 --- a/crates/path-cli/src/config.rs +++ b/crates/path-cli/src/config.rs @@ -152,15 +152,6 @@ impl Config { } } -/// The configured toolpath config directory (default `~/.toolpath`, -/// overridable via `$TOOLPATH_CONFIG_DIR`). -/// -/// Transitional: loads a [`Config`] per call. New code takes `&Config` -/// as a parameter and calls [`Config::config_dir`]. -pub(crate) fn config_dir() -> Result { - Config::load()?.config_dir() -} - /// Cross-platform `$HOME` lookup matching the providers' internal helpers. /// Returns `None` only when neither `$HOME` nor `$USERPROFILE` is set. pub(crate) fn home_dir() -> Option { @@ -332,21 +323,6 @@ mod tests { assert!(err.to_string().contains("$HOME")); } - /// The transitional free function honors the env override - /// end-to-end. - #[test] - fn config_dir_honors_override() { - let _g = TEST_ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); - unsafe { - std::env::set_var(CONFIG_DIR_ENV, "/tmp/test-toolpath"); - } - let dir = config_dir().unwrap(); - unsafe { - std::env::remove_var(CONFIG_DIR_ENV); - } - assert_eq!(dir, PathBuf::from("/tmp/test-toolpath")); - } - #[test] fn home_relative_strips_home_prefix() { let home = std::path::Path::new("/Users/alex"); diff --git a/crates/path-cli/src/lib.rs b/crates/path-cli/src/lib.rs index 9f559d0f..f26448d3 100644 --- a/crates/path-cli/src/lib.rs +++ b/crates/path-cli/src/lib.rs @@ -158,7 +158,7 @@ pub fn run() -> Result<()> { #[cfg(not(target_os = "emscripten"))] Commands::Auth { op } => cmd_auth::run(op, &config), #[cfg(not(target_os = "emscripten"))] - Commands::Config { op } => cmd_config::run(op), + Commands::Config { op } => cmd_config::run(op, &config), Commands::P { command } => cmd_p::run(command, cli.pretty, &config), } } diff --git a/crates/path-cli/src/query/mod.rs b/crates/path-cli/src/query/mod.rs index 26771916..3b6d77fb 100644 --- a/crates/path-cli/src/query/mod.rs +++ b/crates/path-cli/src/query/mod.rs @@ -51,6 +51,7 @@ pub struct Scope { /// Anything the planner can't prove decomposable falls back to the whole-array /// path, which is still lean — the step values are held once, not re-serialized. pub fn run( + config_dir: &FsPath, scope: &Scope, code: &str, compact: bool, @@ -67,7 +68,7 @@ pub fn run( // raw `StdoutLock` is line-buffered (a syscall per line). let stdout = std::io::stdout(); let mut out = std::io::BufWriter::new(stdout.lock()); - execute_plan(scope, &plan, code, compact, raw, &mut out)?; + execute_plan(config_dir, scope, &plan, code, compact, raw, &mut out)?; out.flush().context("flush stdout") } @@ -78,6 +79,7 @@ pub fn run( /// parallelizes parsing only. #[cfg(not(target_os = "emscripten"))] fn execute_plan( + config_dir: &FsPath, scope: &Scope, plan: &plan::Plan, code: &str, @@ -87,11 +89,12 @@ fn execute_plan( ) -> Result<()> { match plan { plan::Plan::Slurp => filter::execute(plan, code, compact, raw, out, |emit| { - stream_files(scope, emit) + stream_files(config_dir, scope, emit) }), plan::Plan::PerFileStream => { filter::compile_check(code)?; for_each_file( + config_dir, scope, |steps| filter::render_file(code, steps, compact, raw), |bytes| { @@ -105,6 +108,7 @@ fn execute_plan( let mut partials: Vec = Vec::new(); let mut saw_file = false; for_each_file( + config_dir, scope, |steps| filter::partials_file(code, steps), |bytes| { @@ -122,6 +126,7 @@ fn execute_plan( /// plan runs on the sequential engine. #[cfg(target_os = "emscripten")] fn execute_plan( + config_dir: &FsPath, scope: &Scope, plan: &plan::Plan, code: &str, @@ -130,7 +135,7 @@ fn execute_plan( out: &mut dyn Write, ) -> Result<()> { filter::execute(plan, code, compact, raw, out, |emit| { - stream_files(scope, emit) + stream_files(config_dir, scope, emit) }) } @@ -143,6 +148,7 @@ fn execute_plan( /// sequential scan. #[cfg(not(target_os = "emscripten"))] fn for_each_file( + config_dir: &FsPath, scope: &Scope, per_file: impl Fn(Vec) -> Result + Sync, mut consume: impl FnMut(T) -> Result<()>, @@ -152,7 +158,7 @@ fn for_each_file( let kind_sel = scope.kind.as_deref().map(kinds::parse_kind_selector); let project = scope.project.as_deref().map(canonicalize_or_self); let project_under = scope.project_under.as_deref().map(canonicalize_or_self); - let sources = select_files(scope)?; + let sources = select_files(config_dir, scope)?; let chunk = rayon::current_num_threads().max(1) * 2; for batch in sources.chunks(chunk) { @@ -211,11 +217,15 @@ impl DocSource { /// thread because jaq values are `Rc`-based, not `Send`. Chunking keeps /// output (and per-file warnings) byte-identical to a sequential scan while /// holding at most one chunk of parsed documents in memory. -fn stream_files(scope: &Scope, emit: &mut dyn FnMut(Val) -> Result<()>) -> Result<()> { +fn stream_files( + config_dir: &FsPath, + scope: &Scope, + emit: &mut dyn FnMut(Val) -> Result<()>, +) -> Result<()> { let kind_sel = scope.kind.as_deref().map(kinds::parse_kind_selector); let project = scope.project.as_deref().map(canonicalize_or_self); let project_under = scope.project_under.as_deref().map(canonicalize_or_self); - let sources = select_files(scope)?; + let sources = select_files(config_dir, scope)?; #[cfg(not(target_os = "emscripten"))] { @@ -293,7 +303,7 @@ fn emit_wrapped( /// that is, when `--source`/`--id` is present, or when no `--input` is given /// at all (the default whole-cache scan). `--input` files are appended in the /// order given. -fn select_files(scope: &Scope) -> Result> { +fn select_files(config_dir: &FsPath, scope: &Scope) -> Result> { let mut sources = Vec::new(); let restrict = scope.source.is_some() || !scope.ids.is_empty(); @@ -310,7 +320,7 @@ fn select_files(scope: &Scope) -> Result> { // dropped. A `--source`/default scan is not explicit (skip-warn). let by_id = id_set.is_some(); let mut seen_ids: HashSet = HashSet::new(); - for entry in crate::cache::list_cached()? { + for entry in crate::cache::list_cached(config_dir)? { if let Some(ids) = &id_set && !ids.contains(entry.id.as_str()) { @@ -623,7 +633,7 @@ mod tests { project_under: None, kind: None, }; - let files = select_files(&scope).unwrap(); + let files = select_files(FsPath::new("/nonexistent"), &scope).unwrap(); assert_eq!(files.len(), 2); // The full path as given: inputs sharing a basename stay distinct. assert_eq!(files[0].cache_id, "/tmp/some.json"); diff --git a/crates/path-cli/src/sync/engine.rs b/crates/path-cli/src/sync/engine.rs index 7f673401..c72d811d 100644 --- a/crates/path-cli/src/sync/engine.rs +++ b/crates/path-cli/src/sync/engine.rs @@ -11,7 +11,7 @@ use std::path::{Path, PathBuf}; use super::sources::{self, ArtifactSource}; use crate::artifact::{ArtifactRef, ArtifactType}; use crate::cache::write_cached; -use crate::config::{Config, MANIFEST_FILE_NAME, MANIFEST_LOCK_FILE_NAME}; +use crate::config::{MANIFEST_FILE_NAME, MANIFEST_LOCK_FILE_NAME}; use crate::harness::HarnessBundle; /// How many manifest writes accumulate before a mid-run checkpoint. @@ -133,15 +133,14 @@ pub(crate) fn sync_bundle( /// artifact needs nothing — no read, no scope check. All-`None` stamps /// mean freshness is unknowable; only a real stamp can vouch /// (mirrors `record_is_current`). -fn is_unchanged(rec: Option<&SyncRecord>, artifact: &ArtifactRef) -> bool { +fn is_unchanged(config_dir: &Path, rec: Option<&SyncRecord>, artifact: &ArtifactRef) -> bool { rec.is_some_and(|rec| { (rec.modified.is_some() || rec.size.is_some()) && rec.modified == artifact.modified && rec.size == artifact.size - && rec - .cache_id - .as_deref() - .is_some_and(|id| crate::cache::cache_path(id).is_ok_and(|p| p.exists())) + && rec.cache_id.as_deref().is_some_and(|id| { + crate::cache::cache_path(config_dir, id).is_ok_and(|p| p.exists()) + }) }) } @@ -193,7 +192,12 @@ fn sync_artifacts( // progress denominator and the loop's skip decision. let order: Vec<(&ArtifactRef, bool)> = newest_first(artifacts) .into_iter() - .map(|artifact| (artifact, is_unchanged(records.get(&artifact.id), artifact))) + .map(|artifact| { + ( + artifact, + is_unchanged(config_dir, records.get(&artifact.id), artifact), + ) + }) .collect(); let pending_total = order.iter().filter(|(_, unchanged)| !unchanged).count(); observer.begin(artifact_type, pending_total); @@ -264,7 +268,7 @@ fn sync_artifacts( // force: sync owns refresh semantics — a re-sync or a // prior manual `p import` of the same session must not // error on the existing cache entry. - write_cached(&derived.cache_id, &derived.doc, true)?; + write_cached(config_dir, &derived.cache_id, &derived.doc, true)?; stage( &mut writes, SyncRecord { @@ -304,12 +308,11 @@ fn sync_artifacts( /// Record an externally-derived cache write (`p import`, `share`) in /// the manifest, so sync doesn't re-derive what was just written. pub(crate) fn record_artifact( - config: &Config, + config_dir: &Path, artifact: &ArtifactRef, cache_id: &str, ) -> Result<()> { - let config_dir = config.config_dir()?; - update_manifest(&config_dir, |manifest| { + update_manifest(config_dir, |manifest| { manifest .entry(artifact.artifact_type.name().to_string()) .or_default() @@ -329,11 +332,8 @@ pub(crate) fn record_artifact( /// Whether the manifest already records exactly this artifact state /// under exactly this cache entry, with the doc present — i.e. a /// write would reproduce what's already there. -pub(crate) fn record_is_current(config: &Config, artifact: &ArtifactRef, cache_id: &str) -> bool { - let Ok(config_dir) = config.config_dir() else { - return false; - }; - let Ok(manifest) = load_manifest(&config_dir) else { +pub(crate) fn record_is_current(config_dir: &Path, artifact: &ArtifactRef, cache_id: &str) -> bool { + let Ok(manifest) = load_manifest(config_dir) else { return false; }; manifest @@ -346,7 +346,7 @@ pub(crate) fn record_is_current(config: &Config, artifact: &ArtifactRef, cache_i && (rec.modified.is_some() || rec.size.is_some()) && rec.modified == artifact.modified && rec.size == artifact.size - && crate::cache::cache_path(cache_id).is_ok_and(|p| p.exists()) + && crate::cache::cache_path(config_dir, cache_id).is_ok_and(|p| p.exists()) }) } @@ -356,14 +356,13 @@ pub(crate) fn record_is_current(config: &Config, artifact: &ArtifactRef, cache_i /// Used by `share` to upload straight from the cache. The stat /// targets one artifact directly — no enumeration of its siblings. pub(crate) fn fresh_cache_id( - config: &Config, + config_dir: &Path, bundle: &HarnessBundle, artifact_type: ArtifactType, project: Option<&str>, id: &str, ) -> Option { - let config_dir = config.config_dir().ok()?; - let manifest = load_manifest(&config_dir).ok()?; + let manifest = load_manifest(config_dir).ok()?; let rec = manifest.get(artifact_type.name())?.get(id)?; let cache_id = rec.cache_id.clone()?; let (modified, size) = sources::source_for(bundle, artifact_type)?.stamp(project, id)?; @@ -372,7 +371,7 @@ pub(crate) fn fresh_cache_id( ((rec.modified.is_some() || rec.size.is_some()) && rec.modified == modified && rec.size == size - && crate::cache::cache_path(&cache_id).is_ok_and(|p| p.exists())) + && crate::cache::cache_path(config_dir, &cache_id).is_ok_and(|p| p.exists())) .then_some(cache_id) } @@ -470,29 +469,15 @@ fn save_manifest(config_dir: &Path, manifest: &Manifest) -> Result<()> { #[cfg(test)] mod tests { use super::*; - use crate::config::{CONFIG_DIR_ENV, TEST_ENV_LOCK}; use std::path::Path; - /// Run `f` with `$TOOLPATH_CONFIG_DIR` pinned to `/.toolpath`; - /// `f` receives the tempdir root for building provider fixtures and - /// the config directory itself. The variable stays set because - /// the cache still reads it. + /// Run `f` with a config directory at `/.toolpath`; `f` + /// receives the tempdir root for building provider fixtures and the + /// config directory itself. fn with_cfg R, R>(f: F) -> R { - let _g = TEST_ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let temp = tempfile::tempdir().unwrap(); let config_root = temp.path().join(".toolpath"); - let prev = std::env::var_os(CONFIG_DIR_ENV); - unsafe { - std::env::set_var(CONFIG_DIR_ENV, &config_root); - } - let result = f(temp.path(), &config_root); - unsafe { - match prev { - Some(v) => std::env::set_var(CONFIG_DIR_ENV, v), - None => std::env::remove_var(CONFIG_DIR_ENV), - } - } - result + f(temp.path(), &config_root) } fn write_claude_session(home: &Path, project_slug: &str, session: &str, prompt: &str) { @@ -519,8 +504,8 @@ mod tests { } } - fn cached_step_count(cache_id: &str) -> usize { - let path = crate::cache::cache_path(cache_id).unwrap(); + fn cached_step_count(config_dir: &Path, cache_id: &str) -> usize { + let path = crate::cache::cache_path(config_dir, cache_id).unwrap(); let json = std::fs::read_to_string(path).unwrap(); let doc = toolpath::v1::Graph::from_json(&json).unwrap(); doc.single_path().map(|p| p.steps.len()).unwrap_or(0) @@ -628,7 +613,9 @@ mod tests { .as_deref() .expect("synced record is materialized"); assert!( - crate::cache::cache_path(cache_id).unwrap().exists(), + crate::cache::cache_path(config_dir, cache_id) + .unwrap() + .exists(), "cache doc must exist for {cache_id}" ); @@ -653,7 +640,7 @@ mod tests { .cache_id .clone() .expect("synced record is materialized"); - let steps_before = cached_step_count(&cache_id); + let steps_before = cached_step_count(config_dir, &cache_id); // Session continues: a later user turn lands in the file, // changing its size (and mtime). @@ -678,7 +665,7 @@ mod tests { (0, 1, 0, 0) ); assert!( - cached_step_count(&cache_id) > steps_before, + cached_step_count(config_dir, &cache_id) > steps_before, "re-derived doc must contain the appended turn" ); }); @@ -757,7 +744,7 @@ mod tests { .cache_id .clone() .unwrap(); - let steps_before = cached_step_count(&cache_id); + let steps_before = cached_step_count(config_dir, &cache_id); // The session rotates: a successor file whose first entry // carries the predecessor's sessionId (the bridge). @@ -787,7 +774,7 @@ mod tests { "successor segments are not separate artifacts" ); assert!( - cached_step_count(&cache_id) > steps_before, + cached_step_count(config_dir, &cache_id) > steps_before, "post-rotation turns must reach the cached doc" ); @@ -846,13 +833,8 @@ mod tests { let artifact = derived.provenance.as_ref().unwrap(); assert_eq!(artifact.id, "sess-aaa"); assert!(artifact.modified.is_some() && artifact.size.is_some()); - crate::cache::write_cached(&derived.cache_id, &derived.doc, true).unwrap(); - let config = Config { - home: Some(home.to_path_buf()), - toolpath_config_dir: Some(config_dir.to_path_buf()), - ..Config::default() - }; - record_artifact(&config, artifact, &derived.cache_id).unwrap(); + crate::cache::write_cached(config_dir, &derived.cache_id, &derived.doc, true).unwrap(); + record_artifact(config_dir, artifact, &derived.cache_id).unwrap(); // The import's stamp must match sync's own enumeration. let (_, outcome) = @@ -1034,7 +1016,7 @@ mod tests { .unwrap(); // `p cache rm`: doc removed, record downgraded to known. - crate::cache::remove_cached(&cache_id).unwrap(); + crate::cache::remove_cached(config_dir, &cache_id).unwrap(); evict_cache_id(config_dir, &cache_id).unwrap(); assert!( load_manifest(config_dir).unwrap()["claude"]["sess-aaa"] @@ -1047,7 +1029,9 @@ mod tests { [0]; assert_eq!((outcome.new, outcome.updated), (0, 1)); assert!( - crate::cache::cache_path(&cache_id).unwrap().exists(), + crate::cache::cache_path(config_dir, &cache_id) + .unwrap() + .exists(), "evicted artifact re-materializes" ); }); @@ -1066,7 +1050,7 @@ mod tests { // Doc deleted behind the CLI's back: the record still claims // materialization, but sync verifies the doc exists. - let doc = crate::cache::cache_path(&cache_id).unwrap(); + let doc = crate::cache::cache_path(config_dir, &cache_id).unwrap(); std::fs::remove_file(&doc).unwrap(); let (_, outcome) = sync_bundle(config_dir, &bundle, &[ArtifactType::Claude], None, &mut ()).unwrap() @@ -1081,15 +1065,11 @@ mod tests { with_cfg(|home, config_dir| { write_claude_session(home, "-test-project", "sess-aaa", "Add a feature"); let bundle = claude_bundle(home); - let config = &Config { - toolpath_config_dir: Some(config_dir.to_path_buf()), - ..Config::default() - }; // Nothing synced yet: no fresh copy. assert!( fresh_cache_id( - config, + config_dir, &bundle, ArtifactType::Claude, Some("/test/project"), @@ -1100,7 +1080,7 @@ mod tests { sync_bundle(config_dir, &bundle, &[ArtifactType::Claude], None, &mut ()).unwrap(); let cache_id = fresh_cache_id( - config, + config_dir, &bundle, ArtifactType::Claude, Some("/test/project"), @@ -1118,7 +1098,7 @@ mod tests { std::fs::write(&file, body).unwrap(); assert!( fresh_cache_id( - config, + config_dir, &bundle, ArtifactType::Claude, Some("/test/project"), @@ -1129,7 +1109,7 @@ mod tests { sync_bundle(config_dir, &bundle, &[ArtifactType::Claude], None, &mut ()).unwrap(); assert!( fresh_cache_id( - config, + config_dir, &bundle, ArtifactType::Claude, Some("/test/project"), @@ -1139,11 +1119,11 @@ mod tests { ); // Evicted: known but not materialized, so not fresh. - crate::cache::remove_cached(&cache_id).unwrap(); + crate::cache::remove_cached(config_dir, &cache_id).unwrap(); evict_cache_id(config_dir, &cache_id).unwrap(); assert!( fresh_cache_id( - config, + config_dir, &bundle, ArtifactType::Claude, Some("/test/project"),