Summary
Netsuke reads environment variables ambiently in nine production modules and mutates the process environment in fourteen test files. The mutations are serialized behind a process-wide Mutex in test_support::env_lock::EnvLock. This issue tracks replacing that arrangement with injected environments, retiring the lock, and gating against its return.
Commit 3f84545 rewrote the AGENTS.md testing section to mandate this. The remaining work makes the codebase comply.
Why the previous guidance failed
The old rule named dependency injection and mockable as "preferred", then sanctioned a fallback: wrap mutations in shared guards and mutexes. A fallback that is easier than the preferred path becomes the default, and it did. Four consequences followed.
The lock became load-bearing. EnvLock now serializes the behavioural suite, the CLI config suite, the which suites, and one in-crate unit test module. Tests that never touch the environment still queue behind those that do, because the lock is scenario-scoped rather than mutation-scoped.
Property-based testing became impossible. #383 and #317 both ask for proptest coverage of environment-driven behaviour. Proptest runs hundreds of cases in one process; a global mutex per case is not a viable execution model. Both issues have been blocked on this for months without the reason being recorded.
The mock stopped being a mock. test_support/src/env.rs:42 implements EnvMut for MockEnv by calling std::env::set_var — mutating the real process environment while the double's expectations continue returning their programmed values. Tests believing they are isolated are writing global state. Tracked as #490.
Ambient reads stayed in production signatures. Because tests could always reach around a function to set the variable it reads, no pressure ever accumulated to give those functions a parameter. Nine modules still read std::env directly.
Work breakdown
Phase 0 — decision (done)
Phase 1 — production seams
Each threads &impl mockable::Env through a module and supplies mockable::DefaultEnv at its public boundary. Independent of one another; parallelisable.
Phase 2 — test-support correctness
Phase 3 — suite migration
Each depends on the Phase 1 seam for its subject module.
Phase 4 — removal and enforcement
Phase 5 — documentation
Unblocked on completion
Scope boundaries
Working-directory mutation is in scope only where EnvLock already covers it. The lock serializes std::env::set_current_dir alongside environment writes, so #492 must address both or the mutex survives. Injecting a working-directory capability more broadly — for example the ambient std::env::current_dir() read at src/cli/discovery.rs:122, or Dir::open_ambient_dir in path_utils.rs — is a separate concern, related to #173, and is deliberately excluded.
serial_test is not a migration target. Replacing a bespoke mutex with an attribute-driven one changes the spelling, not the property. The mandate disqualifies both.
Subprocess isolation is not a laundering route. assert_cmd with Command::env is exempt because the mutation is confined to a child process, which is the honest shape for end-to-end scenarios. Converting a unit test into a subprocess invocation to escape the rule trades a fast, precise test for a slow, coarse one, and should be refused in review.
Success criteria
Checklist refresh (2026-08-12)
Ticked the twelve children that are now closed. #491 folded into #493; #317 folded into #385. #483's convergence decision is expected to be resolved by PR #558 (OrthoConfig v0.9.0), which projects ConfigEnvProvider into OrthoConfig's environment adapters — #483, and therefore #385, are blocked on that merge.
Summary
Netsuke reads environment variables ambiently in nine production modules and mutates the process environment in fourteen test files. The mutations are serialized behind a process-wide
Mutexintest_support::env_lock::EnvLock. This issue tracks replacing that arrangement with injected environments, retiring the lock, and gating against its return.Commit 3f84545 rewrote the AGENTS.md testing section to mandate this. The remaining work makes the codebase comply.
Why the previous guidance failed
The old rule named dependency injection and
mockableas "preferred", then sanctioned a fallback: wrap mutations in shared guards and mutexes. A fallback that is easier than the preferred path becomes the default, and it did. Four consequences followed.The lock became load-bearing.
EnvLocknow serializes the behavioural suite, the CLI config suite, thewhichsuites, and one in-crate unit test module. Tests that never touch the environment still queue behind those that do, because the lock is scenario-scoped rather than mutation-scoped.Property-based testing became impossible. #383 and #317 both ask for proptest coverage of environment-driven behaviour. Proptest runs hundreds of cases in one process; a global mutex per case is not a viable execution model. Both issues have been blocked on this for months without the reason being recorded.
The mock stopped being a mock.
test_support/src/env.rs:42implementsEnvMut for MockEnvby callingstd::env::set_var— mutating the real process environment while the double's expectations continue returning their programmed values. Tests believing they are isolated are writing global state. Tracked as #490.Ambient reads stayed in production signatures. Because tests could always reach around a function to set the variable it reads, no pressure ever accumulated to give those functions a parameter. Nine modules still read
std::envdirectly.Work breakdown
Phase 0 — decision (done)
Phase 1 — production seams
Each threads
&impl mockable::Envthrough a module and suppliesmockable::DefaultEnvat its public boundary. Independent of one another; parallelisable.src/cli/discovery.rs) (seam landed as bespokeEnvProvider; convergence expected via PR Adopt OrthoConfig v0.9.0 #558 — re-verify and close on its merge)env()Jinja function (src/manifest/mod.rs)whichPATH/PATHEXTcapture (src/stdlib/which/env.rs)src/stdlib/path/path_utils.rs)src/stdlib/which/lookup/workspace/mod.rs)src/runner/process/mod.rs)EnvProviderand theread_envclosures onmockable::EnvPhase 2 — test-support correctness
EnvMut for MockEnvmutates the real environment (defect; can start immediately)unsafe EnvMut/set_varAPI rather than wrap it (rescoped)Phase 3 — suite migration
Each depends on the Phase 1 seam for its subject module.
src/manifest/tests/workspace.rs, the lastEnvLockuser undersrc/(folded into Migrate the last EnvLock users onto injected seams (env_path_tests + manifest workspace tests) #493)rstest-bddscenarios, including working-directory mutationPhase 4 — removal and enforcement
EnvLock,EnvGuard,EnvVarGuard,CwdGuard; add a lint or CI gate against reintroductionEnvLockis retired rather than hardened (rescoped)Phase 5 — documentation
Unblocked on completion
NETSUKE_NINJAresolver invariants (needs Inject an Env seam into Ninja program resolution #488)Scope boundaries
Working-directory mutation is in scope only where
EnvLockalready covers it. The lock serializesstd::env::set_current_diralongside environment writes, so #492 must address both or the mutex survives. Injecting a working-directory capability more broadly — for example the ambientstd::env::current_dir()read atsrc/cli/discovery.rs:122, orDir::open_ambient_dirinpath_utils.rs— is a separate concern, related to #173, and is deliberately excluded.serial_testis not a migration target. Replacing a bespoke mutex with an attribute-driven one changes the spelling, not the property. The mandate disqualifies both.Subprocess isolation is not a laundering route.
assert_cmdwithCommand::envis exempt because the mutation is confined to a child process, which is the honest shape for end-to-end scenarios. Converting a unit test into a subprocess invocation to escape the rule trades a fast, precise test for a slow, coarse one, and should be refused in review.Success criteria
std::env::var/var_oscall outside aDefaultEnvadapter undersrc/.std::env::set_var/remove_var/set_current_dircall anywhere undersrc/,tests/, ortest_support/, saveCommand::envbuilder calls.EnvLockand the mutation guards are deleted, and a lint gate demonstrably fails on a reintroduced violation.Checklist refresh (2026-08-12)
Ticked the twelve children that are now closed. #491 folded into #493; #317 folded into #385. #483's convergence decision is expected to be resolved by PR #558 (OrthoConfig v0.9.0), which projects
ConfigEnvProviderinto OrthoConfig's environment adapters — #483, and therefore #385, are blocked on that merge.