From 61b61b83658420b203a0ed26a4abbeb14b919973 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 19 Mar 2026 13:02:08 +0000 Subject: [PATCH 1/4] Initial plan From d842870b5f2324bff4c368f65f91675f6ac9fd3c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 19 Mar 2026 13:16:23 +0000 Subject: [PATCH 2/4] fix: unlink binary before upgrade on Linux/WSL to avoid ETXTBSY (#issue) Co-authored-by: dsyme <7204669+dsyme@users.noreply.github.com> --- pkg/cli/update_extension_check.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/pkg/cli/update_extension_check.go b/pkg/cli/update_extension_check.go index 540c8104187..ef288d81ba4 100644 --- a/pkg/cli/update_extension_check.go +++ b/pkg/cli/update_extension_check.go @@ -4,6 +4,8 @@ import ( "fmt" "os" "os/exec" + "path/filepath" + "runtime" "strings" "golang.org/x/mod/semver" @@ -77,6 +79,25 @@ func upgradeExtensionIfOutdated(verbose bool) (bool, error) { updateExtensionCheckLog.Printf("Upgrading extension from %s to %s", currentVersion, latestVersion) fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Upgrading gh-aw extension from %s to %s...", currentVersion, latestVersion))) + // On Linux (including WSL), the kernel returns ETXTBSY when any process + // tries to open a currently-executing binary for writing. Removing the + // directory entry (unlink) first avoids this: the inode stays alive while + // this process is running, but the path is now free for gh to write the + // newly-downloaded binary. + if runtime.GOOS == "linux" { + if exe, err := os.Executable(); err == nil { + // Resolve any symlink so we remove the real file, not just a link. + if resolved, resolveErr := filepath.EvalSymlinks(exe); resolveErr == nil { + exe = resolved + } + if rmErr := os.Remove(exe); rmErr != nil { + updateExtensionCheckLog.Printf("Could not pre-remove executable before upgrade (upgrade may still fail): %v", rmErr) + } else { + updateExtensionCheckLog.Printf("Pre-removed executable to avoid ETXTBSY on Linux: %s", exe) + } + } + } + cmd := exec.Command("gh", "extension", "upgrade", "github/gh-aw") cmd.Stdout = os.Stderr cmd.Stderr = os.Stderr From 87759d204773f1baf8fa2c080c9c221f9c482b5a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 19 Mar 2026 17:14:52 +0000 Subject: [PATCH 3/4] fix: use rename+restore instead of remove to avoid bricking on upgrade failure Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .github/workflows/smoke-claude.lock.yml | 2 + pkg/cli/update_extension_check.go | 108 +++++++++++++++++++----- pkg/cli/update_extension_check_test.go | 99 +++++++++++++++++++++- pkg/cli/upgrade_command.go | 37 +++++--- 4 files changed, 210 insertions(+), 36 deletions(-) diff --git a/.github/workflows/smoke-claude.lock.yml b/.github/workflows/smoke-claude.lock.yml index a339fc73b79..bceba229dfd 100644 --- a/.github/workflows/smoke-claude.lock.yml +++ b/.github/workflows/smoke-claude.lock.yml @@ -2629,6 +2629,8 @@ jobs: /// // Auto-generated safe-output script handler: post-slack-message + const { sanitizeContent } = require("./sanitize_content.cjs"); + /** @type {import('./types/safe-output-script').SafeOutputScriptMain} */ async function main(config = {}) { const { channel, message } = config; diff --git a/pkg/cli/update_extension_check.go b/pkg/cli/update_extension_check.go index ef288d81ba4..5e58c91d36b 100644 --- a/pkg/cli/update_extension_check.go +++ b/pkg/cli/update_extension_check.go @@ -18,12 +18,22 @@ import ( var updateExtensionCheckLog = logger.New("cli:update_extension_check") // upgradeExtensionIfOutdated checks if a newer version of the gh-aw extension is available -// and, if so, upgrades it automatically. Returns true if an upgrade was performed. +// and, if so, upgrades it automatically. // -// When true is returned the CURRENTLY RUNNING PROCESS still has the old version baked in. -// The caller should re-launch the freshly-installed binary so that subsequent work -// (e.g. lock-file compilation) uses the correct new version string. -func upgradeExtensionIfOutdated(verbose bool) (bool, error) { +// Returns: +// - upgraded: true if an upgrade was performed. +// - installPath: on Linux, the resolved path where the new binary was installed +// (captured before any rename so the caller can relaunch the new binary from +// the correct path even after os.Executable() starts returning a "(deleted)" +// suffix). Empty string on non-Linux systems or when the path cannot be +// determined. +// - err: non-nil if the upgrade failed. +// +// When upgraded is true the CURRENTLY RUNNING PROCESS still has the old version +// baked in. The caller should re-launch the freshly-installed binary (at +// installPath) so that subsequent work (e.g. lock-file compilation) uses the +// correct new version string. +func upgradeExtensionIfOutdated(verbose bool) (bool, string, error) { currentVersion := GetVersion() updateExtensionCheckLog.Printf("Checking if extension needs upgrade (current: %s)", currentVersion) @@ -33,7 +43,7 @@ func upgradeExtensionIfOutdated(verbose bool) (bool, error) { if verbose { fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Skipping extension upgrade check (development build)")) } - return false, nil + return false, "", nil } // Query GitHub API for latest release @@ -44,12 +54,12 @@ func upgradeExtensionIfOutdated(verbose bool) (bool, error) { if verbose { fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Could not check for extension updates: %v", err))) } - return false, nil + return false, "", nil } if latestVersion == "" { updateExtensionCheckLog.Print("Could not determine latest version, skipping upgrade") - return false, nil + return false, "", nil } updateExtensionCheckLog.Printf("Latest version: %s", latestVersion) @@ -66,7 +76,7 @@ func upgradeExtensionIfOutdated(verbose bool) (bool, error) { if verbose { fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("✓ gh-aw extension is up to date")) } - return false, nil + return false, "", nil } } else { // Versions are not valid semver; skip unreliable string comparison and @@ -80,20 +90,27 @@ func upgradeExtensionIfOutdated(verbose bool) (bool, error) { fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Upgrading gh-aw extension from %s to %s...", currentVersion, latestVersion))) // On Linux (including WSL), the kernel returns ETXTBSY when any process - // tries to open a currently-executing binary for writing. Removing the - // directory entry (unlink) first avoids this: the inode stays alive while - // this process is running, but the path is now free for gh to write the - // newly-downloaded binary. + // tries to open a currently-executing binary for writing. We rename the + // executable to a temporary backup path before running gh extension upgrade, + // which atomically frees the original path so gh can write the new binary + // there. The inode stays alive for the running process. + // + // Using rename (rather than remove) means we can restore the backup if the + // upgrade command fails, preventing the user from being left without gh-aw. + // We also capture the install path now – after the rename os.Executable() + // returns a "(deleted)" suffix on Linux, so the caller must not rely on it + // for the subsequent relaunch. + var installPath string if runtime.GOOS == "linux" { - if exe, err := os.Executable(); err == nil { - // Resolve any symlink so we remove the real file, not just a link. + if exe, exeErr := os.Executable(); exeErr == nil { + // Resolve any symlink so we work on the real binary, not a gh wrapper. if resolved, resolveErr := filepath.EvalSymlinks(exe); resolveErr == nil { exe = resolved } - if rmErr := os.Remove(exe); rmErr != nil { - updateExtensionCheckLog.Printf("Could not pre-remove executable before upgrade (upgrade may still fail): %v", rmErr) + if path, renameErr := renamePathForUpgrade(exe); renameErr != nil { + updateExtensionCheckLog.Printf("Could not rename executable before upgrade (upgrade may fail with ETXTBSY): %v", renameErr) } else { - updateExtensionCheckLog.Printf("Pre-removed executable to avoid ETXTBSY on Linux: %s", exe) + installPath = path } } } @@ -101,10 +118,59 @@ func upgradeExtensionIfOutdated(verbose bool) (bool, error) { cmd := exec.Command("gh", "extension", "upgrade", "github/gh-aw") cmd.Stdout = os.Stderr cmd.Stderr = os.Stderr - if err := cmd.Run(); err != nil { - return false, fmt.Errorf("failed to upgrade gh-aw extension: %w", err) + if upgradeErr := cmd.Run(); upgradeErr != nil { + // The upgrade failed. If we renamed the binary to a backup, restore it + // now so the user is not left without a working gh-aw installation. + if installPath != "" { + restoreExecutableBackup(installPath) + } + return false, "", fmt.Errorf("failed to upgrade gh-aw extension: %w", upgradeErr) + } + + // Upgrade succeeded – clean up the backup file if one was created. + if installPath != "" { + cleanupExecutableBackup(installPath) } fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("✓ gh-aw extension upgraded to "+latestVersion)) - return true, nil + return true, installPath, nil +} + +// renamePathForUpgrade renames the binary at exe to exe+".bak", freeing the +// original path for the new binary to be written by gh extension upgrade. +// Returns exe (the install path) so the caller can relaunch the new binary and +// restore the backup if the upgrade fails. +func renamePathForUpgrade(exe string) (string, error) { + backup := exe + ".bak" + 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) + return exe, nil +} + +// restoreExecutableBackup renames the exe+".bak" backup back to exe. +// Called when the upgrade command failed and the new binary was not written. +func restoreExecutableBackup(installPath string) { + backup := installPath + ".bak" + if _, statErr := os.Stat(installPath); os.IsNotExist(statErr) { + // New binary was not installed; restore the backup. + if renErr := os.Rename(backup, installPath); renErr != nil { + updateExtensionCheckLog.Printf("could not restore backup %s → %s: %v", backup, installPath, renErr) + fmt.Fprintln(os.Stderr, console.FormatErrorMessage(fmt.Sprintf("Failed to restore gh-aw backup after upgrade failure. Manually rename %s to %s to recover.", backup, installPath))) + } else { + updateExtensionCheckLog.Printf("Restored backup %s → %s after failed upgrade", backup, installPath) + } + } else { + // New binary is present (upgrade partially succeeded); just clean up. + _ = os.Remove(backup) + } +} + +// cleanupExecutableBackup removes the exe+".bak" backup after a successful upgrade. +func cleanupExecutableBackup(installPath string) { + backup := installPath + ".bak" + if err := os.Remove(backup); err != nil && !os.IsNotExist(err) { + updateExtensionCheckLog.Printf("Could not remove backup %s: %v", backup, err) + } } diff --git a/pkg/cli/update_extension_check_test.go b/pkg/cli/update_extension_check_test.go index 6c5e192518c..10b060811a9 100644 --- a/pkg/cli/update_extension_check_test.go +++ b/pkg/cli/update_extension_check_test.go @@ -3,6 +3,8 @@ package cli import ( + "os" + "path/filepath" "testing" "github.com/stretchr/testify/assert" @@ -20,10 +22,11 @@ func TestUpgradeExtensionIfOutdated_DevBuild(t *testing.T) { // Verify the function exits before making any API calls. // If it did make API calls we'd see a network error in test environments, - // but the function must return (false, nil) immediately. - upgraded, err := upgradeExtensionIfOutdated(false) + // but the function must return (false, "", nil) immediately. + upgraded, installPath, err := upgradeExtensionIfOutdated(false) require.NoError(t, err, "Should not return error for dev builds") assert.False(t, upgraded, "Should not report upgrade for dev builds") + assert.Empty(t, installPath, "installPath should be empty for dev builds") } func TestUpgradeExtensionIfOutdated_SilentFailureOnAPIError(t *testing.T) { @@ -37,7 +40,97 @@ func TestUpgradeExtensionIfOutdated_SilentFailureOnAPIError(t *testing.T) { // Use a release version so the API call is attempted SetVersionInfo("v0.1.0") - upgraded, err := upgradeExtensionIfOutdated(false) + upgraded, installPath, err := upgradeExtensionIfOutdated(false) require.NoError(t, err, "Should fail silently on API errors") assert.False(t, upgraded, "Should not report upgrade when API is unreachable") + assert.Empty(t, installPath, "installPath should be empty when API is unreachable") +} + +func TestRenamePathForUpgrade(t *testing.T) { + // Create a temporary file to act as the "executable". + dir := t.TempDir() + exe := filepath.Join(dir, "gh-aw") + require.NoError(t, os.WriteFile(exe, []byte("binary"), 0o755), "Should create temp executable") + + installPath, err := renamePathForUpgrade(exe) + require.NoError(t, err, "renamePathForUpgrade should succeed") + assert.Equal(t, exe, installPath, "installPath should equal the original exe path") + + // The original path should no longer exist. + _, statErr := os.Stat(exe) + assert.True(t, os.IsNotExist(statErr), "Original executable should have been renamed away") + + // The backup should exist. + _, statErr = os.Stat(exe + ".bak") + assert.NoError(t, statErr, "Backup file should exist") +} + +func TestRenamePathForUpgrade_NonExistentFile(t *testing.T) { + dir := t.TempDir() + exe := filepath.Join(dir, "nonexistent") + + _, err := renamePathForUpgrade(exe) + assert.Error(t, err, "renamePathForUpgrade should fail for non-existent file") +} + +func TestRestoreExecutableBackup_NoNewBinary(t *testing.T) { + // Simulate: backup exists, new binary was NOT written (upgrade failed). + dir := t.TempDir() + installPath := filepath.Join(dir, "gh-aw") + backup := installPath + ".bak" + + require.NoError(t, os.WriteFile(backup, []byte("old binary"), 0o755), "Should create backup") + + restoreExecutableBackup(installPath) + + // Backup should be renamed back to installPath. + _, statErr := os.Stat(installPath) + require.NoError(t, statErr, "Original executable should be restored") + + // Backup file should be gone. + _, statErr = os.Stat(backup) + assert.True(t, os.IsNotExist(statErr), "Backup file should have been renamed away") +} + +func TestRestoreExecutableBackup_NewBinaryPresent(t *testing.T) { + // Simulate: both backup and new binary exist (upgrade partially succeeded). + dir := t.TempDir() + installPath := filepath.Join(dir, "gh-aw") + backup := installPath + ".bak" + + require.NoError(t, os.WriteFile(installPath, []byte("new binary"), 0o755), "Should create new binary") + require.NoError(t, os.WriteFile(backup, []byte("old binary"), 0o755), "Should create backup") + + restoreExecutableBackup(installPath) + + // New binary should still be present. + _, statErr := os.Stat(installPath) + require.NoError(t, statErr, "New binary should remain intact") + + // Backup should be cleaned up. + _, statErr = os.Stat(backup) + assert.True(t, os.IsNotExist(statErr), "Backup file should be cleaned up") +} + +func TestCleanupExecutableBackup(t *testing.T) { + dir := t.TempDir() + installPath := filepath.Join(dir, "gh-aw") + backup := installPath + ".bak" + + require.NoError(t, os.WriteFile(backup, []byte("old binary"), 0o755), "Should create backup") + + cleanupExecutableBackup(installPath) + + // Backup file should be removed. + _, statErr := os.Stat(backup) + assert.True(t, os.IsNotExist(statErr), "Backup file should be removed after cleanup") +} + +func TestCleanupExecutableBackup_NoBackup(t *testing.T) { + // Should not fail if backup doesn't exist. + dir := t.TempDir() + installPath := filepath.Join(dir, "gh-aw") + + // No panic or error expected. + cleanupExecutableBackup(installPath) } diff --git a/pkg/cli/upgrade_command.go b/pkg/cli/upgrade_command.go index 54b78e9bf68..e701fdf9a40 100644 --- a/pkg/cli/upgrade_command.go +++ b/pkg/cli/upgrade_command.go @@ -157,7 +157,7 @@ func runUpgradeCommand(verbose bool, workflowDir string, noFix bool, noCompile b // prevents the re-launched process from entering this branch again. if !skipExtensionUpgrade { fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Checking gh-aw extension version...")) - upgraded, err := upgradeExtensionIfOutdated(verbose) + upgraded, installPath, err := upgradeExtensionIfOutdated(verbose) if err != nil { upgradeLog.Printf("Extension upgrade failed: %v", err) return err @@ -165,7 +165,9 @@ func runUpgradeCommand(verbose bool, workflowDir string, noFix bool, noCompile b if upgraded { upgradeLog.Print("Extension was upgraded; re-launching with new binary") fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Continuing upgrade with newly installed version...")) - if err := relaunchWithSameArgs("--skip-extension-upgrade"); err != nil { + // Pass installPath so relaunchWithSameArgs uses the pre-rename path; + // on Linux os.Executable() returns a "(deleted)" suffix after the rename. + if err := relaunchWithSameArgs("--skip-extension-upgrade", installPath); err != nil { return err } // The child process completed all upgrade steps (including any PR creation). @@ -314,17 +316,28 @@ func updateAgentFiles(verbose bool) error { // process. The function blocks until the child exits and returns its error. // It is used after a successful extension upgrade so that the freshly-installed binary // (which carries the new version string) handles all subsequent work. -func relaunchWithSameArgs(extraFlag string) error { - exe, err := os.Executable() - if err != nil { - return fmt.Errorf("failed to determine executable path: %w", err) - } - - // Resolve symlinks to ensure we exec the real binary, not a wrapper. - if resolved, err := filepath.EvalSymlinks(exe); err == nil { - exe = resolved +// +// exeOverride, when non-empty, is used directly as the executable path instead of +// calling os.Executable(). On Linux the caller should pass the pre-rename install +// path because os.Executable() returns a "(deleted)"-suffixed path after the binary +// has been renamed out of the way during the upgrade. +func relaunchWithSameArgs(extraFlag string, exeOverride string) error { + var exe string + if exeOverride != "" { + exe = exeOverride } else { - upgradeLog.Printf("Failed to resolve symlink for executable %s (using as-is): %v", exe, err) + var err error + exe, err = os.Executable() + if err != nil { + return fmt.Errorf("failed to determine executable path: %w", err) + } + + // Resolve symlinks to ensure we exec the real binary, not a wrapper. + if resolved, err := filepath.EvalSymlinks(exe); err == nil { + exe = resolved + } else { + upgradeLog.Printf("Failed to resolve symlink for executable %s (using as-is): %v", exe, err) + } } // Explicitly copy os.Args[1:] so appending the extra flag does not modify From f37859ca91d8be28425b4e74ab97118c495936ff Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 19 Mar 2026 18:10:22 +0000 Subject: [PATCH 4/4] fix: try upgrade first, only rename binary on Linux if first attempt fails Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/update_extension_check.go | 94 ++++++++++++++++++-------- pkg/cli/update_extension_check_test.go | 24 +++++++ 2 files changed, 89 insertions(+), 29 deletions(-) diff --git a/pkg/cli/update_extension_check.go b/pkg/cli/update_extension_check.go index 5e58c91d36b..25e903631be 100644 --- a/pkg/cli/update_extension_check.go +++ b/pkg/cli/update_extension_check.go @@ -1,7 +1,9 @@ package cli import ( + "bytes" "fmt" + "io" "os" "os/exec" "path/filepath" @@ -89,45 +91,68 @@ func upgradeExtensionIfOutdated(verbose bool) (bool, string, error) { updateExtensionCheckLog.Printf("Upgrading extension from %s to %s", currentVersion, latestVersion) fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Upgrading gh-aw extension from %s to %s...", currentVersion, latestVersion))) - // On Linux (including WSL), the kernel returns ETXTBSY when any process - // tries to open a currently-executing binary for writing. We rename the - // executable to a temporary backup path before running gh extension upgrade, - // which atomically frees the original path so gh can write the new binary - // there. The inode stays alive for the running process. + // First attempt: run the upgrade without touching the filesystem. + // On most systems (and on Linux when there is no in-use binary conflict) + // this will succeed. On Linux with WSL the kernel may return ETXTBSY when + // gh tries to open the currently-executing binary for writing; in that case + // we fall through to the rename+retry path below. // - // Using rename (rather than remove) means we can restore the backup if the - // upgrade command fails, preventing the user from being left without gh-aw. - // We also capture the install path now – after the rename os.Executable() - // returns a "(deleted)" suffix on Linux, so the caller must not rely on it - // for the subsequent relaunch. + // On Linux we buffer the first attempt's output rather than printing it + // directly, so that the ETXTBSY error message is suppressed when the + // rename+retry path succeeds and the user is not shown a confusing failure. + var firstAttemptBuf bytes.Buffer + firstAttemptOut := firstAttemptWriter(os.Stderr, &firstAttemptBuf) + firstCmd := exec.Command("gh", "extension", "upgrade", "github/gh-aw") + firstCmd.Stdout = firstAttemptOut + firstCmd.Stderr = firstAttemptOut + firstErr := firstCmd.Run() + if firstErr == nil { + // First attempt succeeded without any file manipulation. + if runtime.GOOS == "linux" { + // Replay the buffered output that was not shown during the attempt. + _, _ = io.Copy(os.Stderr, &firstAttemptBuf) + } + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("✓ gh-aw extension upgraded to "+latestVersion)) + return true, "", nil + } + + // First attempt failed. + if runtime.GOOS != "linux" { + // On non-Linux systems there is nothing more to try. + return false, "", fmt.Errorf("failed to upgrade gh-aw extension: %w", firstErr) + } + + // On Linux the failure is likely ETXTBSY. Log the first attempt's output + // at debug level and attempt the rename+retry workaround. + updateExtensionCheckLog.Printf("First upgrade attempt failed (likely ETXTBSY); retrying with rename workaround. First attempt output: %s", firstAttemptBuf.String()) + + // Resolve the current executable path before renaming; after the rename + // os.Executable() returns a "(deleted)"-suffixed path on Linux. var installPath string - if runtime.GOOS == "linux" { - if exe, exeErr := os.Executable(); exeErr == nil { - // Resolve any symlink so we work on the real binary, not a gh wrapper. - if resolved, resolveErr := filepath.EvalSymlinks(exe); resolveErr == nil { - exe = resolved - } - if path, renameErr := renamePathForUpgrade(exe); renameErr != nil { - updateExtensionCheckLog.Printf("Could not rename executable before upgrade (upgrade may fail with ETXTBSY): %v", renameErr) - } else { - installPath = path - } + if exe, exeErr := os.Executable(); exeErr == nil { + if resolved, resolveErr := filepath.EvalSymlinks(exe); resolveErr == nil { + exe = resolved + } + if path, renameErr := renamePathForUpgrade(exe); renameErr != nil { + // Rename failed; the retry will likely fail again with ETXTBSY. + updateExtensionCheckLog.Printf("Could not rename executable for retry (upgrade will likely fail with ETXTBSY): %v", renameErr) + } else { + installPath = path } } - cmd := exec.Command("gh", "extension", "upgrade", "github/gh-aw") - cmd.Stdout = os.Stderr - cmd.Stderr = os.Stderr - if upgradeErr := cmd.Run(); upgradeErr != nil { - // The upgrade failed. If we renamed the binary to a backup, restore it - // now so the user is not left without a working gh-aw installation. + retryCmd := exec.Command("gh", "extension", "upgrade", "github/gh-aw") + retryCmd.Stdout = os.Stderr + retryCmd.Stderr = os.Stderr + if retryErr := retryCmd.Run(); retryErr != nil { + // Retry also failed. Restore the backup so the user still has gh-aw. if installPath != "" { restoreExecutableBackup(installPath) } - return false, "", fmt.Errorf("failed to upgrade gh-aw extension: %w", upgradeErr) + return false, "", fmt.Errorf("failed to upgrade gh-aw extension: %w", retryErr) } - // Upgrade succeeded – clean up the backup file if one was created. + // Retry succeeded. Clean up the backup. if installPath != "" { cleanupExecutableBackup(installPath) } @@ -136,6 +161,17 @@ func upgradeExtensionIfOutdated(verbose bool) (bool, string, error) { return true, installPath, nil } +// firstAttemptWriter returns a writer that buffers output on Linux (so that +// ETXTBSY error messages from a failed first upgrade attempt can be suppressed +// when the rename+retry workaround succeeds) and writes directly to dst on +// other platforms. +func firstAttemptWriter(dst io.Writer, buf *bytes.Buffer) io.Writer { + if runtime.GOOS == "linux" { + return buf + } + return dst +} + // renamePathForUpgrade renames the binary at exe to exe+".bak", freeing the // original path for the new binary to be written by gh extension upgrade. // Returns exe (the install path) so the caller can relaunch the new binary and diff --git a/pkg/cli/update_extension_check_test.go b/pkg/cli/update_extension_check_test.go index 10b060811a9..1f0ac7c4612 100644 --- a/pkg/cli/update_extension_check_test.go +++ b/pkg/cli/update_extension_check_test.go @@ -3,8 +3,10 @@ package cli import ( + "bytes" "os" "path/filepath" + "runtime" "testing" "github.com/stretchr/testify/assert" @@ -46,6 +48,28 @@ func TestUpgradeExtensionIfOutdated_SilentFailureOnAPIError(t *testing.T) { assert.Empty(t, installPath, "installPath should be empty when API is unreachable") } +func TestFirstAttemptWriter_Linux(t *testing.T) { + if runtime.GOOS != "linux" { + t.Skip("Linux-only behavior") + } + var buf bytes.Buffer + dst := &bytes.Buffer{} + w := firstAttemptWriter(dst, &buf) + // On Linux the writer should be the buffer, not dst. + assert.Equal(t, &buf, w, "firstAttemptWriter should return the buffer on Linux") +} + +func TestFirstAttemptWriter_NonLinux(t *testing.T) { + if runtime.GOOS == "linux" { + t.Skip("Non-Linux behavior only") + } + var buf bytes.Buffer + dst := &bytes.Buffer{} + w := firstAttemptWriter(dst, &buf) + // On non-Linux the writer should be dst. + assert.Equal(t, dst, w, "firstAttemptWriter should return dst on non-Linux") +} + func TestRenamePathForUpgrade(t *testing.T) { // Create a temporary file to act as the "executable". dir := t.TempDir()