Remove BDD process-global state (#492)

docs/adr-008-environment-seam-taxonomy.md

@@ -118,6 +118,24 @@ that need a deterministic Ninja executable use `runner::run_with_ninja_program`
 to supply the already-resolved program path directly, bypassing `NETSUKE_NINJA`
 resolution entirely rather than setting the variable for a child to read.
 
+### BDD route selection
+
+`rstest-bdd` steps execute in the generated test-harness process. A behavioural
+label therefore does not make a step a subprocess test. Issue #492 removes the
+BDD suite's process-global environment/CWD guard and records two allowed routes:
+
+- **Route A — isolated child.** An end-to-end scenario invokes `netsuke` with
+  `assert_cmd`, clears the child environment, and supplies only the required
+  values through `Command::env`.
+- **Route B — injected environment.** A scenario calls an in-process library
+  entry point with its injected environment and retains assertions over values
+  such as `Cli`, `Manifest`, `BuildGraph`, or rendered output.
+
+Route B avoids CWD changes by passing absolute manifest or configuration paths,
+or by preserving `-C/--directory` as a CLI value for automatic project
+discovery. An explicit relative `--config` or `NETSUKE_CONFIG` selector remains
+relative to the child or harness process CWD; it is not rebased under `-C`.
+
 ## Rationale
 
 - **Proportionate abstraction.** A trait object for a single-variable,
@@ -170,11 +188,12 @@ resolution entirely rather than setting the variable for a child to read.
   Rejected: mutating the test process to influence a spawned `netsuke` binary
   reintroduces the shared-mutable-state races that injected readers and
   child-process configuration exist to avoid, and it is exactly the pattern
-  #493 removed from the BDD and integration test helpers.
+  #493 removed from the BDD and integration test helpers. Issue #492 also
+  removed BDD's process-global CWD coordination.
   `.config/nextest.toml` runs no serialized environment group precisely because
-  no sanctioned test still mutates the harness environment; `EnvLock` and
-  `CwdGuard` remain only for the few tests that exercise process
-  working-directory behaviour.
+  no sanctioned test still mutates the harness environment. `EnvLock` and
+  `CwdGuard` remain only for direct tests that exercise process
+  working-directory behaviour; they are not a BDD isolation mechanism.
 
 ## Implementation references
 

docs/developers-guide.md

@@ -1417,9 +1417,10 @@ governs the non-doctest pass only, and deliberately stays small:
 nextest runs each test in its own process, but the codebase does not rely on
 that isolation for environment safety. Tests pass environment values through
 explicit configuration seams or configure a child with `env_clear()` followed by
-`Command::env`. `EnvLock` and `CwdGuard` remain only for the few tests that
-exercise process working-directory behaviour, because the in-process coverage
-runner shares that state.
+`Command::env`. The BDD suite carries no environment or CWD lock: its steps run
+inside the generated test-harness process, not inside an `assert_cmd` child.
+`EnvLock` and `CwdGuard` remain only for direct tests that deliberately
+exercise process working-directory behaviour outside that suite.
 
 ### Runners not covered by this configuration
 
@@ -1705,15 +1706,19 @@ cargo-nextest alongside every other test (see
 point (`world: TestWorld`) to each generated scenario test.
 
 nextest runs each generated scenario in its own process. That reinforces the
-per-scenario isolation policy below rather than conflicting with it: scenario
-state cannot leak across process boundaries, so the policy's requirement to
-recreate state per test is enforced by the runner as well as by convention.
+per-scenario isolation policy below rather than conflicting with it. The
+scenario's steps still execute in that generated test-harness process, so an
+in-process BDD step does not qualify for subprocess isolation.
 
 ### State and isolation policy
 
 - Scenario isolation is the default: scenario state must be recreated per test.
 - Shared process-wide state is avoided unless infrastructure cost requires
   controlled reuse.
+- Route A drives an end-to-end `netsuke` child with `assert_cmd` and configures
+  only that child with `Command::env`.
+- Route B calls a library entry point with its injected environment and keeps
+  assertions on in-process values such as `Cli`, `Manifest`, or `BuildGraph`.
 - Use `Slot<T>` for optional or replaceable scenario values.
 - Use typed wrappers in `tests/bdd/types.rs` for step parameters to avoid
   ambiguous string-heavy signatures.
@@ -1921,9 +1926,12 @@ Environment variable mutations and working-directory changes are process-global
 side effects that can cause data races when tests run in parallel. Tests inject
 environment readers where the API supports them, and configure child processes
 with `env_clear()` followed by `Command::env` where ambient discovery is part
-of the contract. `CwdGuard` is the RAII utility for restoring a process working
-directory after the few tests that exercise it. For locale-sensitive snapshot
-tests, use the `EnLocalizer` scoped pattern documented in the
+of the contract. BDD steps must not change either process-global value: use an
+injected environment and absolute paths for in-process library assertions, or
+an isolated `assert_cmd` child for end-to-end behaviour. `CwdGuard` is the RAII
+utility for the few direct CWD tests that deliberately exercise it. For
+locale-sensitive snapshot tests, use the `EnLocalizer` scoped pattern documented
+in the
 [snapshot testing guide](snapshot-testing-in-netsuke-using-insta.md#locale-pinned-snapshot-tests).
 
 `src/snapshot_test_support.rs` owns output-oriented unit-test fixtures;
@@ -2366,7 +2374,9 @@ let _env_lock = EnvLock::acquire();
 ```
 
 Do not use this lock to justify process-environment mutation. Environment
-access must remain injected, or confined to a spawned child process.
+access must remain injected, or confined to a spawned child process. BDD
+scenarios must not acquire this lock: they are in-process tests, and a lock
+would serialize the suite rather than isolate an ambient dependency.
 
 ### `CwdGuard`
 
@@ -2386,6 +2396,11 @@ std::env::set_current_dir(temp.path())?;
 Acquire `EnvLock` and then `CwdGuard` so Rust drops them in reverse declaration
 order: `CwdGuard` restores the CWD first, and `EnvLock` releases second.
 
+These direct CWD tests are the narrow exception for exercising CWD-dependent
+code itself. BDD scenarios instead retain an absolute manifest path or pass
+`-C/--directory` into the CLI; neither approach changes the harness process
+CWD.
+
 ### Injected and child-process environments
 
 `mutate_env_var` in `tests/bdd/helpers/env_mutation.rs` is the canonical way to
@@ -2409,14 +2424,22 @@ appropriate injected seam, such as `run_with_ninja_program`,
 `StdlibConfig::with_path_override`, `StdlibConfig::with_home_override`, or
 `StdlibConfig::with_command_path_override`. End-to-end tests may call
 `env_clear()` and then apply values with `Command::env`, because the mutation
-is confined to the child.
+is confined to the child. This defines two BDD routes: Route A drives the
+compiled binary with `assert_cmd` and configures only its child environment;
+Route B calls library entry points with an injected environment and keeps the
+scenario's in-process assertions on `Cli`, `Manifest`, `BuildGraph`, or render
+state.
 
 ### Ordering rules
 
 1. Inject environment-dependent inputs whenever the API supports them.
 2. Use an isolated child process for APIs whose contract is ambient discovery.
-3. Acquire `EnvLock` and then `CwdGuard` only for CWD-specific tests.
-4. Never mutate the harness process environment.
+3. In BDD, choose Route A for an end-to-end binary assertion or Route B for an
+   injected in-process library assertion.
+4. Retain absolute paths or pass `-C/--directory` instead of changing the BDD
+   harness CWD.
+5. Acquire `EnvLock` and then `CwdGuard` only for direct CWD-specific tests.
+6. Never mutate the harness process environment.
 
 ### `tracing_capture`
 
@@ -2496,14 +2519,12 @@ Table: Scenario state groups and fields
 | Localization state | `localization_lock`, `localization_guard`, `locale_config`, `locale_env`, `locale_cli_override`, `locale_system`, `resolved_locale`, `locale_message`                                                                                    | Scenario-level localizer overrides and resolution state.   |
 | HTTP server state  | `http_server`, `stdlib_url`                                                                                                                                                                                                              | Test HTTP server fixture for fetch scenarios.              |
 | Output state       | `output_mode`, `simulated_no_color`, `simulated_term`, `output_prefs`, `simulated_no_emoji`, `rendered_prefix`                                                                                                                           | Accessibility and output preference resolution.            |
-| Environment state  | `env_vars_forward`, `global_state_lock`                                                                                                                                                                                                  | Child environment map and the CWD-only scenario lock.      |
+| Environment state  | `env_vars_forward`                                                                                                                                                                                                                       | Child environment map for Route A scenarios.               |
 
 ### Key `TestWorld` methods
 
 - `track_env_var(key, new_value)` — update `env_vars_forward` so
   `build_netsuke_command` can configure the scenario's child process.
-- `ensure_global_state_lock()` — acquire the scenario-scoped CWD lock on first
-  use; subsequent calls are no-ops.
 
 ## Configuration merge architecture
 
@@ -2681,8 +2702,9 @@ uses the bare `EnvProvider` name.
 Tests for injected configuration discovery should provide a map-backed
 `ConfigEnvProvider`. End-to-end tests of the ambient `ConfigStdEnvProvider`
 adapter must run in an isolated child configured with `env_clear()` followed by
-`Command::env`. `EnvLock` is reserved for tests that change the process working
-directory alongside `CwdGuard`; it does not justify environment mutation.
+`Command::env`. BDD configuration steps use the injected route; direct CWD
+tests alone may use `EnvLock` alongside `CwdGuard`. Neither guard justifies
+environment mutation.
 
 Unit tests that only need to verify explicit config path precedence should test
 `explicit_config_path_with_env` with an injected provider instead of mutating

src/cli/discovery.rs

@@ -351,6 +351,10 @@ mod tracing_tests;
 #[path = "discovery_layer_tests.rs"]
 mod layer_tests;
 
+#[cfg(test)]
+#[path = "discovery_path_selection_tests.rs"]
+mod path_selection_tests;
+
 #[cfg(test)]
 #[path = "discovery_helper_proptests.rs"]
 mod helper_proptests;

src/cli/discovery_layer_tests.rs

@@ -1,8 +1,7 @@
 //! Tests for configuration file-layer collection.
 //!
-//! These cover which branch the shared file-layer boundary takes — explicit path
-//! versus automatic discovery — and the project-scope second pass. Selector
-//! precedence and event-schema snapshots live in the tracing test module.
+//! These cover explicit-versus-discovery collection and the project-scope second pass.
+//! Selector precedence and event-schema snapshots live in the tracing test module.
 use super::*;
 use crate::cli::test_support::TestEnv;
 use anyhow::{Context, Result, ensure};

src/cli/discovery_path_selection_tests.rs

@@ -0,0 +1,42 @@
+//! Tests for explicit configuration-path selection.
+//!
+//! These tests keep explicit relative selectors anchored to the process CWD
+//! instead of rebasing them beneath the CLI directory.
+
+use super::*;
+use crate::cli::test_support::TestEnv;
+use anyhow::{Context, Result, ensure};
+use tempfile::tempdir;
+
+/// An explicit relative configuration file does not use the CLI directory.
+#[test]
+fn explicit_relative_config_does_not_use_cli_directory() -> Result<()> {
+    let temp = tempdir().context("create temp dir")?;
+    let unique_dir_name = temp
+        .path()
+        .file_name()
+        .and_then(std::ffi::OsStr::to_str)
+        .context("temporary directory has a UTF-8 name")?;
+    let config_name = format!("{unique_dir_name}-relative-config.toml");
+    let config_path = temp.path().join(&config_name);
+    test_support::fs::write(&config_path, "emoji = \"always\"\n")
+        .context("write explicit config")?;
+    let cli = Cli {
+        config: Some(config_name.into()),
+        directory: Some(temp.path().to_path_buf()),
+        ..Cli::default()
+    };
+
+    let discovered = discover_file_layers(&cli, &TestEnv::default());
+    let error = discovered
+        .first_error()
+        .context("relative explicit config must not load from the CLI directory")?;
+
+    ensure!(
+        error
+            .to_string()
+            .contains("explicit configuration file not found"),
+        "expected missing explicit config error, got {error}"
+    );
+    Ok(())
+}

tests/bdd/fixtures/mod.rs

@@ -20,16 +20,8 @@ use std::collections::HashMap;
 use std::ffi::OsString;
 use std::path::PathBuf;
 use std::sync::MutexGuard;
-use test_support::CwdGuard;
-use test_support::env_lock::EnvLock;
 use test_support::http::HttpServer;
 
-#[derive(Debug)]
-struct GlobalStateGuard {
-    env_lock: EnvLock,
-    cwd_guard: CwdGuard,
-}
-
 /// Combined test world for all BDD scenarios.
 ///
 /// Non-Clone types are stored in `RefCell<Option<T>>` to allow interior
@@ -149,28 +141,9 @@ pub struct TestWorld {
     // Environment state
     /// Values supplied to child Netsuke processes for scenario-tracked variables.
     pub env_vars_forward: RefCell<HashMap<String, OsString>>,
-    /// Scenario-scoped lock for the few remaining process-global CWD operations.
-    global_state_lock: RefCell<Option<GlobalStateGuard>>,
 }
 
 impl TestWorld {
-    /// Acquire the scenario lock before changing the process working directory.
-    ///
-    /// # Errors
-    ///
-    /// Returns an error if the current working directory cannot be captured.
-    pub fn ensure_global_state_lock(&self) -> std::io::Result<()> {
-        if self.global_state_lock.borrow().is_none() {
-            let env_lock = EnvLock::acquire();
-            let cwd_guard = CwdGuard::acquire()?;
-            *self.global_state_lock.borrow_mut() = Some(GlobalStateGuard {
-                env_lock,
-                cwd_guard,
-            });
-        }
-        Ok(())
-    }
-
     /// Set or remove a variable in the child-process environment specification.
     pub fn track_env_var(&self, key: String, forward_value: Option<OsString>) {
         if let Some(value) = forward_value {
@@ -206,14 +179,6 @@ impl Drop for TestWorld {
         self.localization_guard.borrow_mut().take();
         self.localization_lock.borrow_mut().take();
         self.env_vars_forward.borrow_mut().clear();
-        if let Some(GlobalStateGuard {
-            env_lock,
-            cwd_guard,
-        }) = self.global_state_lock.borrow_mut().take()
-        {
-            drop(cwd_guard);
-            drop(env_lock);
-        }
         self.stdlib_text.clear();
     }
 }