From ac674d0050963ddca3aada3774a116b6f5f21065 Mon Sep 17 00:00:00 2001 From: Jason Robert Date: Sat, 4 Apr 2026 17:28:02 -0400 Subject: [PATCH] fix: Windows update entrypoint locking and idempotent releases - Use Path.replace() instead of Path.rename() for Windows exe rename, fixing failures when .exe.old exists from a previous interrupted update - Also target ~/.local/bin/conductor.exe explicitly (the standard uv entrypoint location) in addition to shutil.which() result - Handle partial success: when uv installs the package but fails to copy the entrypoint due to file locking, report success with a restart note instead of a scary error - Make release workflow idempotent: if the release already exists, upload artifacts to it instead of failing - Upload constraints.txt to existing v0.1.6 release Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/release.yml | 15 +++++-- src/conductor/cli/update.py | 84 ++++++++++++++++++++++++++++------- tests/test_cli/test_update.py | 35 +++++++++++++++ 3 files changed, 113 insertions(+), 21 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 2c9ca61..7476e1d 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -158,7 +158,14 @@ jobs: if [[ "${{ steps.version.outputs.prerelease }}" == "true" ]]; then PRERELEASE_FLAG="--prerelease" fi - gh release create "${{ github.ref_name }}" \ - dist/* \ - --generate-notes \ - $PRERELEASE_FLAG + # If the release already exists (e.g. created manually), upload artifacts + # to it instead of failing. + if gh release view "${{ github.ref_name }}" &>/dev/null; then + echo "Release ${{ github.ref_name }} already exists, uploading artifacts…" + gh release upload "${{ github.ref_name }}" dist/* --clobber + else + gh release create "${{ github.ref_name }}" \ + dist/* \ + --generate-notes \ + $PRERELEASE_FLAG + fi diff --git a/src/conductor/cli/update.py b/src/conductor/cli/update.py index 552439e..2f04e33 100644 --- a/src/conductor/cli/update.py +++ b/src/conductor/cli/update.py @@ -329,19 +329,9 @@ def run_update(console: Console) -> None: # On Windows, rename our exe out of the way so uv can write the new one. # Windows locks running executables but allows renaming them. - old_exe: Path | None = None + renamed_exes: list[tuple[Path, Path]] = [] if sys.platform == "win32": - exe_path = _get_conductor_exe() - if exe_path and exe_path.exists(): - old_exe = exe_path.with_suffix(".exe.old") - # Clean up leftover .old from a previous successful update - if old_exe.exists(): - with contextlib.suppress(OSError): - old_exe.unlink() - try: - exe_path.rename(old_exe) - except OSError: - old_exe = None # rename failed; proceed anyway, uv will report the error + renamed_exes = _rename_windows_exes() try: proc = subprocess.run(cmd, capture_output=True, text=True) # noqa: S603 @@ -350,16 +340,24 @@ def run_update(console: Console) -> None: console.print(f"[green]Successfully upgraded to v{version}[/green]") cache_path = get_cache_path() cache_path.unlink(missing_ok=True) + elif sys.platform == "win32" and "Failed to install entrypoint" in (proc.stderr or ""): + # On Windows, uv may fail to copy the entrypoint because the running + # executable is locked. The package itself was installed successfully. + console.print(f"[green]Successfully upgraded to v{version}[/green]") + console.print( + "[dim]Note: restart your terminal for the update to take full effect.[/dim]" + ) + cache_path = get_cache_path() + cache_path.unlink(missing_ok=True) else: console.print(f"[bold red]Upgrade failed[/bold red] (exit code {proc.returncode})") if proc.stderr: console.print(f"[dim]{proc.stderr.strip()}[/dim]") - # On Windows, restore the original exe if uv failed to write a new one - if old_exe and old_exe.exists(): - exe_path = old_exe.with_suffix("") # .exe.old → .exe - if not exe_path.exists(): + # On Windows, restore the original exe(s) if uv failed + for orig, backup in renamed_exes: + if backup.exists() and not orig.exists(): with contextlib.suppress(OSError): - old_exe.rename(exe_path) + backup.rename(orig) finally: # Clean up temp constraints file if constraints_path: @@ -368,6 +366,58 @@ def run_update(console: Console) -> None: constraints_path.parent.rmdir() +def _rename_windows_exes() -> list[tuple[Path, Path]]: + """Rename conductor executables on Windows so ``uv`` can overwrite them. + + Windows locks running executables, preventing overwrite. Renaming is + still allowed, so we move them out of the way before ``uv tool install``. + + We target both the executable found on ``PATH`` (the one currently running) + and the standard ``uv`` entrypoint location at ``~/.local/bin``, deduplicating + by resolved path. + + Returns: + A list of ``(original_path, backup_path)`` tuples for later restoration. + """ + renamed: list[tuple[Path, Path]] = [] + seen: set[str] = set() + candidates: list[Path] = [] + + # 1. The exe on PATH (the one currently running) + exe_from_which = _get_conductor_exe() + if exe_from_which: + candidates.append(exe_from_which) + + # 2. The standard uv entrypoint location + uv_bin_exe = Path.home() / ".local" / "bin" / "conductor.exe" + candidates.append(uv_bin_exe) + + for exe_path in candidates: + if not exe_path.exists(): + continue + + # Deduplicate by resolved path (case-insensitive on Windows) + try: + key = str(exe_path.resolve()).lower() + except OSError: + continue + if key in seen: + continue + seen.add(key) + + old_path = exe_path.with_suffix(".exe.old") + try: + # replace() overwrites an existing .old file, unlike rename() which + # fails on Windows when the destination already exists (e.g. from a + # previous interrupted update). + exe_path.replace(old_path) + renamed.append((exe_path, old_path)) + except OSError: + pass # rename failed; proceed, uv will report the error + + return renamed + + def _download_constraints(tag_name: str, console: Console) -> Path | None: """Download and verify the constraints file for a release. diff --git a/tests/test_cli/test_update.py b/tests/test_cli/test_update.py index ba36161..38d514c 100644 --- a/tests/test_cli/test_update.py +++ b/tests/test_cli/test_update.py @@ -633,6 +633,41 @@ def test_unix_skips_rename(self, cache_dir: Path) -> None: output = buf.getvalue() assert "Successfully upgraded" in output + def test_windows_entrypoint_failure_reports_success( + self, cache_dir: Path, tmp_path: Path + ) -> None: + """On Windows, if uv installs the package but fails to copy the entrypoint, + report success with a restart note instead of failure.""" + fake_exe = tmp_path / "conductor.exe" + fake_exe.write_text("fake") + + cache_file = cache_dir / "update-check.json" + cache_file.write_text("{}") + + c, buf = _make_console(is_terminal=True) + mock_proc = MagicMock() + mock_proc.returncode = 2 + mock_proc.stderr = "error: Failed to install entrypoint\n Caused by: failed to copy file" + + with ( + patch( + "conductor.cli.update.fetch_latest_version", + return_value=("99.0.0", "v99.0.0", "https://example.com"), + ), + patch("conductor.cli.update.sys.platform", "win32"), + patch("conductor.cli.update._get_conductor_exe", return_value=fake_exe), + patch("conductor.cli.update.subprocess.run", return_value=mock_proc), + ): + run_update(c) + + output = buf.getvalue() + assert "Successfully upgraded" in output + assert "restart your terminal" in output + assert "Upgrade failed" not in output + + # Cache should be cleared on partial success + assert not cache_file.exists() + # =================================================================== # E3-T3: CLI-level tests