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 index dc3e0fcf..92d7899c 100644 --- a/work-items/open/20260531-104614-pure-path-fallback-tests/item.md +++ b/work-items/open/20260531-104614-pure-path-fallback-tests/item.md @@ -7,7 +7,7 @@ kind: task priority: P2 labels: [test, env, manifest, cleanup] created_at: 2026-05-31T10:46:14Z -updated_at: 2026-05-31T10:46:51Z +updated_at: 2026-05-31T10:54:18Z assignee: null legacy_ticket: null --- 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 index ae756992..fad86e35 100644 --- a/work-items/open/20260531-104614-pure-path-fallback-tests/thread.md +++ b/work-items/open/20260531-104614-pure-path-fallback-tests/thread.md @@ -68,4 +68,35 @@ Validation: - 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. + + ---