🟡 Changes recommended

Path expansion can still select the working directory unexpectedly, and fallback lookup and resource-cleanup defects remain.

• Close token handle when environment block creation fails · New
• Correct CommonProgramFiles allowlist entries · New
• Use non-raising lookup for absent allowlisted variables · New
• Reject missing variables concatenated with other text · New

This PR addresses #427 by recovering missing Windows environment variables during configuration loading, helping prevent Python Manager from resolving installation paths against the working directory.

Changes:

• Adds a native helper to generate a default Windows environment and exposes it through both builds.
• Adds configuration fallback lookups and rejects empty substitutions before path separators.
• Adds absolute-path detection and tests for configuration expansion and fallback behavior.

Review effort: Balanced

🔵 Needs a closer look

Explicit environment mappings are ignored when empty and are not forwarded during nested configuration resolution.

No open issues 🎉

In code that hasn't changed since last review

src/​manage/​config.py:259

The new environ argument is not forwarded by the recursive call on line 275. Nested settings such as install.source and list.format therefore use the process environment instead of the supplied mapping, unlike top-level settings. A value consisting solely of a variable supplied only in that mapping is silently removed. Forward environ when resolving nested schemas so all levels use the same environment.

src/​manage/​config.py:283

An explicitly supplied empty environment is ignored because {} is falsy. For example, resolving %AppData%/Python with environ={} reads the process environment and OS fallbacks instead of reporting the missing variable. Select the default only when environ is None so an intentionally empty mapping is respected.

• Reject missing variables concatenated with other text
• Use non-raising lookup for absent allowlisted variables
• Correct CommonProgramFiles allowlist entries
• Close token handle when environment block creation fails

Review effort: Balanced

🟡 Changes recommended

Unresolved configuration fallback, strict path expansion, path utility, and test failures block approval.

• Reset cached environment wrapper before fallback test · New

In code that hasn't changed since last review

src/​manage/​config.py:254

This only raises when the placeholder is followed by a path separator. A path-valued setting whose entire value is %TEMP% or %AppData% still expands to an empty string, and resolve_config then deletes the setting, allowing a later default (such as the current directory) instead of failing as promised. Apply strict empty-substitution validation to path-valued settings while preserving optional expansion for non-path settings.

src/​manage/​pathutils.py:110

drive and root returns the root string rather than a boolean (for example, an absolute drive path returns "\\"). That makes is_absolute() type-inconsistent with the pathlib-style API and breaks callers that use is True or otherwise require a boolean; return bool(drive and root) instead.

Review effort: Lite

🔵 Needs a closer look

Unresolved moderate issues remain in log-directory fallback, empty-variable validation without separators, and UNC-path absolute detection.

No open issues 🎉

In code that hasn't changed since last review

src/​manage/​config.py:190

Adding TEMP/TMP here does not protect the log path in the reported minimal-environment case: BaseCommand.get_log_file() still calls os.getenv("TMP") or os.getenv("TEMP") or os.getcwd(), so when both variables are absent it writes into the caller's current directory. Please route log-directory resolution through the same fallback environment (or fail rather than choosing the CWD), otherwise this part of #427 remains reproducible.

• Reset cached environment wrapper before fallback test

Review effort: Lite