diff --git a/work-items/open/20260531-104614-pure-path-fallback-tests/artifacts/.gitkeep b/work-items/open/20260531-104614-pure-path-fallback-tests/artifacts/.gitkeep new file mode 100644 index 00000000..e69de29b diff --git a/work-items/open/20260531-104614-pure-path-fallback-tests/item.md b/work-items/open/20260531-104614-pure-path-fallback-tests/item.md new file mode 100644 index 00000000..dc3e0fcf --- /dev/null +++ b/work-items/open/20260531-104614-pure-path-fallback-tests/item.md @@ -0,0 +1,57 @@ +--- +id: 20260531-104614-pure-path-fallback-tests +slug: pure-path-fallback-tests +title: Tests: make path fallback tests independent from process env +status: open +kind: task +priority: P2 +labels: [test, env, manifest, cleanup] +created_at: 2026-05-31T10:46:14Z +updated_at: 2026-05-31T10:46:51Z +assignee: null +legacy_ticket: null +--- + +## Background + +The environment-variable cleanup removed test-only env surfaces such as `INSOMNIA_TEST_*` and the Pod runtime command env override. The next issue is that several tests still mutate process-global environment even when they only need to verify deterministic fallback/precedence logic. + +The user clarified the intended direction: + +- even env vars used by the real application do not need to be read from process env in most tests; +- tests should verify fallback/precedence behavior with direct inputs; +- do not introduce a `PathEnv`/`EnvSnapshot` abstraction or a shared test-support crate just to preserve process-env mutation; +- write the fallback helpers directly and narrowly where needed. + +This ticket starts with path resolution, where the behavior is centralized and the env mutation is avoidable. + +## Requirements + +- In `manifest::paths`, separate fallback/precedence logic from process env reads enough that fallback tests can call pure helpers with direct `Option`-style inputs. +- Prefer small per-key helpers over a general `PathEnv` struct/trait. + - Example shape: `resolve_config_dir_from_parts(...)`, `resolve_data_dir_from_parts(...)`, `resolve_runtime_dir_from_parts(...)`, or equivalent private helpers. + - Keep helper visibility private or `pub(crate)` only if needed by nearby tests. +- Keep runtime behavior unchanged: + - public path functions still read the same supported env vars in the same order; + - empty env values keep the current unset-equivalent behavior; + - resource lookup fallback behavior remains compatible with packaging/dev use. +- Convert path fallback tests that currently use `std::env::set_var` / `remove_var` to direct helper tests where possible. +- Do not add a new helper crate or broad env abstraction. +- Do not remove credential env behavior in this ticket; that belongs to `manifest-profile-encrypted-secrets`. +- Do not attempt to eliminate subprocess integration env setup if a spawned process still needs runtime isolation; report any remaining env mutation rather than forcing risky API changes. + +## Non-goals + +- Removing all env reads from production code. +- Removing provider credential env support. +- Redesigning runtime directory authority or Pod process startup. +- Introducing test-support / EnvSnapshot / trait-based environment abstraction. +- Changing documented path semantics. + +## Acceptance criteria + +- `manifest::paths` fallback/precedence tests no longer mutate process-global environment just to test fallback order. +- The fallback order documented in `docs/environment.md` still matches the code. +- Any remaining `set_var` / `remove_var` in touched tests is either eliminated or explicitly justified in the implementation report. +- No new environment variable surface is introduced. +- `cargo fmt --check`, `cargo test -p manifest paths`, relevant affected tests, `cargo check -p manifest`, `./tickets.sh doctor`, and `git diff --check` pass. diff --git a/work-items/open/20260531-104614-pure-path-fallback-tests/thread.md b/work-items/open/20260531-104614-pure-path-fallback-tests/thread.md new file mode 100644 index 00000000..ae756992 --- /dev/null +++ b/work-items/open/20260531-104614-pure-path-fallback-tests/thread.md @@ -0,0 +1,71 @@ + + +## 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. + + +---