From c8996de94d925dd062d05aca8e767989ad6db69a Mon Sep 17 00:00:00 2001 From: Hare Date: Sat, 30 May 2026 01:52:48 +0900 Subject: [PATCH] review: approve nix manifest profile foundation --- .../thread.md | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/work-items/open/20260527-000022-manifest-profiles/thread.md b/work-items/open/20260527-000022-manifest-profiles/thread.md index 39d74fe9..d7c783e2 100644 --- a/work-items/open/20260527-000022-manifest-profiles/thread.md +++ b/work-items/open/20260527-000022-manifest-profiles/thread.md @@ -24,4 +24,38 @@ Initial implementation target: The child orchestrator may split implementation among sub-Pods, but final merge/close remains parent-side. +--- + + + +## Review: approve + +Reviewed the nested Pod implementation from branch `work/nix-manifest-profiles`. + +Result: approved after blocking fix. + +Findings: + +- Initial review found one blocking issue: `--profile` rejected non-empty `INSOMNIA_USER_MANIFEST`. That contradicted the profile design because a selected Nix profile is a standalone runtime config artifact and ambient user/project manifest cascade should not affect it. +- The child orchestrator fixed this in commit `c9a175a fix: ignore user manifest for profiles` and added `profile_ignores_non_empty_user_manifest_env` without invoking real Nix. +- Profile foundation is intentionally a vertical slice: explicit path-based Nix profile resolution, minimal Nix helper, CLI/TUI spawn entrypoints, resolved snapshot metadata, and restore-from-snapshot behavior. Rich discovery/default picker remains future work. + +Validation run by reviewer: + +- `cargo fmt --check` +- `cargo test -p manifest profile -- --nocapture` +- `cargo test -p pod --bin insomnia-pod profile -- --nocapture` +- `cargo check -p session-store -p manifest -p pod -p client -p tui -p provider` +- `cargo check -p pod -p tui` +- `git diff --check` +- Manual `nix eval --json --file` smoke check for `resources/nix/profile-lib.nix` + +Non-blocking follow-up candidates: + +- Hide or narrow `ResolvedProfile::raw_artifact` if future call sites might log/persist accidental raw Nix output. +- Add a timeout around `nix eval` so profile startup cannot hang indefinitely. +- Validate direct `client::SpawnConfig` construction that combines `profile_path` with `resume_from`; TUI currently avoids it. +- Build richer profile discovery/default selection and the full TUI profile picker. + + ---