Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
252 changes: 200 additions & 52 deletions crates/coven-cli/src/config_paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ impl ProcessRoots {
/// not create state, spawn a process, or inspect workspace contents.
pub fn report() -> PathsReport {
let roots = ProcessRoots::capture();
let cwd = std::env::current_dir().unwrap_or_else(|_| PathBuf::from("."));
let cwd = std::env::current_dir().unwrap_or_default();
let mut surfaces = Vec::new();

match roots.coven_home {
Expand Down Expand Up @@ -241,10 +241,11 @@ fn append_home_surfaces(
source,
cwd,
),
Err(_) => push_terminal(
Err(_) => push_terminal_with_source(
surfaces,
"state.familiar_workspaces",
PathStatus::Unresolved,
PathSource::Configuration,
),
}
push_path(surfaces, "state.skills", &home.join("skills"), source, cwd);
Expand Down Expand Up @@ -411,13 +412,9 @@ fn push_adapter_environment_surfaces(surfaces: &mut Vec<PathSurface>, cwd: &Path
),
}

let paths: Vec<String> = std::env::var_os(harness::EXTERNAL_ADAPTER_DIRS_ENV)
let paths: Vec<PathBuf> = std::env::var_os(harness::EXTERNAL_ADAPTER_DIRS_ENV)
.filter(|value| !value.is_empty())
.map(|value| {
std::env::split_paths(&value)
.map(|path| absolute_path(&path, cwd).display().to_string())
.collect()
})
.map(|value| std::env::split_paths(&value).collect())
.unwrap_or_default();
if paths.is_empty() {
push_terminal(
Expand All @@ -426,14 +423,13 @@ fn push_adapter_environment_surfaces(surfaces: &mut Vec<PathSurface>, cwd: &Path
PathStatus::NotApplicable,
);
} else {
surfaces.push(PathSurface {
id: "adapters.external_roots",
status: PathStatus::Resolved,
path: None,
paths,
source: PathSource::Environment,
access: AccessMode::ReadOnly,
});
push_paths(
surfaces,
"adapters.external_roots",
paths.iter().map(PathBuf::as_path),
PathSource::Environment,
cwd,
);
}
}

Expand Down Expand Up @@ -470,7 +466,7 @@ fn push_optional_path(
) {
match path {
Some(path) => push_path(surfaces, id, path, source, cwd),
None => push_terminal(surfaces, id, PathStatus::Unresolved),
None => push_terminal_with_source(surfaces, id, PathStatus::Unresolved, source),
}
}

Expand All @@ -481,14 +477,17 @@ fn push_path(
source: PathSource,
cwd: &Path,
) {
surfaces.push(PathSurface {
id,
status: PathStatus::Resolved,
path: Some(absolute_path(path, cwd).display().to_string()),
paths: Vec::new(),
source,
access: AccessMode::ReadOnly,
});
match resolved_path_string(path, cwd) {
Some(path) => surfaces.push(PathSurface {
id,
status: PathStatus::Resolved,
path: Some(path),
paths: Vec::new(),
source,
access: AccessMode::ReadOnly,
}),
None => push_terminal_with_source(surfaces, id, PathStatus::Unresolved, source),
}
}

fn push_paths<'a>(
Expand All @@ -498,36 +497,75 @@ fn push_paths<'a>(
source: PathSource,
cwd: &Path,
) {
surfaces.push(PathSurface {
id,
status: PathStatus::Resolved,
path: None,
paths: paths
.into_iter()
.map(|path| absolute_path(path, cwd).display().to_string())
.collect(),
source,
access: AccessMode::ReadOnly,
});
let paths: Option<Vec<String>> = paths
.into_iter()
.map(|path| resolved_path_string(path, cwd))
.collect();
match paths {
Some(paths) => surfaces.push(PathSurface {
id,
status: PathStatus::Resolved,
path: None,
paths,
source,
access: AccessMode::ReadOnly,
}),
None => push_terminal_with_source(surfaces, id, PathStatus::Unresolved, source),
}
}

fn push_terminal(surfaces: &mut Vec<PathSurface>, id: &'static str, status: PathStatus) {
push_terminal_with_source(surfaces, id, status, PathSource::Default);
}

fn push_terminal_with_source(
surfaces: &mut Vec<PathSurface>,
id: &'static str,
status: PathStatus,
source: PathSource,
) {
surfaces.push(PathSurface {
id,
status,
path: None,
paths: Vec::new(),
source: PathSource::Default,
source,
access: AccessMode::ReadOnly,
});
}

fn absolute_path(path: &Path, cwd: &Path) -> PathBuf {
fn absolute_path(path: &Path, cwd: &Path) -> Option<PathBuf> {
if path.is_absolute() {
path.to_path_buf()
} else {
cwd.join(path)
return Some(path.to_path_buf());
}
if !cwd.is_absolute() {
return None;
}

#[cfg(windows)]
{
use std::path::Component;

let needs_process_resolution =
path.has_root() || matches!(path.components().next(), Some(Component::Prefix(_)));
if needs_process_resolution {
// Drive-relative paths (for example `C:state`) use per-drive
// process state that a lexical join cannot reproduce.
std::path::absolute(path)
.ok()
.filter(|resolved| resolved.is_absolute())
} else {
Some(cwd.join(path))
}
}
#[cfg(not(windows))]
{
Some(cwd.join(path))
}
}

fn resolved_path_string(path: &Path, cwd: &Path) -> Option<String> {
absolute_path(path, cwd)?.to_str().map(ToOwned::to_owned)
}

fn nonempty_env(name: &str) -> bool {
Expand Down Expand Up @@ -567,12 +605,9 @@ fn user_home_source(home: Option<&Path>) -> PathSource {
let drive_and_path = std::env::var_os("HOMEDRIVE")
.filter(|value| !value.is_empty())
.zip(std::env::var_os("HOMEPATH").filter(|value| !value.is_empty()))
.map(|(drive, path)| {
PathBuf::from(format!(
"{}{}",
drive.to_string_lossy(),
path.to_string_lossy()
))
.map(|(mut drive, path)| {
drive.push(path);
PathBuf::from(drive)
});
if matches_home || drive_and_path.is_some_and(|candidate| candidate == home) {
PathSource::Environment
Expand All @@ -592,14 +627,127 @@ mod tests {

#[test]
fn absolute_path_keeps_absolute_paths_and_resolves_relative_paths_lexically() {
let cwd = Path::new("/tmp/coven-config-paths");
let cwd = std::env::current_dir().expect("absolute process directory");
let resolved = absolute_path(Path::new("relative/state"), &cwd)
.expect("resolve relative path against process directory");
let absolute = cwd.join("absolute/state");

assert!(resolved.is_absolute());
assert!(resolved.ends_with(Path::new("relative/state")));
assert_eq!(
absolute_path(Path::new("relative/state"), cwd),
cwd.join("relative/state")
absolute_path(&absolute, &cwd),
Some(absolute),
"absolute paths must remain unchanged"
);
}

#[test]
fn push_path_fails_closed_without_an_absolute_working_directory() {
let mut surfaces = Vec::new();

push_path(
&mut surfaces,
"test.relative",
Path::new("relative/state"),
PathSource::Environment,
Path::new("."),
);

assert_eq!(surfaces.len(), 1);
assert!(matches!(surfaces[0].status, PathStatus::Unresolved));
assert!(surfaces[0].path.is_none());
assert!(matches!(surfaces[0].source, PathSource::Environment));
}

#[test]
fn push_optional_path_preserves_the_unresolved_source() {
let mut surfaces = Vec::new();

push_optional_path(
&mut surfaces,
"test.optional",
None,
PathSource::Environment,
Path::new("/tmp"),
);

assert_eq!(surfaces.len(), 1);
assert!(matches!(surfaces[0].status, PathStatus::Unresolved));
assert!(matches!(surfaces[0].source, PathSource::Environment));
}

#[cfg(unix)]
#[test]
fn push_path_fails_closed_for_non_unicode_paths() {
use std::os::unix::ffi::OsStringExt;

let path = PathBuf::from(std::ffi::OsString::from_vec(b"/tmp/coven-\xFF".to_vec()));
let mut surfaces = Vec::new();

push_path(
&mut surfaces,
"test.non_unicode",
&path,
PathSource::Configuration,
Path::new("/tmp"),
);

assert_eq!(surfaces.len(), 1);
assert!(matches!(surfaces[0].status, PathStatus::Unresolved));
assert!(surfaces[0].path.is_none());
assert!(matches!(surfaces[0].source, PathSource::Configuration));
}

#[cfg(unix)]
#[test]
fn push_paths_fails_closed_without_partial_output() {
use std::os::unix::ffi::OsStringExt;

let valid = PathBuf::from("/tmp/coven-valid");
let invalid = PathBuf::from(std::ffi::OsString::from_vec(b"/tmp/coven-\xFF".to_vec()));
let mut surfaces = Vec::new();

push_paths(
&mut surfaces,
"test.non_unicode_array",
[&valid, &invalid].into_iter().map(PathBuf::as_path),
PathSource::Environment,
Path::new("/tmp"),
);

assert_eq!(surfaces.len(), 1);
assert!(matches!(surfaces[0].status, PathStatus::Unresolved));
assert!(surfaces[0].paths.is_empty());
assert!(matches!(surfaces[0].source, PathSource::Environment));
}

#[cfg(windows)]
#[test]
fn absolute_path_resolves_drive_relative_paths() {
use std::path::{Component, Prefix};

let resolved = absolute_path(Path::new(r"C:relative\state"), Path::new(r"C:\workspace"))
.expect("resolve drive-relative path");

assert!(
resolved.is_absolute(),
"drive-relative path remained relative: {}",
resolved.display()
);
assert!(matches!(
resolved.components().next(),
Some(Component::Prefix(prefix)) if matches!(prefix.kind(), Prefix::Disk(b'C'))
));
}

#[cfg(windows)]
#[test]
fn absolute_path_resolves_plain_relative_paths_against_captured_cwd() {
let cwd = Path::new(r"C:\workspace");

assert_eq!(
absolute_path(Path::new("/var/lib/coven"), cwd),
PathBuf::from("/var/lib/coven")
absolute_path(Path::new(r"relative\state"), cwd),
Some(PathBuf::from(r"C:\workspace\relative\state"))
);
}

Expand Down
34 changes: 34 additions & 0 deletions crates/coven-cli/tests/config_paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,40 @@ fn paths_json_does_not_guess_workspaces_when_familiar_manifest_is_invalid() {
assert!(surface.get("paths").is_none());
}

#[cfg(unix)]
#[test]
fn paths_json_fails_closed_for_non_unicode_environment_paths() {
use std::os::unix::ffi::OsStringExt;

let temp = TempDir::new().expect("temporary directory");
let coven_home = temp
.path()
.join(OsString::from_vec(b"coven-home-\xFF".to_vec()));
let valid_adapter = temp.path().join("adapter-valid");
let invalid_adapter = temp
.path()
.join(OsString::from_vec(b"adapter-\xFF".to_vec()));
let adapter_dirs =
std::env::join_paths([&valid_adapter, &invalid_adapter]).expect("join adapter directories");

let report = report(&run_paths(
&temp,
&coven_home,
&[("COVEN_HARNESS_ADAPTER_DIRS", adapter_dirs)],
));
let surfaces = surfaces(&report);

assert_eq!(surfaces["coven.home"]["status"], "unresolved");
assert_eq!(surfaces["coven.home"]["source"], "environment");
assert!(surfaces["coven.home"].get("path").is_none());
assert_eq!(surfaces["adapters.external_roots"]["status"], "unresolved");
assert_eq!(surfaces["adapters.external_roots"]["source"], "environment");
assert!(
surfaces["adapters.external_roots"].get("paths").is_none(),
"a multi-path surface must not emit partial or lossy output"
);
}

#[test]
fn paths_requires_machine_readable_output_flag() {
let output = Command::new(env!("CARGO_BIN_EXE_coven"))
Expand Down
7 changes: 7 additions & 0 deletions docs/reference/cli-config.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,13 @@ environment-provided adapter search roots and configured familiar workspaces.
Terminal `not_applicable`, `unsupported`, and `unresolved` surfaces
intentionally omit `path`.

Because JSON path fields are UTF-8 strings, a filesystem path that cannot be
represented exactly as UTF-8 is reported as `unresolved`; Coven never emits a
lossy replacement-character path. Multi-path surfaces fail closed as a whole
rather than returning a partial list. Relative inputs are resolved against the
process working directory using platform path semantics, including
drive-relative paths on Windows.

`source` is `environment` when an applicable environment override selected
the location, `configuration` when `familiars.toml` selected familiar
workspaces, and `default` otherwise. `access` is always `read_only`: it
Expand Down