fix: resolve venv executable paths on Windows#41
Open
zeukid wants to merge 1 commit into
Open
Conversation
Abhishek21g
approved these changes
Jul 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #40
Summary
venv_python_path()andvenv_pip_path()insia/layout.pyhardcoded thePOSIX venv layout (
bin/python,bin/pip). On Windows a venv places itsexecutables in
Scripts\python.exe/Scripts\pip.exe, so every subprocess SIAlaunches against the per-run venv pointed at a path that doesn't exist —
sia runcould not complete on Windows.
This branches on
os.nameto return the correct path per platform. POSIX outputis byte-identical to before, so all existing call sites in
run_setup.py(pip install, both the
uvand stdlib-venvbranches) andorchestrator.py(
_run_target_agent,run_evaluation) are fixed transitively. The Dockersandbox path is unaffected (it runs a Linux container).
The limitation was already noted in
docs/onboarding.md("Common Pitfalls").Changes
sia/layout.py— platform-awarevenv_python_path/venv_pip_pathvia ashared
_venv_bin_dirhelper (Scripts+.exeon Windows,binelsewhere).tests/test_layout.py(new) — parametrized overos.nameso both theWindows and POSIX layouts are asserted on any host, plus a guard that POSIX
output is unchanged.
.github/workflows/ci.yml— scopedwindows-latestjob runningtests/test_layout.pyto guard the fix on a real Windows runner.Testing
tests/test_layout.py: 5 passed.ruff checkandruff format --checkpass on the changed files.Notes
test_layout.py: a handful ofexisting tests carry pre-existing POSIX-only assumptions (
/dev/null,LF-pinned golden snapshots, file reads missing
encoding="utf-8") that fail onWindows independently of this change. Making the full suite Windows-clean is
a separate follow-up (test fixtures + a
.gitattributesfor line endings).