From 3267c120f9e56e106934394c44e46e3473b838c5 Mon Sep 17 00:00:00 2001 From: Robert Queenin <2177841+ecalifornica@users.noreply.github.com> Date: Thu, 13 Aug 2026 13:24:50 -0400 Subject: [PATCH] feat(cli): thread Config into the resume path (#184) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `resume` receives `&Config` from run() and passes it to the input resolver and the projectors. The transitional `Config::load()` calls in `run_with_strategy` and `resolve_input` are gone. - `cmd_resume::run`, `run_with_strategy`, and `resolve_input` take `&Config`. The Pathbase fetch calls `derive::pathbase_fetch_to_doc(config, …)`. - `Config` is public and `config` is a public module. `run_with_strategy` is the public entry point for `tests/resume.rs`, so its parameter type must be nameable there. The public items are `#[doc(hidden)]` and the fields stay crate-private; `Config::load` is the only constructor outside the crate. - The `$PATH` read for the harness binary lookup stays. A binary lookup is execution environment, not configuration. - The `project_into_harness` unit test injects a `Config` rooted at a tempdir. The `run_with_strategy` unit test does the same and keeps the `$PATH` guard. - The `resolve_input` unit tests inject a `Config`. The Pathbase fetch test needs no environment guard. The cache-hit test and the unresolvable-input test still set `$TOOLPATH_CONFIG_DIR`, because `cache.rs` reads it. - `ScopedHomeForResume` is deleted. The unit tests set no `$HOME`. - `ScopedHome::config` gives the integration tests the `Config` the CLI extracts at its composition root. --- crates/path-cli/src/cmd_resume.rs | 97 +++++++++++----------------- crates/path-cli/src/config.rs | 11 +++- crates/path-cli/src/lib.rs | 5 +- crates/path-cli/tests/resume.rs | 37 +++++++---- crates/path-cli/tests/support/mod.rs | 8 +++ 5 files changed, 83 insertions(+), 75 deletions(-) diff --git a/crates/path-cli/src/cmd_resume.rs b/crates/path-cli/src/cmd_resume.rs index 18c606a4..6521eb72 100644 --- a/crates/path-cli/src/cmd_resume.rs +++ b/crates/path-cli/src/cmd_resume.rs @@ -83,14 +83,14 @@ pub struct ResumeArgs { pub url: Option, } -pub fn run(args: ResumeArgs) -> Result<()> { - run_with_strategy(args, &RealExec) +pub fn run(args: ResumeArgs, config: &Config) -> Result<()> { + run_with_strategy(args, &RealExec, config) } /// Internal entry point that the integration tests call with a /// `RecordingExec` strategy. Production callers use [`run`]. -pub fn run_with_strategy(args: ResumeArgs, exec: &dyn ExecStrategy) -> Result<()> { - let (graph, source_harness) = resolve_input(&args)?; +pub fn run_with_strategy(args: ResumeArgs, exec: &dyn ExecStrategy, config: &Config) -> Result<()> { + let (graph, source_harness) = resolve_input(&args, config)?; let path = ensure_path_with_agent(&graph)?; let cwd = match args.cwd.as_ref() { @@ -111,10 +111,7 @@ pub fn run_with_strategy(args: ResumeArgs, exec: &dyn ExecStrategy) -> Result<() } ); - // Transitional: `resume` does not take `&Config` yet; load one for - // the export path. - let config = Config::load()?; - let session_id = project_into_harness(path, target, &cwd, &config)?; + let session_id = project_into_harness(path, target, &cwd, config)?; let (binary, argv) = invocation_for(target, &session_id, &cwd); exec_harness(&binary, &argv, &cwd, exec) } @@ -200,7 +197,10 @@ pub(crate) fn ensure_path_with_agent(g: &Graph) -> Result<&TPath> { /// Resolve the user-supplied `` argument into a parsed `Graph` /// plus the source harness inferred from its single inline path (if /// any). See spec § "Input resolution" for the order. -pub(crate) fn resolve_input(args: &ResumeArgs) -> Result<(Graph, Option)> { +pub(crate) fn resolve_input( + args: &ResumeArgs, + config: &Config, +) -> Result<(Graph, Option)> { let raw = args.input.as_str(); enum Shape<'a> { @@ -240,11 +240,7 @@ pub(crate) fn resolve_input(args: &ResumeArgs) -> Result<(Graph, Option Graph::from_json(&json) .map_err(|e| anyhow::anyhow!("cached toolpath document is invalid: {}", e))? } else { - // Transitional: `resolve_input` does not take `&Config` - // yet; load one for the Pathbase fetch. - let config = Config::load()?; - let derived = - crate::derive::pathbase_fetch_to_doc(&config, u, args.url.as_deref())?; + let derived = crate::derive::pathbase_fetch_to_doc(config, u, args.url.as_deref())?; if !args.no_cache { // force=true here: we either short-circuited above // (cache miss) or the user explicitly passed --force, @@ -597,12 +593,23 @@ fn looks_like_pathbase_shorthand(s: &str) -> bool { mod tests { use super::*; + /// A `Config` rooted at `home`, so the projectors write inside the + /// test's tempdir. + fn config_with_home(home: &std::path::Path) -> Config { + Config { + home: Some(home.to_path_buf()), + ..Config::default() + } + } + #[test] fn run_with_strategy_records_invocation_for_file_input_with_explicit_harness() { + // The `$PATH` guard mutates process-global state; the lock + // serializes it against the other env-mutating tests. let _env = crate::config::TEST_ENV_LOCK .lock() .unwrap_or_else(|e| e.into_inner()); - let _home = scoped_home_for_resume(); + let home = tempfile::tempdir().unwrap(); let _path_guard = ScopedPathForResume::with_binaries(&["claude"]); let cwd = tempfile::tempdir().unwrap(); let doc_file = cwd.path().join("doc.json"); @@ -627,7 +634,7 @@ mod tests { }; let recorder = RecordingExec::default(); - run_with_strategy(args, &recorder).unwrap(); + run_with_strategy(args, &recorder, &config_with_home(home.path())).unwrap(); let cap = recorder.captured(); assert_eq!(cap.binary, "claude"); @@ -761,16 +768,13 @@ mod tests { force: false, url: None, }; - let (g, harness) = resolve_input(&args).unwrap(); + let (g, harness) = resolve_input(&args, &Config::default()).unwrap(); let _path = ensure_path_with_agent(&g).unwrap(); assert_eq!(harness, Some(Harness::Claude)); } #[test] fn resolve_input_url_dispatches_to_pathbase_fetch() { - let _env = crate::config::TEST_ENV_LOCK - .lock() - .unwrap_or_else(|e| e.into_inner()); use crate::cmd_pathbase::tests::MockServer; let body = { let mut path = make_path_with_actor("agent:codex"); @@ -795,7 +799,12 @@ mod tests { force: false, url: None, }; - let (g, harness) = resolve_input(&args).unwrap(); + let cfg_dir = tempfile::tempdir().unwrap(); + let config = Config { + toolpath_config_dir: Some(cfg_dir.path().to_path_buf()), + ..Config::default() + }; + let (g, harness) = resolve_input(&args, &config).unwrap(); let _ = ensure_path_with_agent(&g).unwrap(); assert_eq!(harness, Some(Harness::Codex)); } @@ -856,7 +865,11 @@ mod tests { force: false, url: None, }; - let result = resolve_input(&args); + let config = Config { + toolpath_config_dir: Some(cfg_dir.path().to_path_buf()), + ..Config::default() + }; + let result = resolve_input(&args, &config); // Restore env before asserting so a panic doesn't poison sibling tests. unsafe { @@ -884,7 +897,7 @@ mod tests { force: false, url: None, }; - let err = resolve_input(&args).unwrap_err(); + let err = resolve_input(&args, &Config::default()).unwrap_err(); let s = err.to_string(); assert!(s.contains("couldn't resolve"), "actual: {s}"); } @@ -989,14 +1002,11 @@ mod tests { #[test] fn project_into_harness_claude_round_trip() { - let _env = crate::config::TEST_ENV_LOCK - .lock() - .unwrap_or_else(|e| e.into_inner()); - let _home = scoped_home_for_resume(); + let home = tempfile::tempdir().unwrap(); let cwd = tempfile::tempdir().unwrap(); let path = make_convo_path_for_resume("claude-code://resume-test-session"); - let config = Config::load().unwrap(); + let config = config_with_home(home.path()); let session_id = project_into_harness(&path, Harness::Claude, cwd.path(), &config).unwrap(); assert!(!session_id.is_empty()); } @@ -1044,10 +1054,6 @@ mod tests { } } - fn scoped_home_for_resume() -> ScopedHomeForResume { - ScopedHomeForResume::new() - } - struct ScopedPathForResume { _bin_dir: tempfile::TempDir, prev: Option, @@ -1085,33 +1091,6 @@ mod tests { } } - struct ScopedHomeForResume { - _td: tempfile::TempDir, - prev: Option, - } - - impl ScopedHomeForResume { - fn new() -> Self { - let td = tempfile::tempdir().unwrap(); - let prev = std::env::var_os("HOME"); - unsafe { - std::env::set_var("HOME", td.path()); - } - Self { _td: td, prev } - } - } - - impl Drop for ScopedHomeForResume { - fn drop(&mut self) { - unsafe { - match &self.prev { - Some(v) => std::env::set_var("HOME", v), - None => std::env::remove_var("HOME"), - } - } - } - } - #[test] fn exec_strategy_recording_captures_invocation() { let recorder = RecordingExec::default(); diff --git a/crates/path-cli/src/config.rs b/crates/path-cli/src/config.rs index 7f27416f..3e0f51cd 100644 --- a/crates/path-cli/src/config.rs +++ b/crates/path-cli/src/config.rs @@ -43,8 +43,14 @@ pub(crate) const DOCUMENTS_DIR_NAME: &str = "documents"; /// Environment-derived configuration. [`Config::load`] reads the /// environment once, at the composition root. Code below the root /// receives values as parameters and does not read the environment. +/// +/// Public because `cmd_resume::run_with_strategy` takes a `&Config` +/// across the crate boundary. It is a test seam, not API: the item is +/// `#[doc(hidden)]` and the fields stay crate-private, so +/// [`Config::load`] is the only constructor outside the crate. +#[doc(hidden)] #[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize)] -pub(crate) struct Config { +pub struct Config { /// `$APPDATA`: Windows harness data root. pub(crate) appdata: Option, /// `$COPILOT_HOME`: Copilot CLI session root override. @@ -109,7 +115,8 @@ impl Config { } /// Read the process environment and extract an immutable `Config`. - pub(crate) fn load() -> Result { + #[doc(hidden)] + pub fn load() -> Result { let vars: Vec<&str> = Self::env_var_names().collect(); let env = Env::raw().only(&vars).map(|key| { Self::ENV_MAP diff --git a/crates/path-cli/src/lib.rs b/crates/path-cli/src/lib.rs index dc1d51a7..9f559d0f 100644 --- a/crates/path-cli/src/lib.rs +++ b/crates/path-cli/src/lib.rs @@ -28,7 +28,8 @@ mod cmd_share; mod cmd_show; mod cmd_track; mod cmd_validate; -mod config; +#[doc(hidden)] +pub mod config; mod derive; #[cfg(not(target_os = "emscripten"))] mod fuzzy; @@ -151,7 +152,7 @@ pub fn run() -> Result<()> { #[cfg(not(target_os = "emscripten"))] Commands::Share { args } => cmd_share::run(args, &config), #[cfg(not(target_os = "emscripten"))] - Commands::Resume { args } => cmd_resume::run(args), + Commands::Resume { args } => cmd_resume::run(args, &config), Commands::Query { args } => cmd_query::run(args, cli.pretty, &config), Commands::Kind { args } => cmd_kind::run(args), #[cfg(not(target_os = "emscripten"))] diff --git a/crates/path-cli/tests/resume.rs b/crates/path-cli/tests/resume.rs index f751c40e..0df0a4c1 100644 --- a/crates/path-cli/tests/resume.rs +++ b/crates/path-cli/tests/resume.rs @@ -19,7 +19,7 @@ use support::*; #[test] fn file_input_explicit_claude_projects_and_records_exec() { let _env = env_lock(); - let _home = ScopedHome::new(); + let home = ScopedHome::new(); let _path = ScopedPath::with_binary("claude"); let cwd = tempfile::tempdir().unwrap(); @@ -30,6 +30,7 @@ fn file_input_explicit_claude_projects_and_records_exec() { run_with_strategy( args_explicit(doc_file, cwd.path(), Harness::Claude), &recorder, + &home.config(), ) .unwrap(); @@ -53,7 +54,7 @@ fn file_input_explicit_claude_projects_and_records_exec() { #[test] fn file_input_explicit_gemini_projects_and_records_exec() { let _env = env_lock(); - let _home = ScopedHome::new(); + let home = ScopedHome::new(); let _path = ScopedPath::with_binary("gemini"); let cwd = tempfile::tempdir().unwrap(); @@ -64,6 +65,7 @@ fn file_input_explicit_gemini_projects_and_records_exec() { run_with_strategy( args_explicit(doc_file, cwd.path(), Harness::Gemini), &recorder, + &home.config(), ) .unwrap(); @@ -81,7 +83,7 @@ fn file_input_explicit_gemini_projects_and_records_exec() { #[test] fn file_input_explicit_codex_projects_and_records_exec() { let _env = env_lock(); - let _home = ScopedHome::new(); + let home = ScopedHome::new(); let _path = ScopedPath::with_binary("codex"); let cwd = tempfile::tempdir().unwrap(); @@ -92,6 +94,7 @@ fn file_input_explicit_codex_projects_and_records_exec() { run_with_strategy( args_explicit(doc_file, cwd.path(), Harness::Codex), &recorder, + &home.config(), ) .unwrap(); @@ -109,7 +112,7 @@ fn file_input_explicit_codex_projects_and_records_exec() { #[test] fn file_input_explicit_copilot_projects_and_records_exec() { let _env = env_lock(); - let _home = ScopedHome::new(); + let home = ScopedHome::new(); let _path = ScopedPath::with_binary("copilot"); let cwd = tempfile::tempdir().unwrap(); @@ -120,6 +123,7 @@ fn file_input_explicit_copilot_projects_and_records_exec() { run_with_strategy( args_explicit(doc_file, cwd.path(), Harness::Copilot), &recorder, + &home.config(), ) .unwrap(); @@ -144,7 +148,7 @@ fn file_input_explicit_copilot_projects_and_records_exec() { #[test] fn file_input_explicit_opencode_projects_and_records_exec() { let _env = env_lock(); - let _home = ScopedHome::new(); + let home = ScopedHome::new(); let _path = ScopedPath::with_binary("opencode"); let cwd = tempfile::tempdir().unwrap(); @@ -194,6 +198,7 @@ fn file_input_explicit_opencode_projects_and_records_exec() { run_with_strategy( args_explicit(doc_file, cwd.path(), Harness::Opencode), &recorder, + &home.config(), ) .unwrap(); @@ -212,7 +217,7 @@ fn file_input_explicit_opencode_projects_and_records_exec() { #[test] fn file_input_explicit_pi_projects_and_records_exec() { let _env = env_lock(); - let _home = ScopedHome::new(); + let home = ScopedHome::new(); let _path = ScopedPath::with_binary("pi"); let cwd = tempfile::tempdir().unwrap(); @@ -220,7 +225,12 @@ fn file_input_explicit_pi_projects_and_records_exec() { let doc_file = write_path_to_temp(cwd.path(), path); let recorder = RecordingExec::default(); - run_with_strategy(args_explicit(doc_file, cwd.path(), Harness::Pi), &recorder).unwrap(); + run_with_strategy( + args_explicit(doc_file, cwd.path(), Harness::Pi), + &recorder, + &home.config(), + ) + .unwrap(); let cap = recorder.captured(); assert_eq!(cap.binary, "pi"); @@ -238,7 +248,7 @@ fn file_input_explicit_pi_projects_and_records_exec() { #[test] fn cache_id_input_loads_and_projects() { let _env = env_lock(); - let _home = ScopedHome::new(); + let home = ScopedHome::new(); let _path = ScopedPath::with_binary("claude"); let cwd = tempfile::tempdir().unwrap(); @@ -268,7 +278,7 @@ fn cache_id_input_loads_and_projects() { }; let recorder = RecordingExec::default(); - run_with_strategy(resume_args, &recorder).unwrap(); + run_with_strategy(resume_args, &recorder, &home.config()).unwrap(); let cap = recorder.captured(); assert_eq!(cap.binary, "claude"); @@ -280,7 +290,7 @@ fn cache_id_input_loads_and_projects() { #[test] fn multi_path_graph_returns_clear_error() { let _env = env_lock(); - let _home = ScopedHome::new(); + let home = ScopedHome::new(); let _path = ScopedPath::with_binary("claude"); let cwd = tempfile::tempdir().unwrap(); @@ -303,6 +313,7 @@ fn multi_path_graph_returns_clear_error() { let err = run_with_strategy( args_explicit(doc_file, cwd.path(), Harness::Claude), &recorder, + &home.config(), ) .unwrap_err(); let s = err.to_string(); @@ -313,7 +324,7 @@ fn multi_path_graph_returns_clear_error() { #[test] fn agentless_path_returns_clear_error() { let _env = env_lock(); - let _home = ScopedHome::new(); + let home = ScopedHome::new(); let _path = ScopedPath::with_binary("claude"); let cwd = tempfile::tempdir().unwrap(); @@ -325,6 +336,7 @@ fn agentless_path_returns_clear_error() { let err = run_with_strategy( args_explicit(doc_file, cwd.path(), Harness::Claude), &recorder, + &home.config(), ) .unwrap_err(); assert!(err.to_string().contains("no agent session")); @@ -333,7 +345,7 @@ fn agentless_path_returns_clear_error() { #[test] fn explicit_harness_not_on_path_errors() { let _env = env_lock(); - let _home = ScopedHome::new(); + let home = ScopedHome::new(); let _path = ScopedPath::empty(); let cwd = tempfile::tempdir().unwrap(); @@ -344,6 +356,7 @@ fn explicit_harness_not_on_path_errors() { let err = run_with_strategy( args_explicit(doc_file, cwd.path(), Harness::Claude), &recorder, + &home.config(), ) .unwrap_err(); let s = err.to_string(); diff --git a/crates/path-cli/tests/support/mod.rs b/crates/path-cli/tests/support/mod.rs index bf7597ba..c652a3cf 100644 --- a/crates/path-cli/tests/support/mod.rs +++ b/crates/path-cli/tests/support/mod.rs @@ -12,6 +12,7 @@ use std::path::{Path, PathBuf}; use std::sync::{Mutex, OnceLock}; use path_cli::cmd_resume::ResumeArgs; +use path_cli::config::Config; use path_cli::harness::Harness; /// Process-wide lock for tests that mutate `$HOME`, `$PATH`, or @@ -53,6 +54,13 @@ impl ScopedHome { pub fn home_dir(&self) -> PathBuf { PathBuf::from(self._td.path()) } + + /// The `Config` the CLI extracts at its composition root. Loaded + /// under this guard, so every path it carries points into the + /// sandbox. + pub fn config(&self) -> Config { + Config::load().expect("load config") + } } impl Drop for ScopedHome {