Skip to content

Fix: Windows: os.replace onto in-use cached binary raises PermissionError - #118

Open
pullapprove5[bot] wants to merge 1 commit into
masterfrom
pullapprove/pa-6-7e84de
Open

Fix: Windows: os.replace onto in-use cached binary raises PermissionError#118
pullapprove5[bot] wants to merge 1 commit into
masterfrom
pullapprove/pa-6-7e84de

Conversation

@pullapprove5

@pullapprove5 pullapprove5 Bot commented Sep 4, 2026

Copy link
Copy Markdown

The finding is real: on Windows, os.replace(tmp_path, binary_path) in both plain-tailwind/plain/tailwind/core.py:173 and plain-code/plain/code/oxc.py:195 raises PermissionError (sharing violation) when a second checkout tries to install/download the same version binary while a first checkout is actively executing it from the shared machine-wide ~/.cache/plain cache. POSIX is unaffected because os.replace there succeeds even onto an open/executing file. I confirmed the exact behavior by exercising the real Tailwind.download() and OxcTool.download() code paths end-to-end (mocked network I/O, real os.replace monkeypatched to raise the actual Windows sharing-violation errno/message) with the destination binary pre-populated to simulate "another process already installed and is running it": on the pre-fix code this raised PermissionError; after the fix it succeeded and left the pre-existing binary in place. I fixed both call sites identically: wrap os.replace in try/except PermissionError, and only re-raise if the destination binary_path doesn't already exist (i.e. genuinely still missing, not just contended) — matching the deferred finding's suggested approach. Ran ./scripts/fix on both plain-tailwind and plain-code (after installing uv via pip since it wasn't preinstalled in this sandbox) — both passed clean with zero diff (ruff, oxlint/oxfmt, prettier all no-op). Could not run ./scripts/test: it requires a Postgres server via scripts/start-postgres, and docker isn't available in this sandbox; additionally neither plain-tailwind nor plain-code has a tests/ directory or appears in scripts/test's ALL_PACKAGES list, so the repo's own suite never covered this code before or after the change. One commit was made covering both fixes.


Opened by a PullApprove implementation run (implement-finding v4) for:

  • PA-6 — Windows: os.replace onto in-use cached binary raises PermissionError

Merging this is what closes them as fixed.

…rror

The finding is real: on Windows, os.replace(tmp_path, binary_path) in both plain-tailwind/plain/tailwind/core.py:173 and plain-code/plain/code/oxc.py:195 raises PermissionError (sharing violation) when a second checkout tries to install/download the same version binary while a first checkout is actively executing it from the shared machine-wide ~/.cache/plain cache. POSIX is unaffected because os.replace there succeeds even onto an open/executing file. I confirmed the exact behavior by exercising the real Tailwind.download() and OxcTool.download() code paths end-to-end (mocked network I/O, real os.replace monkeypatched to raise the actual Windows sharing-violation errno/message) with the destination binary pre-populated to simulate "another process already installed and is running it": on the pre-fix code this raised PermissionError; after the fix it succeeded and left the pre-existing binary in place. I fixed both call sites identically: wrap os.replace in try/except PermissionError, and only re-raise if the destination binary_path doesn't already exist (i.e. genuinely still missing, not just contended) — matching the deferred finding's suggested approach. Ran ./scripts/fix on both plain-tailwind and plain-code (after installing uv via pip since it wasn't preinstalled in this sandbox) — both passed clean with zero diff (ruff, oxlint/oxfmt, prettier all no-op). Could not run ./scripts/test: it requires a Postgres server via scripts/start-postgres, and docker isn't available in this sandbox; additionally neither plain-tailwind nor plain-code has a tests/ directory or appears in scripts/test's ALL_PACKAGES list, so the repo's own suite never covered this code before or after the change. One commit was made covering both fixes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants