From 9c1993233b887a9f4b61dc544beefd5bbee678b1 Mon Sep 17 00:00:00 2001 From: clawmetry-autofix Date: Fri, 11 Sep 2026 15:39:17 +0000 Subject: [PATCH 1/3] fix(desktop): probe py -3.12 launcher as fallback in _known_good_python() (#5859) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When Python 3.12 is installed system-wide (not at the per-user LOCALAPPDATA path that _winget_install_python writes), the existing code returned None immediately — causing the interpreter retry to abort and bootstrap_python_version to stay at "3.11", which is exactly what repeated field-failure telemetry showed. The py launcher with an explicit version pin (`py -3.12`) finds Python 3.12 wherever it lives without risking the "newest interpreter" trap that bare `py` poses. Closes #5859 Co-Authored-By: ClawMetry Autofix Bot Co-Authored-By: Claude Sonnet 4.6 Claude-Session: https://claude.ai/code/session_01QngcJH4PkF4SgSdHs1UoV8 --- desktop/app.py | 64 +++++++++++++++------- tests/test_desktop_bootstrap_resilience.py | 51 +++++++++++++++++ 2 files changed, 94 insertions(+), 21 deletions(-) diff --git a/desktop/app.py b/desktop/app.py index f3294a71d6..a49dd191e2 100644 --- a/desktop/app.py +++ b/desktop/app.py @@ -828,31 +828,53 @@ def _bootstrap_python_platform(cache_file: Optional[Path]) -> str: def _known_good_python() -> Optional[str]: - """The pinned per-user python.org interpreter, if it is on disk and - passes the usual probe — the exact path `_winget_install_python` - installs to. - - Addressed by path, NOT by re-probing: `_bootstrap_python()` tries - `py` first, and the `py` launcher resolves to the NEWEST installed - interpreter, which in the case this exists to fix is precisely the - one that has no wheels. A re-probe would hand back the broken - interpreter and the retry would fail identically.""" + """The pinned python.org interpreter, if it is on disk and passes the + usual probe. + + Checks the exact per-user path `_winget_install_python` writes first. + If that is absent or fails the probe, also asks the py launcher for the + specific minor (``py -3.12``) — unlike bare ``py`` (which resolves to + the NEWEST installed interpreter, the one whose wheels are missing), the + version flag resolves only to Python 3.12 regardless of what else is + installed, covering system-wide installs that winget did not make.""" if platform.system() != "Windows": return None exe = (Path(os.environ.get("LOCALAPPDATA", "")) / "Programs" / "Python" / KNOWN_GOOD_PYTHON_DIRNAME / "python.exe") - if not exe.exists(): - return None - try: - r = subprocess.run( - [str(exe), "-c", _PYTHON_PROBE], - capture_output=True, text=True, timeout=20, - env=_child_env(), stdin=subprocess.DEVNULL, - **_win_subprocess_kwargs(), - ) - return str(exe) if r.returncode == 0 else None - except Exception: - return None + if exe.exists(): + try: + r = subprocess.run( + [str(exe), "-c", _PYTHON_PROBE], + capture_output=True, text=True, timeout=20, + env=_child_env(), stdin=subprocess.DEVNULL, + **_win_subprocess_kwargs(), + ) + if r.returncode == 0: + return str(exe) + except Exception: + pass + # Fallback: the py launcher with an explicit minor finds Python 3.12 + # wherever it lives (system-wide, another user path, etc.) when the + # per-user LOCALAPPDATA path above was absent or failed the probe. + # Bare 'py' is NOT used: it resolves to the newest interpreter (the + # one whose wheels are missing). + py_launcher = shutil.which("py") + if py_launcher: + try: + r = subprocess.run( + [py_launcher, f"-{KNOWN_GOOD_PYTHON_MINOR}", "-c", + "import sys, venv, ensurepip; print(sys.executable)"], + capture_output=True, text=True, timeout=20, + env=_child_env(), stdin=subprocess.DEVNULL, + **_win_subprocess_kwargs(), + ) + if r.returncode == 0: + path = r.stdout.strip() + if path and Path(path).exists(): + return path + except Exception: + pass + return None def _winget_install_python(log: Callable[[str], None]) -> None: diff --git a/tests/test_desktop_bootstrap_resilience.py b/tests/test_desktop_bootstrap_resilience.py index 50cdc5254a..4217a16658 100644 --- a/tests/test_desktop_bootstrap_resilience.py +++ b/tests/test_desktop_bootstrap_resilience.py @@ -645,6 +645,57 @@ def test_interpreter_retry_is_windows_only(tmp_path, monkeypatch): 1, _NO_WHEEL_FOR_INTERPRETER) == (1, _NO_WHEEL_FOR_INTERPRETER) +def test_known_good_python_falls_back_to_py_launcher_for_system_install( + tmp_path, monkeypatch): + """Python 3.12 installed system-wide (not at the per-user LOCALAPPDATA + path) is found via 'py -3.12' and returned. Covers machines where winget + installs to a non-standard location or the user ran the python.org + installer in system/all-users mode.""" + _windows(monkeypatch) + # Per-user path is absent: point LOCALAPPDATA at a dir with no Python3x. + monkeypatch.setenv("LOCALAPPDATA", "/no-such-localappdata") + # Suppress the Windows-only creationflags that don't exist on Linux. + monkeypatch.setattr(dapp, "_win_subprocess_kwargs", lambda: {}) + + # A real file for Path(path).exists() to find. + system_exe = str(tmp_path / "python.exe") + (tmp_path / "python.exe").write_text("") + + import subprocess as _sp + + def fake_run(cmd, **kwargs): + if (len(cmd) >= 3 + and cmd[1] == f"-{dapp.KNOWN_GOOD_PYTHON_MINOR}" + and "-c" in cmd): + return _sp.CompletedProcess(cmd, 0, system_exe + "\n", "") + return _sp.CompletedProcess(cmd, 1, "", "not found") + + monkeypatch.setattr(dapp.shutil, "which", + lambda name: "/usr/bin/py" if name == "py" else None) + monkeypatch.setattr(dapp.subprocess, "run", fake_run) + + result = dapp._known_good_python() + assert result == system_exe, ( + "when the LOCALAPPDATA path is absent, _known_good_python() must " + "probe 'py -3.12' to find a system-wide Python 3.12" + ) + + +def test_known_good_python_skips_py_launcher_when_absent(monkeypatch): + """If the py launcher is not on PATH, the fallback is skipped and + _known_good_python() returns None without spawning any subprocess.""" + _windows(monkeypatch) + monkeypatch.setenv("LOCALAPPDATA", "/no-such-localappdata") + monkeypatch.setattr(dapp, "_win_subprocess_kwargs", lambda: {}) + monkeypatch.setattr(dapp.shutil, "which", lambda name: None) + called = [] + monkeypatch.setattr(dapp.subprocess, "run", + lambda *a, **k: called.append(a) or None) + result = dapp._known_good_python() + assert result is None + assert not called, "subprocess.run must not be called when py is absent" + + def test_probe_caches_interpreter_version(tmp_path): cache = tmp_path / "bootstrap-python.json" py = dapp._bootstrap_python(cache) From ef968666cf9bf7a685fae7a0fa4ba3ebec3ad612 Mon Sep 17 00:00:00 2001 From: clawmetry-autofix Date: Fri, 11 Sep 2026 18:53:02 +0000 Subject: [PATCH 2/3] fix(desktop): use Windows Registry instead of py launcher in _known_good_python() Blueprint 'Desktop Application Distribution' requires _known_good_python() to work "by path, never by re-probing" and to avoid any py launcher usage. The previous fallback (py -3.12) violated this constraint as flagged by Drift Bot. Replace the py-launcher fallback with a Windows Registry lookup: python.org's installer writes the exact install path under SOFTWARE\Python\PythonCore\\InstallPath in both HKCU (per-user) and HKLM (system-wide), giving a direct filesystem path with no launcher or re-probing involved. Tests updated to mock winreg (injected into sys.modules) rather than shutil.which/py, and renamed to match the new mechanism: - test_known_good_python_falls_back_to_registry_for_system_install - test_known_good_python_returns_none_when_no_candidates Closes #5859 Co-Authored-By: ClawMetry Autofix Bot Co-Authored-By: Claude Sonnet 4.6 Claude-Session: https://claude.ai/code/session_01QngcJH4PkF4SgSdHs1UoV8 --- desktop/app.py | 69 ++++++++++++---------- tests/test_desktop_bootstrap_resilience.py | 69 ++++++++++++++-------- 2 files changed, 81 insertions(+), 57 deletions(-) diff --git a/desktop/app.py b/desktop/app.py index a49dd191e2..f44c3a67c1 100644 --- a/desktop/app.py +++ b/desktop/app.py @@ -831,47 +831,52 @@ def _known_good_python() -> Optional[str]: """The pinned python.org interpreter, if it is on disk and passes the usual probe. - Checks the exact per-user path `_winget_install_python` writes first. - If that is absent or fails the probe, also asks the py launcher for the - specific minor (``py -3.12``) — unlike bare ``py`` (which resolves to - the NEWEST installed interpreter, the one whose wheels are missing), the - version flag resolves only to Python 3.12 regardless of what else is - installed, covering system-wide installs that winget did not make.""" + Checks the exact per-user path ``_winget_install_python`` writes first, + then the Windows Registry (HKCU before HKLM) where the python.org + installer records the installation directory for every minor version it + manages. Both paths are exact filesystem paths — no py launcher, no + re-probing.""" if platform.system() != "Windows": return None - exe = (Path(os.environ.get("LOCALAPPDATA", "")) - / "Programs" / "Python" / KNOWN_GOOD_PYTHON_DIRNAME / "python.exe") - if exe.exists(): - try: - r = subprocess.run( - [str(exe), "-c", _PYTHON_PROBE], - capture_output=True, text=True, timeout=20, - env=_child_env(), stdin=subprocess.DEVNULL, - **_win_subprocess_kwargs(), - ) - if r.returncode == 0: - return str(exe) - except Exception: - pass - # Fallback: the py launcher with an explicit minor finds Python 3.12 - # wherever it lives (system-wide, another user path, etc.) when the - # per-user LOCALAPPDATA path above was absent or failed the probe. - # Bare 'py' is NOT used: it resolves to the newest interpreter (the - # one whose wheels are missing). - py_launcher = shutil.which("py") - if py_launcher: + candidates = [] + # Primary: the per-user path _winget_install_python writes. + per_user = (Path(os.environ.get("LOCALAPPDATA", "")) + / "Programs" / "Python" / KNOWN_GOOD_PYTHON_DIRNAME + / "python.exe") + if per_user.exists(): + candidates.append(str(per_user)) + # Secondary: the registered install path. The python.org installer + # writes SOFTWARE\Python\PythonCore\\InstallPath in HKCU + # (per-user install) and HKLM (system-wide install). Reading the + # registry directly gives the exact path without involving the py + # launcher or any re-probing. winreg is stdlib on Windows; the + # ImportError branch handles non-Windows test runners. + try: + import winreg # type: ignore[import] + reg_subkey = (rf"SOFTWARE\Python\PythonCore" + rf"\{KNOWN_GOOD_PYTHON_MINOR}\InstallPath") + for hive in (winreg.HKEY_CURRENT_USER, winreg.HKEY_LOCAL_MACHINE): + try: + with winreg.OpenKey(hive, reg_subkey) as k: + install_dir, _ = winreg.QueryValueEx(k, "") + exe = Path(install_dir) / "python.exe" + path = str(exe) + if path not in candidates and exe.exists(): + candidates.append(path) + except OSError: + pass + except ImportError: + pass + for candidate in candidates: try: r = subprocess.run( - [py_launcher, f"-{KNOWN_GOOD_PYTHON_MINOR}", "-c", - "import sys, venv, ensurepip; print(sys.executable)"], + [candidate, "-c", _PYTHON_PROBE], capture_output=True, text=True, timeout=20, env=_child_env(), stdin=subprocess.DEVNULL, **_win_subprocess_kwargs(), ) if r.returncode == 0: - path = r.stdout.strip() - if path and Path(path).exists(): - return path + return candidate except Exception: pass return None diff --git a/tests/test_desktop_bootstrap_resilience.py b/tests/test_desktop_bootstrap_resilience.py index 4217a16658..9ff1bca761 100644 --- a/tests/test_desktop_bootstrap_resilience.py +++ b/tests/test_desktop_bootstrap_resilience.py @@ -645,55 +645,74 @@ def test_interpreter_retry_is_windows_only(tmp_path, monkeypatch): 1, _NO_WHEEL_FOR_INTERPRETER) == (1, _NO_WHEEL_FOR_INTERPRETER) -def test_known_good_python_falls_back_to_py_launcher_for_system_install( +def test_known_good_python_falls_back_to_registry_for_system_install( tmp_path, monkeypatch): - """Python 3.12 installed system-wide (not at the per-user LOCALAPPDATA - path) is found via 'py -3.12' and returned. Covers machines where winget - installs to a non-standard location or the user ran the python.org - installer in system/all-users mode.""" + """Python 3.12 installed system-wide (registered in HKLM, not at the + per-user LOCALAPPDATA path) is found via the Windows Registry and + returned as an exact filesystem path — no py launcher involved.""" + import types _windows(monkeypatch) - # Per-user path is absent: point LOCALAPPDATA at a dir with no Python3x. monkeypatch.setenv("LOCALAPPDATA", "/no-such-localappdata") - # Suppress the Windows-only creationflags that don't exist on Linux. monkeypatch.setattr(dapp, "_win_subprocess_kwargs", lambda: {}) - # A real file for Path(path).exists() to find. - system_exe = str(tmp_path / "python.exe") - (tmp_path / "python.exe").write_text("") + system_exe = tmp_path / "python.exe" + system_exe.write_text("") + + # Inject a fake winreg that reports tmp_path as the HKLM install dir. + fake_winreg = types.ModuleType("winreg") + fake_winreg.HKEY_CURRENT_USER = 0x80000001 + fake_winreg.HKEY_LOCAL_MACHINE = 0x80000002 + + class _FakeKey: + def __enter__(self): return self + def __exit__(self, *a): return False + + def _fake_open(hive, path): + if hive == fake_winreg.HKEY_LOCAL_MACHINE: + return _FakeKey() + raise OSError("not found") + + def _fake_query(key, name): + # On Windows the registry stores the path with a trailing backslash; + # omit it here so Path(install_dir) / "python.exe" resolves correctly + # on the Linux CI runner (a backslash is not a separator on Linux). + return (str(tmp_path), 1) + + fake_winreg.OpenKey = _fake_open + fake_winreg.QueryValueEx = _fake_query + monkeypatch.setitem(sys.modules, "winreg", fake_winreg) import subprocess as _sp def fake_run(cmd, **kwargs): - if (len(cmd) >= 3 - and cmd[1] == f"-{dapp.KNOWN_GOOD_PYTHON_MINOR}" - and "-c" in cmd): - return _sp.CompletedProcess(cmd, 0, system_exe + "\n", "") - return _sp.CompletedProcess(cmd, 1, "", "not found") - - monkeypatch.setattr(dapp.shutil, "which", - lambda name: "/usr/bin/py" if name == "py" else None) + if cmd[0] == str(system_exe): + return _sp.CompletedProcess(cmd, 0, "", "") + return _sp.CompletedProcess(cmd, 1, "", "") + monkeypatch.setattr(dapp.subprocess, "run", fake_run) result = dapp._known_good_python() - assert result == system_exe, ( + assert result == str(system_exe), ( "when the LOCALAPPDATA path is absent, _known_good_python() must " - "probe 'py -3.12' to find a system-wide Python 3.12" + "find a system-wide Python 3.12 via the Windows Registry" ) -def test_known_good_python_skips_py_launcher_when_absent(monkeypatch): - """If the py launcher is not on PATH, the fallback is skipped and - _known_good_python() returns None without spawning any subprocess.""" +def test_known_good_python_returns_none_when_no_candidates(monkeypatch): + """When neither the per-user path nor any registry path provides a + usable Python 3.12, _known_good_python() returns None without spawning + any subprocess.""" _windows(monkeypatch) monkeypatch.setenv("LOCALAPPDATA", "/no-such-localappdata") monkeypatch.setattr(dapp, "_win_subprocess_kwargs", lambda: {}) - monkeypatch.setattr(dapp.shutil, "which", lambda name: None) + # On Linux winreg is absent (ImportError caught inside the function), + # so no registry candidates are added. subprocess.run must not be called. called = [] monkeypatch.setattr(dapp.subprocess, "run", lambda *a, **k: called.append(a) or None) result = dapp._known_good_python() assert result is None - assert not called, "subprocess.run must not be called when py is absent" + assert not called, "subprocess.run must not be called when no candidates exist" def test_probe_caches_interpreter_version(tmp_path): From 9c16f4be7b02910b8a8e0d0028f9bddfd8af5176 Mon Sep 17 00:00:00 2001 From: clawmetry-autofix Date: Fri, 11 Sep 2026 18:57:44 +0000 Subject: [PATCH 3/3] chore: tighten CI test-file coverage ratchet after #5859 tests renamed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two test functions in test_desktop_bootstrap_resilience.py were renamed (py-launcher variants → registry variants) in the previous commit. The ratchet script detected the improvement (unlisted: 930 vs 931) and requires the baseline to be updated. Run: python3 scripts/check_ci_test_coverage.py --update-baseline Co-Authored-By: Claude Sonnet 4.6 Claude-Session: https://claude.ai/code/session_01QngcJH4PkF4SgSdHs1UoV8 --- docs/ci_test_coverage_baseline.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/ci_test_coverage_baseline.json b/docs/ci_test_coverage_baseline.json index 7f39c4c56f..522e27abc2 100644 --- a/docs/ci_test_coverage_baseline.json +++ b/docs/ci_test_coverage_baseline.json @@ -7,7 +7,7 @@ "Ratchet down by running --update-baseline after wiring new tests in.", "Related: issue #5813" ], - "total": 1138, - "listed": 206, - "unlisted_max": 932 + "total": 1142, + "listed": 211, + "unlisted_max": 931 }