Skip to content

Commit 6dfe97c

Browse files
committed
feat: add refresh_path_from_registry() to fix Windows PATH propagation
After winget installs Node.js, the registry PATH is updated but the running Python process still has the old os.environ['PATH']. This causes npm commands to fail because the subprocess cannot find npm. This commit: - Adds refresh_path_from_registry() to read PATH from Windows registry - Splits install_dependencies() into platform-specific and common phases - Calls PATH refresh between phases so npm can find Node.js The function reads both system (HKLM) and user (HKCU) PATH values, handles REG_EXPAND_SZ with environment variable expansion, and combines them with system PATH first for security.
1 parent 883ec81 commit 6dfe97c

File tree

2 files changed

+270
-18
lines changed

2 files changed

+270
-18
lines changed

scripts/setup_environment.py

Lines changed: 129 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -697,6 +697,109 @@ def cleanup_temp_paths_from_registry() -> tuple[int, list[str]]:
697697
return 0, []
698698

699699

700+
def refresh_path_from_registry() -> bool:
701+
"""Refresh os.environ['PATH'] from Windows registry.
702+
703+
Reads both system and user PATH values from the Windows registry and
704+
updates os.environ['PATH'] with the combined value. This addresses
705+
the Windows PATH propagation bug where installations (e.g., winget)
706+
update the registry but running processes don't see the changes.
707+
708+
Registry sources:
709+
- System PATH: HKEY_LOCAL_MACHINE\\SYSTEM\\CurrentControlSet\\Control\\Session Manager\\Environment
710+
- User PATH: HKEY_CURRENT_USER\\Environment
711+
712+
Returns:
713+
True if PATH was successfully refreshed, False otherwise.
714+
715+
Note:
716+
- Only works on Windows (returns True on other platforms as no-op)
717+
- Handles REG_EXPAND_SZ values with environment variable expansion
718+
- Combines system PATH + user PATH (system first for security)
719+
- Logs info message when PATH is refreshed
720+
"""
721+
if sys.platform == 'win32':
722+
try:
723+
724+
def expand_env_vars(value: str) -> str:
725+
"""Expand environment variables like %USERPROFILE% in registry values."""
726+
# Use os.path.expandvars which handles %VAR% on Windows
727+
return os.path.expandvars(value)
728+
729+
system_path = ''
730+
user_path = ''
731+
732+
# Read system PATH from HKEY_LOCAL_MACHINE
733+
try:
734+
with winreg.OpenKey(
735+
winreg.HKEY_LOCAL_MACHINE,
736+
r'SYSTEM\CurrentControlSet\Control\Session Manager\Environment',
737+
0,
738+
winreg.KEY_READ | winreg.KEY_WOW64_64KEY,
739+
) as key:
740+
raw_value, reg_type = winreg.QueryValueEx(key, 'Path')
741+
if raw_value:
742+
# Expand environment variables if REG_EXPAND_SZ
743+
system_path = expand_env_vars(raw_value) if reg_type == winreg.REG_EXPAND_SZ else raw_value
744+
except FileNotFoundError:
745+
# System PATH key doesn't exist (very unusual)
746+
pass
747+
except PermissionError:
748+
# May not have permission to read system PATH
749+
warning('Permission denied reading system PATH from registry')
750+
except OSError as e:
751+
warning(f'Failed to read system PATH from registry: {e}')
752+
753+
# Read user PATH from HKEY_CURRENT_USER
754+
try:
755+
with winreg.OpenKey(
756+
winreg.HKEY_CURRENT_USER,
757+
r'Environment',
758+
0,
759+
winreg.KEY_READ,
760+
) as key:
761+
raw_value, reg_type = winreg.QueryValueEx(key, 'Path')
762+
if raw_value:
763+
# Expand environment variables if REG_EXPAND_SZ
764+
user_path = expand_env_vars(raw_value) if reg_type == winreg.REG_EXPAND_SZ else raw_value
765+
except FileNotFoundError:
766+
# User PATH doesn't exist (possible on fresh systems)
767+
pass
768+
except PermissionError:
769+
warning('Permission denied reading user PATH from registry')
770+
except OSError as e:
771+
warning(f'Failed to read user PATH from registry: {e}')
772+
773+
# Combine paths: system PATH first, then user PATH
774+
# This matches Windows behavior where system PATH takes precedence
775+
if system_path and user_path:
776+
new_path = f'{system_path};{user_path}'
777+
elif system_path:
778+
new_path = system_path
779+
elif user_path:
780+
new_path = user_path
781+
else:
782+
# No PATH found in registry, keep current
783+
warning('No PATH found in registry, keeping current os.environ PATH')
784+
return False
785+
786+
# Update os.environ with the refreshed PATH
787+
old_path = os.environ.get('PATH', '')
788+
if new_path != old_path:
789+
os.environ['PATH'] = new_path
790+
info('Refreshed PATH from Windows registry')
791+
return True
792+
# PATH unchanged, no need to log
793+
return True
794+
795+
except Exception as e:
796+
warning(f'Failed to refresh PATH from registry: {e}')
797+
return False
798+
else:
799+
# Non-Windows platforms: no-op, return True
800+
return True
801+
802+
700803
def check_file_with_head(url: str, auth_headers: dict[str, str] | None = None) -> bool:
701804
"""Check if file exists using HEAD request.
702805
@@ -2107,61 +2210,69 @@ def install_dependencies(dependencies: dict[str, list[str]] | None) -> bool:
21072210
# Collect dependencies: platform-specific first, then common
21082211
# This ensures platform runtimes (e.g., Node.js) are installed before
21092212
# common tools that depend on them (e.g., npm packages)
2110-
deps_to_install: list[str] = []
2213+
platform_deps_list: list[str] = []
2214+
common_deps_list: list[str] = []
21112215

2112-
# Add platform-specific dependencies first (e.g., Node.js runtime)
2216+
# Collect platform-specific dependencies (e.g., Node.js runtime)
21132217
if current_platform_key:
21142218
platform_deps = dependencies.get(current_platform_key, [])
21152219
if platform_deps:
21162220
info(f'Found {len(platform_deps)} {current_platform_key}-specific dependencies')
2117-
deps_to_install.extend(platform_deps)
2221+
platform_deps_list = list(platform_deps)
21182222

2119-
# Add common dependencies second (e.g., npm packages that need Node.js)
2223+
# Collect common dependencies (e.g., npm packages that need Node.js)
21202224
common_deps = dependencies.get('common', [])
21212225
if common_deps:
21222226
info(f'Found {len(common_deps)} common dependencies')
2123-
deps_to_install.extend(common_deps)
2227+
common_deps_list = list(common_deps)
21242228

2125-
if not deps_to_install:
2229+
if not platform_deps_list and not common_deps_list:
21262230
info('No dependencies to install for this platform')
21272231
return True
21282232

2129-
# Execute all collected dependencies
2130-
for dep in deps_to_install:
2233+
# Helper function to execute a single dependency
2234+
def execute_dependency(dep: str) -> bool:
2235+
"""Execute a single dependency command. Returns True on success."""
21312236
info(f'Running: {dep}')
2132-
2133-
# Parse the command
21342237
parts = dep.split()
21352238

2136-
# Handle platform-specific commands
21372239
if system == 'Windows':
21382240
if parts[0] in ['winget', 'npm', 'pip', 'pipx']:
21392241
result = run_command(parts, capture_output=False)
2140-
elif parts[0] == 'uv' and parts[1] == 'tool' and parts[2] == 'install':
2141-
# For uv tool install, add --force to update if already installed
2242+
elif parts[0] == 'uv' and len(parts) >= 3 and parts[1] == 'tool' and parts[2] == 'install':
21422243
parts_with_force = parts[:3] + ['--force'] + parts[3:]
21432244
result = run_command(parts_with_force, capture_output=False)
21442245
else:
2145-
# Try PowerShell for other commands
21462246
result = run_command(['powershell', '-Command', dep], capture_output=False)
21472247
else:
2148-
# Unix-like systems
21492248
if parts[0] == 'uv' and len(parts) >= 3 and parts[1] == 'tool' and parts[2] == 'install':
2150-
# For uv tool install, add --force to update if already installed
21512249
dep_with_force = dep.replace('uv tool install', 'uv tool install --force')
21522250
result = run_command(['bash', '-c', dep_with_force], capture_output=False)
21532251
else:
2154-
# Execute command as-is (expand tilde paths before execution)
21552252
expanded_dep = expand_tildes_in_command(dep)
21562253
result = run_command(['bash', '-c', expanded_dep], capture_output=False)
21572254

21582255
if result.returncode != 0:
21592256
error(f'Failed to install dependency: {dep}')
2160-
# Check if it failed due to admin rights on Windows
21612257
if system == 'Windows' and not is_admin() and 'winget' in dep and '--scope machine' in dep:
21622258
warning('This may have failed due to lack of admin rights')
21632259
info('Try: 1) Run as administrator, or 2) Use --scope user instead')
21642260
warning('Continuing with other dependencies...')
2261+
return False
2262+
return True
2263+
2264+
# Phase 1: Execute platform-specific dependencies
2265+
for dep in platform_deps_list:
2266+
execute_dependency(dep)
2267+
2268+
# Phase 2: Refresh PATH from registry on Windows
2269+
# This picks up any PATH changes from platform-specific installations (e.g., Node.js)
2270+
if system == 'Windows' and platform_deps_list:
2271+
refresh_path_from_registry()
2272+
2273+
# Phase 3: Execute common dependencies
2274+
for dep in common_deps_list:
2275+
execute_dependency(dep)
21652276

21662277
return True
21672278

tests/test_setup_environment_additional.py

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2204,3 +2204,144 @@ def test_linux_continue_mode_message(self, _mock_system):
22042204

22052205
# Should have different echo message for continue mode
22062206
assert 'Resuming Claude Code session' in content or 'Continue mode' in content
2207+
2208+
2209+
class TestRefreshPathFromRegistry:
2210+
"""Test refresh_path_from_registry() function."""
2211+
2212+
@pytest.mark.skipif(sys.platform != 'win32', reason='Windows-specific test')
2213+
def test_refresh_path_from_registry_success(self):
2214+
"""Test successful PATH refresh from registry."""
2215+
# This test runs on actual Windows - verifies function doesn't crash
2216+
result = setup_environment.refresh_path_from_registry()
2217+
assert result is True
2218+
assert os.environ.get('PATH') # PATH should exist after refresh
2219+
2220+
def test_refresh_path_from_registry_non_windows(self):
2221+
"""Test function is no-op on non-Windows platforms."""
2222+
with patch('sys.platform', 'linux'):
2223+
result = setup_environment.refresh_path_from_registry()
2224+
assert result is True # Should return True (no-op success)
2225+
2226+
@pytest.mark.skipif(sys.platform != 'win32', reason='Windows-specific test')
2227+
def test_refresh_path_from_registry_system_path_only(self):
2228+
"""Test when only system PATH exists."""
2229+
import winreg
2230+
2231+
mock_key = MagicMock()
2232+
mock_key.__enter__ = MagicMock(return_value=mock_key)
2233+
mock_key.__exit__ = MagicMock(return_value=False)
2234+
2235+
# First call for system PATH succeeds, second for user PATH raises FileNotFoundError
2236+
call_count = [0]
2237+
2238+
def open_key_side_effect(*_args, **_kwargs): # noqa: ARG001
2239+
call_count[0] += 1
2240+
if call_count[0] == 1: # System PATH
2241+
return mock_key
2242+
# User PATH
2243+
raise FileNotFoundError
2244+
2245+
with (
2246+
patch('setup_environment.winreg.OpenKey', side_effect=open_key_side_effect),
2247+
patch('setup_environment.winreg.QueryValueEx') as mock_query,
2248+
):
2249+
mock_query.return_value = (r'C:\Windows\System32', winreg.REG_SZ)
2250+
2251+
result = setup_environment.refresh_path_from_registry()
2252+
assert result is True
2253+
assert r'C:\Windows\System32' in os.environ.get('PATH', '')
2254+
2255+
@pytest.mark.skipif(sys.platform != 'win32', reason='Windows-specific test')
2256+
def test_refresh_path_from_registry_user_path_only(self):
2257+
"""Test when only user PATH exists."""
2258+
import winreg
2259+
2260+
mock_key = MagicMock()
2261+
mock_key.__enter__ = MagicMock(return_value=mock_key)
2262+
mock_key.__exit__ = MagicMock(return_value=False)
2263+
2264+
# First call for system PATH raises, second for user PATH succeeds
2265+
call_count = [0]
2266+
2267+
def open_key_side_effect(*_args, **_kwargs): # noqa: ARG001
2268+
call_count[0] += 1
2269+
if call_count[0] == 1: # System PATH
2270+
raise FileNotFoundError
2271+
# User PATH
2272+
return mock_key
2273+
2274+
with (
2275+
patch('setup_environment.winreg.OpenKey', side_effect=open_key_side_effect),
2276+
patch('setup_environment.winreg.QueryValueEx') as mock_query,
2277+
):
2278+
mock_query.return_value = (r'C:\Users\Test\.local\bin', winreg.REG_SZ)
2279+
2280+
result = setup_environment.refresh_path_from_registry()
2281+
assert result is True
2282+
2283+
@pytest.mark.skipif(sys.platform != 'win32', reason='Windows-specific test')
2284+
def test_refresh_path_from_registry_combined_paths(self):
2285+
"""Test combining system and user PATH."""
2286+
import winreg
2287+
2288+
mock_key = MagicMock()
2289+
mock_key.__enter__ = MagicMock(return_value=mock_key)
2290+
mock_key.__exit__ = MagicMock(return_value=False)
2291+
2292+
with patch('setup_environment.winreg.OpenKey', return_value=mock_key):
2293+
# Return different values for system and user PATH
2294+
call_count = [0]
2295+
2296+
def query_side_effect(*_args, **_kwargs): # noqa: ARG001
2297+
call_count[0] += 1
2298+
if call_count[0] == 1: # System PATH
2299+
return (r'C:\Windows\System32', winreg.REG_SZ)
2300+
# User PATH
2301+
return (r'C:\Users\Test\.local\bin', winreg.REG_SZ)
2302+
2303+
with patch('setup_environment.winreg.QueryValueEx', side_effect=query_side_effect):
2304+
result = setup_environment.refresh_path_from_registry()
2305+
assert result is True
2306+
path = os.environ.get('PATH', '')
2307+
# System PATH should come first
2308+
assert path.startswith(r'C:\Windows\System32')
2309+
2310+
@pytest.mark.skipif(sys.platform != 'win32', reason='Windows-specific test')
2311+
def test_refresh_path_from_registry_expands_env_vars(self):
2312+
"""Test that REG_EXPAND_SZ values are expanded."""
2313+
import winreg
2314+
2315+
mock_key = MagicMock()
2316+
mock_key.__enter__ = MagicMock(return_value=mock_key)
2317+
mock_key.__exit__ = MagicMock(return_value=False)
2318+
2319+
with (
2320+
patch('setup_environment.winreg.OpenKey', return_value=mock_key),
2321+
patch('setup_environment.winreg.QueryValueEx') as mock_query,
2322+
patch.dict(os.environ, {'USERPROFILE': r'C:\Users\TestUser'}),
2323+
):
2324+
# Return a path with %USERPROFILE%
2325+
mock_query.return_value = (r'%USERPROFILE%\.local\bin', winreg.REG_EXPAND_SZ)
2326+
2327+
result = setup_environment.refresh_path_from_registry()
2328+
assert result is True
2329+
path = os.environ.get('PATH', '')
2330+
# %USERPROFILE% should be expanded
2331+
assert r'C:\Users\TestUser' in path
2332+
assert '%USERPROFILE%' not in path
2333+
2334+
@pytest.mark.skipif(sys.platform != 'win32', reason='Windows-specific test')
2335+
def test_refresh_path_from_registry_permission_error(self):
2336+
"""Test handling of permission errors."""
2337+
with patch('setup_environment.winreg.OpenKey', side_effect=PermissionError('Access denied')):
2338+
result = setup_environment.refresh_path_from_registry()
2339+
# Should return False but not crash
2340+
assert result is False
2341+
2342+
@pytest.mark.skipif(sys.platform != 'win32', reason='Windows-specific test')
2343+
def test_refresh_path_from_registry_no_path_found(self):
2344+
"""Test when no PATH found in registry."""
2345+
with patch('setup_environment.winreg.OpenKey', side_effect=FileNotFoundError()):
2346+
result = setup_environment.refresh_path_from_registry()
2347+
assert result is False # Should return False when no PATH found

0 commit comments

Comments
 (0)