fix(upgrade): extend rename+retry workaround to Windows#27271
Conversation
On Windows, `gh extension upgrade` fails with "Access is denied" when trying to overwrite the currently-running gh-aw.exe binary. This is the same root cause as the Linux ETXTBSY error, solved the same way: rename the running binary away before running the upgrade. Changes: - Add needsRenameWorkaround() helper (Linux or Windows) - Buffer first-attempt output on Windows (same as Linux) to suppress the confusing "Access is denied" message when retry succeeds - Attempt rename+retry on Windows, not just Linux - When rename+retry fails on Windows, print clear manual upgrade instructions: `gh extension upgrade gh-aw` or reinstall - Update renamePathForUpgrade log message to be platform-generic - Update tests: rename TestFirstAttemptWriter_NonLinux, add TestFirstAttemptWriter_Windows and TestNeedsRenameWorkaround Fixes #<windows-upgrade-issue> Agent-Logs-Url: https://github.com/github/gh-aw/sessions/f1110ed2-0ef6-41a7-9076-b76acf864fb7 Co-authored-by: dsyme <7204669+dsyme@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Extends the existing rename+retry self-upgrade workaround (previously Linux-only) to also cover Windows, and updates tests to reflect the new platform behavior.
Changes:
- Centralized platform gating into a new
needsRenameWorkaround()helper (Linux + Windows). - Buffered first-attempt upgrade output on Windows to suppress confusing “Access is denied” when retry succeeds.
- Added Windows-specific fallback guidance plus updated/added unit tests.
Show a summary per file
| File | Description |
|---|---|
pkg/cli/update_extension_check.go |
Applies rename+retry + buffered output behavior to Windows, adds needsRenameWorkaround(), and prints Windows fallback guidance on failure. |
pkg/cli/update_extension_check_test.go |
Splits non-Linux test into Windows vs non-Linux/non-Windows, and adds coverage for needsRenameWorkaround(). |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 3
| @@ -181,7 +204,7 @@ func renamePathForUpgrade(exe string) (string, error) { | |||
| if err := os.Rename(exe, backup); err != nil { | |||
| return "", fmt.Errorf("could not rename %s → %s: %w", exe, backup, err) | |||
| } | |||
| updateExtensionCheckLog.Printf("Renamed %s → %s to avoid ETXTBSY on Linux", exe, backup) | |||
| updateExtensionCheckLog.Printf("Renamed %s → %s to free path for upgrade", exe, backup) | |||
There was a problem hiding this comment.
On Windows, the renamed ".bak" file is the currently-running executable, so cleanupExecutableBackup(installPath) will typically fail (cannot delete/replace a running .exe). That leaves a persistent *.bak, and future calls to renamePathForUpgrade will fail because the destination already exists. Consider using a unique backup filename (e.g., include PID/timestamp) and/or deferring cleanup to the relaunched process (or a next-run cleanup) so repeated upgrades don't get stuck.
There was a problem hiding this comment.
Fixed in 332839a — renamePathForUpgrade now uses a PID-qualified backup path (exe.<pid>.bak). Each invocation gets a unique backup name, so a failed cleanup (e.g. Windows can't delete the running .exe) won't block a subsequent upgrade attempt. restoreExecutableBackup and cleanupExecutableBackup now take the backup path directly instead of deriving it from installPath.
| if runtime.GOOS == "windows" { | ||
| // On Windows, self-upgrade may not be possible while the binary is | ||
| // running. Guide the user to upgrade manually from a separate shell. | ||
| fmt.Fprintln(os.Stderr, console.FormatInfoMessage("On Windows, gh-aw cannot self-upgrade while it is running.")) | ||
| fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Please upgrade manually by running one of the following:")) | ||
| fmt.Fprintln(os.Stderr, console.FormatCommandMessage("gh extension upgrade gh-aw")) |
There was a problem hiding this comment.
The Windows fallback guidance is printed for any retry failure, even when the error isn't related to the running-binary lock (e.g., network/auth failures). This can mislead users. Consider emitting this guidance only when the first-attempt output (or retry error) matches the expected Windows lock symptom (e.g., contains "Access is denied" / sharing violation), and otherwise return the underlying upgrade error without the Windows-specific message.
There was a problem hiding this comment.
Fixed in 332839a — added isWindowsLockError which checks for "Access is denied" or "The process cannot access the file" in both the first-attempt output and the retry error. The Windows self-upgrade guidance is now gated behind this check, so unrelated failures (network, auth, etc.) just return the underlying error without the Windows-specific message.
🧪 Test Quality Sentinel ReportTest Quality Score: 90/100✅ Excellent test quality
Test Classification Details
Flagged Tests — Requires ReviewNone. All tests passed quality checks. Language SupportTests analyzed:
Verdict
📖 Understanding Test ClassificationsDesign Tests (High Value) verify what the system does:
Implementation Tests (Low Value) verify how the system does it:
Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators. References:
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
Hey This PR looks well-structured and ready for maintainer review. ✅ Contribution check result: 🟢 Aligned
Warning The following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "pypi.org"See Network Configuration for more information.
|
- Update upgradeExtensionIfOutdated doc comment: installPath is now captured on both Linux and Windows, not just Linux - Use PID-qualified backup path (exe.<pid>.bak) in renamePathForUpgrade so a failed cleanup (Windows can't delete running .exe) does not block a subsequent upgrade attempt - Thread backupPath through to restoreExecutableBackup/cleanupExecutableBackup instead of re-deriving it from installPath - Add isWindowsLockError helper; gate Windows self-upgrade guidance behind it so the message is only shown for lock-related errors, not network/auth - Update tests for new signatures; add TestIsWindowsLockError table test Agent-Logs-Url: https://github.com/github/gh-aw/sessions/b19bd17d-d6cc-4706-a5d7-e9bf2bec4f79 Co-authored-by: dsyme <7204669+dsyme@users.noreply.github.com>
|
@copilot update rendering of instructions so that they can be copy pasted. See extra ⚡ below ℹ On Windows, gh-aw cannot self-upgrade while it is running. |
…d text Replace FormatCommandMessage (which prepends ⚡) with plain fmt.Fprintln calls using two-space indentation, so the commands can be copy-pasted directly from the terminal output. Agent-Logs-Url: https://github.com/github/gh-aw/sessions/a1aa13a5-ade7-48fb-a03f-f7ce17767d62 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Fixed in |
|
New upgrade-test.yml workflow runs a matrix (ubuntu/macos/windows) that: - Finds the two most recent stable (non-prerelease) releases dynamically - Installs n-1 with gh extension install --pin - Runs `gh aw upgrade --no-fix` which triggers upgradeExtensionIfOutdated, exercising the rename+retry workaround on Windows - Verifies the installed version matches n Agent-Logs-Url: https://github.com/github/gh-aw/sessions/2cba2b36-2ef5-482f-bd58-834760f0ad7c Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Both items addressed in
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
| name: Extension Upgrade Test | ||
|
|
||
| on: | ||
| workflow_dispatch: {} |
| # --no-fix keeps the test focused: skips codemods, action updates, and compilation. | ||
| gh aw upgrade --no-fix | ||
|
|
||
| - name: Verify version after upgrade |
On Windows,
gh aw upgradefails withAccess is deniedwhengh extension upgradetries to overwrite the runninggh-aw.exe— the same root cause as Linux ETXTBSY, just a different OS error.Changes
needsRenameWorkaround()— new helper centralizing thelinux || windowsplatform check, replacing scatteredruntime.GOOS == "linux"guards*.<pid>.bak, freeing the path forghto write the new binary) now runs on Windows as well as Linux; PID-qualified backup names prevent collisions when cleanup fails (Windows cannot delete the running.exe)Access is deniedmessage when the retry succeeds, consistent with existing Linux behaviorAccess is denied/ sharing violation), prints copy-pasteable instructions using plain indented text (no⚡prefix):isWindowsLockError()— new helper that detects Windows binary-lock errors from the first-attempt output and retry error.github/workflows/upgrade-test.ymlruns a matrix (ubuntu/macos/windows) that dynamically finds the two most recent stable releases, installs n-1, runsgh aw upgrade --no-fix(which callsupgradeExtensionIfOutdated— the running binary tries to upgrade itself, exercising the rename+retry path on Windows), and verifies the installed version is now nTestFirstAttemptWriter_NonLinux→NonLinuxNonWindows, addedTestFirstAttemptWriter_Windows,TestNeedsRenameWorkaround, andTestIsWindowsLockError