fix: amplifier reset can't self-modify the running tool env on Windows - #274
Closed
Brian Krabach (bkrabach) wants to merge 1 commit into
Closed
fix: amplifier reset can't self-modify the running tool env on Windows#274Brian Krabach (bkrabach) wants to merge 1 commit into
Brian Krabach (bkrabach) wants to merge 1 commit into
Conversation
`amplifier reset` uninstalls and reinstalls amplifier IN-PROCESS
(reset.py: _uninstall_amplifier -> `uv tool uninstall amplifier`, and
_install_amplifier -> `uv tool install`). On native Windows the OS locks a
running program's own files: while amplifier.exe is alive, its loaded
python3xx.dll / .pyd under %APPDATA%\uv\tools\amplifier\Lib\ cannot be
deleted, so uv fails with exit status 2 / "Access is denied (os error 5)".
POSIX unlinks open files, so the same code works there -- this never showed
up in Mac/WSL testing. Reported by Salil on a fresh native-Windows install.
Fix (Windows-only, os.name == "nt", not dry_run): do the safe in-process
cleanup now (uv cache clean + ~/.amplifier data removal -- neither touches the
tool env), then hand the tool-env swap to a generated throw-away .cmd that:
- waits for THIS process id to exit (releasing the lock),
- retries `uv tool uninstall`/`install` with backoff (a just-exited process
releases handles lazily; another open Amplifier window, an antivirus scan,
or the file indexer can keep files locked briefly), and
- fails loud with a re-runnable script path + guidance if still locked --
never a silent half-swapped install.
The script is auto-launched in a new console; its path and the equivalent
manual commands are always printed as a fallback.
POSIX path is byte-identical: the only non-guarded addition is `import os`
(inert); the new _windows_defer_tool_swap() is only called behind the guard.
Proven: `amplifier reset --dry-run` output on Linux is byte-for-byte identical
to main (git stash A/B).
Evidence (native Windows, alienware-r13):
- BASELINE: `amplifier reset -y --no-install` -> "Failed to uninstall
amplifier: ... 'uv','tool','uninstall','amplifier' returned non-zero exit
status 2" + "[WinError 5] Access is denied" (Salil's exact error).
- MECHANISM: a running process holds its own file -> "Access is denied" to
delete it; the instant the process exits, the delete succeeds
(LOCK_RELEASED) -- exactly why deferring past process exit works.
Reported-by: Salil Das
🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)
Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Collaborator
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.
Summary
amplifier resetuninstalls and reinstalls amplifier in-process (reset.py:_uninstall_amplifier->uv tool uninstall amplifier, and_install_amplifier->uv tool install). On native Windows the OS locks a running program's own files: whileamplifier.exeis alive, its loadedpython3xx.dll/.pydunder%APPDATA%\uv\tools\amplifier\Lib\cannot be deleted, so uv fails with exit status 2 / "Access is denied (os error 5)". POSIX unlinks open files, so the same code works there — this never showed up in Mac/WSL testing.Reported by Salil Das (@sadlilas) on a fresh native-Windows install.
The fix (Windows-only,
os.name == "nt"and not--dry-run)Do the safe cleanup in-process now —
uv cache clean+~/.amplifierdata removal (neither touches the tool env) — then hand the tool-env swap to a generated throw-away.cmdthat:uv tool uninstall/installwith backoff — a just-exited process releases handles lazily, and another open Amplifier window, an antivirus scan, or the file indexer can keep files locked briefly, andThe script is auto-launched in a new console; its path and the equivalent manual commands are always printed as a fallback.
POSIX is byte-identical
The only non-guarded addition is
import os(inert); the new_windows_defer_tool_swap()is only called behind the guard. Proven:amplifier reset --dry-runoutput on Linux is byte-for-byte identical tomain(git stashA/B), so POSIX behavior cannot regress.Evidence (native Windows, real box)
BASELINE —
amplifier reset -y --no-install:(the reporter's exact error, reproduced.)
MECHANISM — a running process holds its own file open; deleting it returns Access is denied while the process lives, and the delete succeeds the instant the process exits (
LOCK_RELEASED). This is exactly why deferring the tool swap past process exit works:Honest limits
amplifier reset, reporter's exact error) and the lock-release-after-exit mechanism (decisive before/after). The POSIX byte-identical claim is proven bygit stashA/B on Linux.amplifier resetcompleting the deferred uninstall+reinstall on a clean Windows box. The test box has other live Amplifier processes (concurrent sessions) holding the tool env, which I must not kill — so the deferred script's retries can't drain there. A reviewer with a clean Windows box can confirm the full round-trip in ~2 minutes:amplifier reset -y, let the new console window finish, thenamplifier --version.