From 3f26a7b0978fead951e0180f0f59286cf7cdf0ab Mon Sep 17 00:00:00 2001 From: leynos Date: Sat, 22 Aug 2026 04:29:29 +0200 Subject: [PATCH] Inject a base-directory seam into manifest workspace resolution `resolve_absolute_workspace_root` and `open_manifest_workspace` now accept an `Option<&Path>` base that anchors relative manifest parents. `None` keeps the ambient `env::current_dir()` fallback, so production behaviour is unchanged (query.rs passes `None`); tests inject the temporary directory through the seam instead of mutating the process CWD. The manifest workspace unit tests drop the local `CurrentDirGuard` struct and the `EnvLock` import entirely, satisfying the AGENTS.md mandate that no test mutates in-process environment or working-directory state. Part of #493; unblocks the EnvLock/CwdGuard deletions in #494. --- src/manifest/query.rs | 2 +- src/manifest/tests/workspace.rs | 48 +++++---------------------------- src/manifest/workspace.rs | 28 ++++++++++++++----- 3 files changed, 30 insertions(+), 48 deletions(-) diff --git a/src/manifest/query.rs b/src/manifest/query.rs index 3e78656df..35e433734 100644 --- a/src/manifest/query.rs +++ b/src/manifest/query.rs @@ -55,7 +55,7 @@ fn from_path_with_registration( ) -> Result { notify_stage(&mut on_stage, ManifestLoadStage::ManifestIngestion); let path_ref = path.as_ref(); - let workspace = open_manifest_workspace(path_ref)?; + let workspace = open_manifest_workspace(path_ref, None)?; let data = workspace .dir .read_to_string(&workspace.manifest_file) diff --git a/src/manifest/tests/workspace.rs b/src/manifest/tests/workspace.rs index 8231d0dd2..58246623d 100644 --- a/src/manifest/tests/workspace.rs +++ b/src/manifest/tests/workspace.rs @@ -11,54 +11,21 @@ use rstest::rstest; use std::{path::Path, sync::Arc}; use tempfile::tempdir; use test_support::fs as test_fs; -use test_support::{env_lock::EnvLock, hash, http}; +use test_support::{hash, http}; use url::Url; -struct CurrentDirGuard { - original: std::path::PathBuf, - _lock: EnvLock, -} - -impl CurrentDirGuard { - fn change_to(path: &std::path::Path) -> AnyResult { - let lock = EnvLock::acquire(); - let original = std::env::current_dir().context("capture current working directory")?; - std::env::set_current_dir(path) - .with_context(|| format!("switch to working directory {}", path.display()))?; - Ok(Self { - original, - _lock: lock, - }) - } -} - -impl Drop for CurrentDirGuard { - fn drop(&mut self) { - if let Err(err) = std::env::set_current_dir(&self.original) { - tracing::warn!( - "failed to restore working directory to {}: {err}", - self.original.display() - ); - } - } -} - #[rstest] #[case(true)] #[case(false)] fn open_manifest_workspace_resolves_workspace_root(#[case] use_relative: bool) -> AnyResult<()> { let temp = tempdir().context("create temp workspace")?; - let _guard = if use_relative { - Some(CurrentDirGuard::change_to(temp.path())?) - } else { - None - }; let manifest_path = if use_relative { Path::new("Netsukefile").to_path_buf() } else { temp.path().join("Netsukefile") }; - let workspace = open_manifest_workspace(&manifest_path)?; + let base = use_relative.then(|| temp.path()); + let workspace = open_manifest_workspace(&manifest_path, base)?; let expected = Utf8Path::from_path(temp.path()).context("temp workspace path should be valid UTF-8")?; ensure!( @@ -86,7 +53,7 @@ fn open_manifest_workspace_rejects_non_utf_workspace_root() -> AnyResult<()> { test_fs::create_dir_all(&manifest_dir) .context("create manifest directory with invalid UTF-8 component")?; let manifest_path = manifest_dir.join("manifest.yml"); - let err = open_manifest_workspace(&manifest_path) + let err = open_manifest_workspace(&manifest_path, None) .expect_err("workspace should fail when its root contains non-UTF-8 components"); ensure!( err.to_string().contains("path is not valid UTF-8"), @@ -99,7 +66,7 @@ fn open_manifest_workspace_rejects_non_utf_workspace_root() -> AnyResult<()> { fn open_manifest_workspace_reports_missing_file_name() -> AnyResult<()> { // The filesystem root has no file-name component, so extraction fails with a // missing-name error, distinct from the non-UTF-8 case. - let err = open_manifest_workspace(Path::new("/")) + let err = open_manifest_workspace(Path::new("/"), None) .expect_err("workspace should fail when the path has no file name"); ensure!( err.to_string().contains("has no file name"), @@ -117,7 +84,7 @@ fn open_manifest_workspace_rejects_non_utf_file_name() -> AnyResult<()> { let temp = tempdir().context("create temp workspace")?; let invalid_name = OsString::from_vec(vec![b'm', 0xFF]); // invalid trailing byte let manifest_path = temp.path().join(&invalid_name); - let err = open_manifest_workspace(&manifest_path) + let err = open_manifest_workspace(&manifest_path, None) .expect_err("workspace should fail when the file name is not valid UTF-8"); ensure!( err.to_string().contains("path is not valid UTF-8"), @@ -132,7 +99,7 @@ fn open_manifest_workspace_reports_open_failure() -> AnyResult<()> { // the error is wrapped as a workspace open failure. let temp = tempdir().context("create temp workspace")?; let manifest_path = temp.path().join("missing-subdir").join("Netsukefile"); - let err = open_manifest_workspace(&manifest_path) + let err = open_manifest_workspace(&manifest_path, None) .expect_err("workspace open should fail when the parent directory is absent"); ensure!( err.to_string().contains("Failed to open workspace"), @@ -170,7 +137,6 @@ fn from_path_uses_manifest_directory_for_caches() -> AnyResult<()> { ); test_fs::write(&manifest_path, manifest_yaml)?; - let _cwd_guard = CurrentDirGuard::change_to(&outside)?; let manifest_url = url.clone(); let env_reader: EnvReader = Arc::new(move |key| { if key == "NETSUKE_MANIFEST_URL" { diff --git a/src/manifest/workspace.rs b/src/manifest/workspace.rs index 33de34724..2433ad3bb 100644 --- a/src/manifest/workspace.rs +++ b/src/manifest/workspace.rs @@ -11,13 +11,23 @@ use cap_std::{ambient_authority, fs_utf8::Dir}; use std::{env, path::Path}; /// Resolve a potentially relative manifest parent path to an absolute UTF-8 workspace root. -fn resolve_absolute_workspace_root(utf8_parent: &Utf8Path) -> Result { +/// +/// An injected `base` directory anchors relative parents for tests; `None` +/// falls back to the process current directory so production behaviour is +/// unchanged. +fn resolve_absolute_workspace_root( + utf8_parent: &Utf8Path, + base: Option<&Path>, +) -> Result { let workspace_base = if utf8_parent.is_absolute() { utf8_parent.to_path_buf().into_std_path_buf() } else { - env::current_dir() - .context(localization::message(keys::MANIFEST_RESOLVE_WORKSPACE_ROOT))? - .join(utf8_parent.as_std_path()) + let anchor = match base { + Some(dir) => dir.to_path_buf(), + None => env::current_dir() + .context(localization::message(keys::MANIFEST_RESOLVE_WORKSPACE_ROOT))?, + }; + anchor.join(utf8_parent.as_std_path()) }; Utf8PathBuf::from_path_buf(workspace_base).map_err(|invalid| { anyhow!( @@ -40,7 +50,13 @@ pub(super) struct ManifestWorkspace { } /// Open the directory containing `path` as a capability-scoped workspace. -pub(super) fn open_manifest_workspace(path: &Path) -> Result { +/// +/// `base` anchors relative manifest paths for tests; `None` keeps the ambient +/// current-directory resolution used by production callers. +pub(super) fn open_manifest_workspace( + path: &Path, + base: Option<&Path>, +) -> Result { let parent = match path.parent() { Some(parent) if !parent.as_os_str().is_empty() => parent, _ => Path::new("."), @@ -70,7 +86,7 @@ pub(super) fn open_manifest_workspace(path: &Path) -> Result .with_arg("path", path.display().to_string()) ) })?; - let root = resolve_absolute_workspace_root(utf8_parent)?; + let root = resolve_absolute_workspace_root(utf8_parent, base)?; tracing::debug!(workspace = %root, manifest = %manifest_file, "opening manifest workspace directory"); let dir = Dir::open_ambient_dir(root.as_path(), ambient_authority()) .inspect_err(|err| {