Repository navigation
fix(hooks): try python before python3 under Git for Windows so the Store alias never runs (#4563) - #4568
fix(hooks): try python before python3 under Git for Windows so the Store alias never runs (#4563)#4568Deepjyoti-Sarmah wants to merge 2 commits into
Conversation
…Labs#4563) The last resort of the hook's interpreter probe tried `python3` before `python`. On Windows 10 and 11, `python3` on PATH is usually the Microsoft Store App Execution Alias, because the python.org installer ships python.exe but no python3.exe. Running it opens a console window for about a second and exits 9009, so every commit, checkout and merge flashed a window and waited before the rebuild could start, whenever the earlier probes missed. When WINDIR or MSYSTEM is set, the last resort now tries `python` first and `python3` second. Elsewhere the order is unchanged. The probe, the interpreter names, the final message and the exit status are the same, and the earlier probes are untouched.
|
Thanks for the pull request, @Deepjyoti-Sarmah. A maintainer will review it soon. Want to talk it through while it is in review? Come join us on our Discord server. For longer-form discussion there is also GitHub Discussions. A couple of things that speed up review: make sure the test suite passes on Python 3.10 and 3.13, and that the change keeps extraction deterministic. |
github-actions
Bot
added
the
contributor
![graphify-labs[bot]](https://avatars.githubusercontent.com/in/4356270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Graphify reviewed this change.
No issues found by static checks — no coupling regressions or blocking issues in the code graph. That is not the same as safe to merge: see what was not checked.
Not checked: tests were not run; no formal proof of the changed code.
Graphify review — findings
Fixes the hooks' last-resort Python probe to try python before python3 under Git for Windows (detected via WINDIR or MSYSTEM), so the Microsoft Store python3 alias no longer flashes a console window and exits 9009 on every commit, checkout and merge. Other platforms keep the python3, then python order, Windows still falls back to python3 when python lacks graphify, and with neither usable the hook warns and exits 0. Adds tests that run the emitted _PYTHON_DETECT under a real sh with fake interpreters on a bare PATH, using a copy of os.environ that drops only WINDIR, MSYSTEM and UV_TOOL_DIR, and checks that all four generated hooks embed the same chain.
No blocking issues surfaced.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 562 functions depend on the 303 functions this change touches.
Health — this change adds coupling hotspots:
- new:
_last_resort_run()— 4 callers, 3 callees
Verification — 562 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 338 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
32 of 432 test file(s) selected (7%) via static blast radius.
tests/test_affected_cli.py— impacttests/test_agents_platform.py— impacttests/test_codebuddy.py— impacttests/test_dedup_shrink_refuses_force_write.py— impacttests/test_devin.py— impacttests/test_explain_cli.py— impacttests/test_explain_same_file_tie.py— impacttests/test_extract_cli.py— impacttests/test_global_add_tag_inference.py— impacttests/test_god_nodes_cli.py— impacttests/test_hollow_chunks_arm_shrink_guard.py— impacttests/test_hook_chain_survives_skip.py— impacttests/test_hooks.py— impact, changed-testtests/test_hooks_rewrite.py— impacttests/test_incomplete_build_guard.py— impacttests/test_install.py— impacttests/test_install_references.py— impacttests/test_language_overrides.py— impacttests/test_lookup_exit_codes.py— impacttests/test_merge_chunks_validation.py— impacttests/test_multigraph_diagnostics.py— impacttests/test_no_dedup_flag.py— impacttests/test_opencode_plugin_refresh.py— impacttests/test_path_cli.py— impacttests/test_pi_guard_install.py— impacttests/test_query_cli.py— impacttests/test_query_induced_edges.py— impacttests/test_query_seeds.py— impacttests/test_skill_auto_refresh.py— impacttests/test_truncation_full_answer.py— impacttests/test_uninstall_purge_safety.py— impacttests/test_unverified_semantic_shrink.py— impact
Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.
Risky patterns (advisory)
- medium
no-stderr-to-stdoutingraphify/hooks.py:142: stderr redirected to stdout: if command -v "$_GFY_FIRST" >/dev/null 2>&1 && "$_GFY_FIRST" -c "$_GFY_PROBE" 2>/dev/null; then - medium
no-stderr-to-stdoutingraphify/hooks.py:144: stderr redirected to stdout: elif command -v "$_GFY_SECOND" >/dev/null 2>&1 && "$_GFY_SECOND" -c "$_GFY_PROBE" 2>/dev/null; then
· 1 grounded finding(s) anchored inline below.
|
|
||
| # ── #4563: the last-resort probe must not run the Microsoft Store python3 alias first on Windows ── | ||
|
|
||
| def _last_resort_run(tmp_path, env_extra=None, python3="stub", python="real"): |
There was a problem hiding this comment.
_last_resort_run()
high coupling complexity (Ca·Ce = 12).
Grounded coupling-delta finding (deterministic), not an LLM guess.
This branch has not been deployed
1 participant
What does this PR do?
Fixes #4563. When the hook's interpreter probes 1 to 4 all miss, its last resort tried
python3beforepython. On Windows 10/11,python3is usually the Microsoft Store App Execution Alias in%LOCALAPPDATA%\Microsoft\WindowsApps, because python.org's installer shipspython.exebut nopython3.exe. Running the alias opens a console window for about 1.1 s and exits 9009. The hook then falls through to the realpython, so it works, but every commit, checkout and merge flashes a window and blocks before the detached rebuild starts.The last resort is reached when the pinned interpreter,
.graphify_python, the launcher and the uv-tools scan all miss: a removed venv, graphify moved to another env, or a GUI client with a minimal PATH.Under Git for Windows the last resort now tries
pythonfirst. The Windows check is the onehooks.pyalready uses for its pipe-handle handling.Probes 1 to 4, the probe command, the interpreter names, the "could not locate a Python" message and
exit 0are unchanged. All four generated hooks splice in the same_PYTHON_DETECT, so one edit covers post-commit, post-checkout, post-merge and post-rewrite.I did not take the issue's second suggestion, skipping a
python3under*/WindowsApps/*. A real Store Python also lives there, so skipping it would remove the last resort for Store-Python users.Before (v8 probe block, a fake
WindowsApps/python3that logs, sleeps 1 s and exits 9009, ahead of a realpython):After (
MSYSTEM=MINGW64):Type of change
Verification & Invariants
Under Git for Windows, the hook never runs
python3whilepythoncan import graphify. Everywhere else the probe order is unchanged.Merge Danger
Door: two-way. A probe-order change inside a generated hook script. No schema, cache or file-format change. Existing installs pick it up on the next
graphify hook install.Blast Radius: narrow. Only the last-resort branch changes, and only when
WINDIRorMSYSTEMis set. Linux and macOS keeppython3thenpython. IfWINDIRorMSYSTEMleaks into a non-Windows shell (for example WSL withWSLENV), the only effect is thatpythonis tried beforepython3.Limitations
sh-script fakes and skip onnt. The reporter's measurements (about 1.1 s, exit 9009, oneAppInstallerPythonRedirector.exeper call) are the Windows evidence.graphify hook installis run again.pythonis itself a Store stub (no real Python on PATH), it still runs once before the warning.pythonlacks graphify,python3is still probed after it.How was this tested?
New tests in
tests/test_hooks.py. They run the emitted_PYTHON_DETECTunder a realsh, with a tempHOMEso probes 1 to 4 miss, and only two fake directories on PATH:WindowsApps/python3andbin/python. Each fake logs its call.test_windows_never_runs_the_store_python3_alias_when_python_works[msystem]python3, pythonpythontest_windows_never_runs_the_store_python3_alias_when_python_works[windir]test_windows_falls_back_to_python3_when_python_lacks_graphifypython3python, python3, pickspython3test_outside_windows_python3_is_still_tried_firsttest_windows_with_no_working_python_still_fails_loudly_and_exits_zerotest_every_hook_embeds_the_probe_chain_unchanged(4 hooks)The ones that pass on v8 guard existing behaviour: the Linux/macOS order, the warning path, and that every hook carries the tested block.
The same block also ran under
dash(debian:stable-slim) and BusyBox ash (alpine:3.20) with podman. Both parse it, and both give the same call order as above, with and withoutMSYSTEM.Graphify-specific checklist
uv run python -m tools.skillgen --bless) when changing their source fragments. (N/A: no skill fragments touched.)