Skip to content

feat(cli): thread Config into the resume path #203

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
ecalifornica wants to merge 1 commit into from
Draft

feat(cli): thread Config into the resume path #203

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 38 additions & 59 deletions crates/path-cli/src/cmd_resume.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,14 +83,14 @@ pub struct ResumeArgs {
pub url: Option<String>,
}

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() {
Expand All @@ -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)
}
Expand Down Expand Up @@ -200,7 +197,10 @@ pub(crate) fn ensure_path_with_agent(g: &Graph) -> Result<&TPath> {
/// Resolve the user-supplied `<input>` 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<Harness>)> {
pub(crate) fn resolve_input(
args: &ResumeArgs,
config: &Config,
) -> Result<(Graph, Option<Harness>)> {
let raw = args.input.as_str();

enum Shape<'a> {
Expand Down Expand Up @@ -240,11 +240,7 @@ pub(crate) fn resolve_input(args: &ResumeArgs) -> Result<(Graph, Option<Harness>
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,
Expand Down Expand Up @@ -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");
Expand All @@ -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");
Expand Down Expand Up @@ -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");
Expand All @@ -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));
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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}");
}
Expand Down Expand Up @@ -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());
}
Expand Down Expand Up @@ -1044,10 +1054,6 @@ mod tests {
}
}

fn scoped_home_for_resume() -> ScopedHomeForResume {
ScopedHomeForResume::new()
}

struct ScopedPathForResume {
_bin_dir: tempfile::TempDir,
prev: Option<std::ffi::OsString>,
Expand Down Expand Up @@ -1085,33 +1091,6 @@ mod tests {
}
}

struct ScopedHomeForResume {
_td: tempfile::TempDir,
prev: Option<std::ffi::OsString>,
}

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();
Expand Down
11 changes: 9 additions & 2 deletions crates/path-cli/src/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<PathBuf>,
/// `$COPILOT_HOME`: Copilot CLI session root override.
Expand Down Expand Up @@ -109,7 +115,8 @@ impl Config {
}

/// Read the process environment and extract an immutable `Config`.
pub(crate) fn load() -> Result<Self> {
#[doc(hidden)]
pub fn load() -> Result<Self> {
let vars: Vec<&str> = Self::env_var_names().collect();
let env = Env::raw().only(&vars).map(|key| {
Self::ENV_MAP
Expand Down
5 changes: 3 additions & 2 deletions crates/path-cli/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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"))]
Expand Down
Loading
Loading