## Created Created by tickets.sh create. --- ## Plan Preflight classification: implementation-ready. Intent: - Remove unnecessary process-global environment mutation from path fallback tests by making the fallback/precedence logic directly testable with explicit inputs. Requirements: - Start with `manifest::paths`, where path resolution is centralized and currently owns env fallback tests. - Use small per-key pure helpers with direct optional path inputs. Do not introduce a general `PathEnv`, `EnvSnapshot`, env trait, or test-support crate. - Keep public runtime behavior unchanged: the production wrappers still read the same env vars, ignore empty path env values as before, and resolve the same fallback order. - Keep docs/environment.md behavior aligned with code. Current code map: - `crates/manifest/src/paths.rs`: core config/data/runtime/resource path resolution and env fallback tests using local `EnvGuard`. - `docs/environment.md`: documented fallback order for home/config_dir/data_dir/runtime_dir/resource_dir. - Other env mutations in `pod`/`pod-registry` are mostly subprocess/runtime-isolation integration setup and should not be force-fixed in this ticket. - Provider credential env mutation belongs to `manifest-profile-encrypted-secrets`, not this ticket. Critical risks: - Accidentally changing path precedence while making tests pure. - Creating an over-broad abstraction that keeps env mutation around indirectly. - Treating spawned-process runtime isolation env setup as equivalent to pure fallback tests and breaking integration tests. Intent packet for implementation: Intent: - Make `manifest::paths` fallback tests pure and independent from process env mutation. Requirements: - Extract narrow pure helper functions for fallback precedence. - Convert fallback-order tests to call those helpers directly. - Keep production path functions and documented behavior unchanged. - Do not introduce a new crate, `PathEnv` struct, env trait, or test-only env var. Invariants: - Path authority remains in `manifest::paths`. - No new environment-variable surface. - No provider credential/env cleanup in this ticket. - No Pod runtime/process model changes. Non-goals: - Eliminating all `set_var`/`remove_var` in the repository. - Reworking resource packaging lookup beyond preserving behavior. Escalate if: - Removing env mutation requires changing public APIs or path semantics. - Resource lookup behavior is ambiguous enough to need a product decision. - Pod runtime integration tests appear to need broader runtime-dir API changes. Validation: - `cargo fmt --check` - `cargo test -p manifest paths` - relevant additional manifest tests if names differ - `cargo check -p manifest` - `./tickets.sh doctor` - `git diff --check` - report remaining `set_var`/`remove_var` in touched areas and why they remain, if any. --- ## Review: approve External reviewer: `pure-path-fallback-reviewer-20260531` Reviewed implementation commit: `e232f54` (`test: make path fallback tests pure`) Verdict: approve Summary: - `manifest::paths` fallback/precedence logic was split into private pure helpers. - Tests now pass direct `Option` inputs to verify fallback order rather than mutating process environment. - No `PathEnv`/`EnvSnapshot`/trait/test-support crate was introduced. - The change is limited to `crates/manifest/src/paths.rs`. Requirements mapping: - `config_dir`, `data_dir`, and `runtime_dir` public wrappers still read the same env keys and pass them in the existing precedence order. - Empty env values still become `None` through the retained `env_path`/path conversion behavior. - `docs/environment.md` fallback order matches the code. - `git grep -n -E 'set_var\(|remove_var\(' -- crates/manifest/src/paths.rs` has no matches. - No credential env behavior, Pod runtime/process behavior, or new env var surface was changed. Blockers: none. Non-blocking follow-ups: none. Validation adequacy: - Coder validation covered fmt, focused/full manifest tests, manifest check, doctor, diff check, and set_var/remove_var grep. - Reviewer performed read-only diff/source/docs consistency review and grep/diff-check spot checks; no additional Cargo tests were rerun by reviewer. --- ## Closed Made `manifest::paths` fallback tests independent from process-global environment mutation. Implementation: - Added narrow private per-key fallback helpers for config/data/runtime path resolution. - Public path functions still read the same env vars and pass them in the same precedence order. - Empty path env values still resolve as unset. - Fallback-order tests now pass direct `Option` inputs instead of using `std::env::set_var` / `remove_var`. - No `PathEnv`, `EnvSnapshot`, env trait, test-support crate, or new env var surface was introduced. - Credential env behavior and Pod runtime/process behavior were not changed. Review: - External reviewer `pure-path-fallback-reviewer-20260531` approved implementation commit `e232f54`. Validation after merge: - `cargo fmt --check` - `cargo test -p manifest paths` - `cargo test -p manifest` - `cargo check -p manifest` (passed with unrelated existing `llm-worker` dead_code warning) - `./tickets.sh doctor` - `git diff --check` - `git grep -n -E 'set_var\(|remove_var\(' -- crates/manifest/src/paths.rs || true` produced no matches. ---