[auto-wood]

Summary

Maintenance closeout iteration closing the two actionable v1.225 residuals (store open backlog: 3 → 1, remaining item is the external Windows-runtime verification).

• I-000121 v226-cli-config-error: nexus42 startup surfaces the contextual config parse error as an explicit stderr warning (config path + cause-chain reason via CliConfig::path()/{error:#}), then continues with defaults — zero behavior change beyond surfacing. Regression tests cover corrupt-TOML and invalid-UTF-8 classes (fail-before recorded).
• I-000122 v226-test-wait-hardening: the ensure_system_layout under-lock re-check regression test no longer relies on a fixed 100 ms sleep; a #[cfg(test)] rendezvous sequences the document commit after boot's pre-lock existence check (deterministic 20/20; witnessed mutant fail→pass). Deviation from the plan's Shape A adjudicated by QC and recorded in the plan contract.

Gates

Plan	QC	QA (mandatory)
v226-cli-config-error	Approve (1 fix round)	PASS
v226-test-wait-hardening	Approve	PASS

Issues closed with evidence: I-000121, I-000122. Store open after merge: I-000029 only.

[auto-wood]

Round 1 — full review (head 1855b4470)

Verdict: ship it

Summary

Two-plan closeout composes cleanly: corrupt CLI config now warns on stderr and continues on defaults (load_startup_config + serial HOME-isolated tests); ensure_system_layout race is hardened with a test-only rendezvous instead of sleep. Empty close commit is harness-only. No unintended public API expansion (load_startup_config is private; rendezvous is #[cfg(test)]).

Findings (1 nit, 0 blocking)

• nit crates/nexus-home-layout/src/lib.rs (~1584–1604): clear AFTER_EXISTENCE_CHECK on drop/guard so a panicking test cannot leak rendezvous state to a later test in-process.

Tally

must-fix 0 · should-fix 0 · nit 1

Static review only (diff + composition); local report: .mstar/sdd/v226-loop/report-r1.md.

[greptile-apps]

Confidence Score: 5/5

[Low impact] The PR appears safe to merge; no blocking issues remain.

Summary

The PR adds startup config warnings and replaces a sleep-based regression test with a test-only rendezvous.

• The CLI names config errors and continues with defaults.
• The config race test orders its write without a timed delay.

_{Reviews (3) · Last reviewed commit: "fix(cli): load startup config from an ex..." · Reviewed by Greptile}

[auto-wood]

⚠️ Verdict: needs fixes · Score 86% (good)

Findings	Count
🔴 must-fix	0
🟠 should-fix	1
🔵 nit	1
❓ unverified	0

Round 2 re-review of d2a34d0bb (increment over 1855b4470, which is still an ancestor). Round-1 verdict was ship it at 98%; the round-1 nit is unchanged, and the new increment does not close the Windows isolation gap it targets, so the verdict moves to needs fixes.

📋 Review

What this PR does: v1.226 maintenance closeout — CLI startup degrades a corrupt config.toml to defaults with a stderr warning (plus targeted main.rs tests), and nexus-home-layout replaces a timing-based race with a #[cfg(test)] rendezvous before config-lock acquisition. The round-2 increment is test-only: TempHome now swaps a platform-selected home env (USERPROFILE on Windows, HOME elsewhere) instead of HOME only.

Revalidated findings

Prior finding	State	Evidence
🔵 Rendezvous slot cleared only on the happy path (AFTER_EXISTENCE_CHECK = None at test end, no Drop guard)	still present	crates/nexus-home-layout/src/lib.rs:1604-1605 — the write is still the last statement of the happy path; a panic before it leaves the slot populated for a later test in the same process. Increment does not touch this file.

Findings

🟠 Windows isolation is inert: dirs 7 resolves the home via SHGetKnownFolderPath, not USERPROFILE

• Evidence: apps/nexus42/src/main.rs:181-184 selects USERPROFILE on Windows and the doc comment claims it is "the env var dirs::home_dir() reads on this platform". dirs 7.0.0 does not: dirs-7.0.0/src/win.rs:5 → dirs_sys::known_folder_profile() → known_folder(Shell::FOLDERID_Profile) (dirs-sys-0.5.0/src/lib.rs:172), i.e. SHGetKnownFolderPath, which reads the profile from the user token/registry. dirs-sys-0.5.0 contains no USERPROFILE reference at all. The config path under test comes from CliConfig::config_path() → nexus_home() → dirs::home_dir() (apps/nexus42/src/config.rs:330-331, 491-495).
• Impact: on Windows the tests still read the real %USERPROFILE%\.nexus42\config.toml while writing/asserting against the temp dir, so the corrupt_config_yields_defaults_and_path_warning / unreadable_config_warning_still_names_the_path assertions still fail against a real user profile. The Greptile P1 is not closed — the change is a semantic no-op on every platform (unix still HOME, unchanged; Windows swaps one ignored variable for another). CI does not catch it because the Rust test jobs run on ubuntu-latest only.
• Merge class: should-fix
• Confidence: HIGH
• Fix sketch: drop the env indirection — give the production path a seam the test can drive, e.g. pub(crate) fn load_at(nexus: &Path) around the existing private CliConfig::read_document(nexus: &Path) (config.rs:197) and load_startup_config(nexus: &Path), with the test passing home.tmp.path(); no env override exists that redirects a known-folder lookup on Windows.

🔵 Rendezvous slot still cleared only on the happy path

• Evidence: crates/nexus-home-layout/src/lib.rs:1604-1605 — *AFTER_EXISTENCE_CHECK.lock() = None is the last statement of the test body.
• Impact: a panic between the rendezvous install and the clear leaks the slot into a later test in the same process; test-only, and the current test's assertions are reached before any panic point in practice. Carried forward unchanged from round 1.
• Merge class: nit
• Confidence: HIGH
• Fix sketch: hold the guard in a small RAII struct with Drop clearing the slot when the recorded home matches.

Linked-issue AC

none

✅ Verified

• git merge-base --is-ancestor 1855b4470 d2a34d0bb → true; increment read as 1855b4470..d2a34d0bb (1 file, +12/−5, test module only).
• git show d2a34d0bb --stat → apps/nexus42/src/main.rs only; no production path touched, load_startup_config still private to main.rs.
• TempHome save/set/restore symmetry: new() reads HOME_ENV into previous before creating the temp dir, sets it after; Drop restores Some / removes None — symmetric on both cfg arms, OsString round-trip is lossless.
• #[cfg(windows)] / #[cfg(not(windows))] const pair matches the bin/nexus-runtime.rs::resolve_home precedent (#[cfg(unix)] HOME / #[cfg(windows)] USERPROFILE); all three tests remain #[serial_test::serial], so the env mutation stays single-threaded.
• ❓ Lead (out of this diff): the same premise drives apps/nexus42/src/bin/nexus-runtime.rs:139-142, so on Windows --home / NEXUS42_HOME cannot reach any dirs::home_dir()-based read either — pre-existing, not tallied here.

🗑️ Considered & rejected

• "Env-based isolation is the wrong shape entirely on Windows; delete the guard": rejected as a separate finding — it is the same root cause as the should-fix above (no env var redirects a known-folder lookup), so one finding carries it.
• "std::env::set_var is thread-unsafe / unsound in Rust 2024": rejected — the suite already serialises these tests via #[serial], and the increment does not add a new parallel caller.
• "USERPROFILE should be set together with HOME on Windows for defence in depth": rejected — no reader in this crate path consults HOME on Windows; it would add a variable nobody reads.

🛠️ Plan to fix

Expand fix plan

1. should-fix — replace env-based isolation with a path seam
   - config.rs: expose the existing `read_document(nexus: &Path)` path via
     `pub(crate) fn load_at(nexus: &Path) -> Result<Self>` on `CliConfig`.
   - main.rs: `fn load_startup_config(nexus: &Path)`; `async_main` passes
     `nexus_home()?` (behaviour unchanged for the binary).
   - tests: delete `HOME_ENV` + `TempHome`; pass `home.tmp.path()` and assert
     on the returned warning path. Tests no longer depend on any env var, so
     they are valid on every platform.
2. nit — RAII guard for the rendezvous slot (optional, unchanged from round 1).
3. Verification: `cargo test -p nexus42 --lib --features connect-host` (CI owns
   the full suite); note the Windows gap stays unobservable until a Windows
   test leg or a local `--target x86_64-pc-windows-msvc` run exists.

[auto-wood]

✅ Verdict: ship it · Score 98% (mergeable)

Findings	Count
🔴 must-fix	0
🟠 should-fix	0
🔵 nit	1
❓ unverified	0

R3 revalidation on 8e4c2058d: the round-2 should-fix (Windows-inert env guard) is fixed. Verdict returns to ship it (round-1 nit on home-layout rendezvous unchanged).

📋 Review

What changed in R3: CliConfig::load_from_nexus_root + CliConfig::path_in expose the loader seam; private load_at(nexus) in main.rs holds warning logic; load_startup_config() still resolves nexus_home() then delegates. Startup tests call load_at(temp_nexus_root) with no HOME/USERPROFILE/serial_test/TempHome.

Revalidated findings

Prior finding	State	Evidence
🟠 Windows isolation inert (USERPROFILE / dirs 7 known-folder)	fixed	apps/nexus42/src/main.rs:76-90,191-243 — tests use load_at(nexus.path()) → CliConfig::load_from_nexus_root(nexus) → load_from_nexus_home / read_document (config.rs:192-224); no dirs::home_dir() on the test path. Grep: no HOME_ENV/TempHome/USERPROFILE in main.rs tests.
🔵 Rendezvous slot cleared only on happy path (nexus-home-layout)	still present	crates/nexus-home-layout/src/lib.rs:1604 — unchanged; nit only.

Findings

none (no new accepted findings in 8e4c2058d)

Linked-issue AC

none

✅ Verified

• Public API: pub fn load_from_nexus_root / pub fn path_in are appropriate — main.rs is a separate bin crate, so pub(crate) on CliConfig would not reach tests; mirrors existing load_from_home(home) + nexus_root_from_home split (config.rs:178-194).
• Contract: tests treat the temp dir as the Nexus root (path_in → nexus/config.toml), matching production where load_startup_config passes nexus_home() (~/.nexus42), not the user home directory.
• Binary path unchanged: async_main still calls parameterless load_startup_config() (main.rs:107-109); nexus_home() failure still degrades to defaults with a warning (95-104).
• Implementer evidence (not re-run here): scoped lib 681 green, clippy/fmt clean.

🗑️ Considered & rejected

• "Widen API surface is unnecessary — only test hook needed": rejected — bin/lib split requires pub on the library type; two entry points (load_from_home vs load_from_nexus_root) document distinct roots and match existing layout helpers.

🛠️ Plan to fix

none