diff --git a/pkg/cli/update_extension_check.go b/pkg/cli/update_extension_check.go index 67ddc1cc1a0..540c8104187 100644 --- a/pkg/cli/update_extension_check.go +++ b/pkg/cli/update_extension_check.go @@ -3,8 +3,11 @@ package cli import ( "fmt" "os" + "os/exec" "strings" + "golang.org/x/mod/semver" + "github.com/github/gh-aw/pkg/console" "github.com/github/gh-aw/pkg/logger" "github.com/github/gh-aw/pkg/workflow" @@ -12,73 +15,75 @@ import ( var updateExtensionCheckLog = logger.New("cli:update_extension_check") -// ensureLatestExtensionVersion checks if the current release matches the latest release -// and issues a warning if an update is needed. This function fails silently if the -// release URL is not available or blocked. -func ensureLatestExtensionVersion(verbose bool) error { - if verbose { - fmt.Fprintln(os.Stderr, console.FormatVerboseMessage("Checking for gh-aw extension updates...")) - } - - // Get current version +// 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. +// +// 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) { currentVersion := GetVersion() - updateExtensionCheckLog.Printf("Current version: %s", currentVersion) + updateExtensionCheckLog.Printf("Checking if extension needs upgrade (current: %s)", currentVersion) - // Skip check for non-release versions (dev builds) + // Skip for non-release versions (dev builds) if !workflow.IsReleasedVersion(currentVersion) { - updateExtensionCheckLog.Print("Not a released version, skipping update check") + updateExtensionCheckLog.Print("Not a released version, skipping upgrade check") if verbose { - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Skipping version check (development build)")) + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Skipping extension upgrade check (development build)")) } - return nil + return false, nil } // Query GitHub API for latest release latestVersion, err := getLatestRelease() if err != nil { - // Fail silently - don't block upgrade if we can't check for updates - updateExtensionCheckLog.Printf("Failed to check for updates (silently ignoring): %v", err) + // Fail silently - don't block the upgrade command if we can't reach GitHub + updateExtensionCheckLog.Printf("Failed to check for latest release (silently ignoring): %v", err) if verbose { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Could not check for updates: %v", err))) + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Could not check for extension updates: %v", err))) } - return nil + return false, nil } if latestVersion == "" { - updateExtensionCheckLog.Print("Could not determine latest version") - return nil + updateExtensionCheckLog.Print("Could not determine latest version, skipping upgrade") + return false, nil } updateExtensionCheckLog.Printf("Latest version: %s", latestVersion) - // Normalize versions for comparison (remove 'v' prefix) - currentVersionNormalized := strings.TrimPrefix(currentVersion, "v") - latestVersionNormalized := strings.TrimPrefix(latestVersion, "v") + // Ensure both versions have the 'v' prefix required by the semver package. + currentSV := "v" + strings.TrimPrefix(currentVersion, "v") + latestSV := "v" + strings.TrimPrefix(latestVersion, "v") - // Compare versions - if currentVersionNormalized == latestVersionNormalized { - if verbose { - fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("✓ gh-aw extension is up to date")) + // Already on the latest (or newer) version – use proper semver comparison so + // that e.g. "0.10.0" is correctly treated as newer than "0.9.0". + if semver.IsValid(currentSV) && semver.IsValid(latestSV) { + if semver.Compare(currentSV, latestSV) >= 0 { + updateExtensionCheckLog.Print("Extension is already up to date") + if verbose { + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("✓ gh-aw extension is up to date")) + } + return false, nil } - updateExtensionCheckLog.Print("Extension is up to date") - return nil + } else { + // Versions are not valid semver; skip unreliable string comparison and + // proceed with the upgrade to avoid incorrectly treating an outdated + // version as up to date (lexicographic comparison breaks for e.g. "0.9.0" vs "0.10.0"). + updateExtensionCheckLog.Printf("Non-semver versions detected (current=%q, latest=%q); proceeding with upgrade", currentVersion, latestVersion) } - // Check if we're on a newer version (development/prerelease) - if currentVersionNormalized > latestVersionNormalized { - updateExtensionCheckLog.Printf("Current version (%s) appears newer than latest release (%s)", currentVersion, latestVersion) - if verbose { - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Running a development or pre-release version")) - } - return nil - } + // A newer version is available – upgrade automatically + 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))) - // A newer version is available - display warning message (not error) - updateExtensionCheckLog.Printf("Newer version available: %s (current: %s)", latestVersion, currentVersion) - fmt.Fprintln(os.Stderr, "") - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("A newer version of gh-aw is available: %s (current: %s)", latestVersion, currentVersion))) - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Consider upgrading with: gh extension upgrade github/gh-aw")) - fmt.Fprintln(os.Stderr, "") + 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) + } - return nil + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("✓ gh-aw extension upgraded to "+latestVersion)) + return true, nil } diff --git a/pkg/cli/update_extension_check_test.go b/pkg/cli/update_extension_check_test.go index 951faa95c52..6c5e192518c 100644 --- a/pkg/cli/update_extension_check_test.go +++ b/pkg/cli/update_extension_check_test.go @@ -5,34 +5,39 @@ package cli import ( "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -func TestEnsureLatestExtensionVersion_DevBuild(t *testing.T) { +func TestUpgradeExtensionIfOutdated_DevBuild(t *testing.T) { // Save original version and restore after test originalVersion := GetVersion() defer SetVersionInfo(originalVersion) - // Set a dev version + // Set a dev version – upgrade check must be skipped for dev builds because + // workflow.IsReleasedVersion returns false for non-release builds. SetVersionInfo("dev") - // Should return nil without error for dev builds - err := ensureLatestExtensionVersion(false) + // 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) require.NoError(t, err, "Should not return error for dev builds") + assert.False(t, upgraded, "Should not report upgrade for dev builds") } -func TestEnsureLatestExtensionVersion_SilentFailure(t *testing.T) { - // This test verifies that network/API errors are handled silently - // The actual API call will fail in the test environment but should not return an error +func TestUpgradeExtensionIfOutdated_SilentFailureOnAPIError(t *testing.T) { + // When the GitHub API is unreachable the function must fail silently and + // must NOT report an upgrade so that the rest of the upgrade command + // continues unaffected. - // Save original version and restore after test originalVersion := GetVersion() defer SetVersionInfo(originalVersion) - // Set a valid release version + // Use a release version so the API call is attempted SetVersionInfo("v0.1.0") - // Should return nil even if API call fails (fails silently) - err := ensureLatestExtensionVersion(false) + upgraded, 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") } diff --git a/pkg/cli/upgrade_command.go b/pkg/cli/upgrade_command.go index 0da8be32079..54b78e9bf68 100644 --- a/pkg/cli/upgrade_command.go +++ b/pkg/cli/upgrade_command.go @@ -1,8 +1,11 @@ package cli import ( + "errors" "fmt" "os" + "os/exec" + "path/filepath" "github.com/github/gh-aw/pkg/console" "github.com/github/gh-aw/pkg/constants" @@ -77,6 +80,7 @@ Examples: noCompile, _ := cmd.Flags().GetBool("no-compile") auditFlag, _ := cmd.Flags().GetBool("audit") jsonOutput, _ := cmd.Flags().GetBool("json") + skipExtensionUpgrade, _ := cmd.Flags().GetBool("skip-extension-upgrade") // Handle audit mode if auditFlag { @@ -89,7 +93,7 @@ Examples: } } - if err := runUpgradeCommand(verbose, dir, noFix, noCompile, noActions); err != nil { + if err := runUpgradeCommand(verbose, dir, noFix, noCompile, noActions, skipExtensionUpgrade); err != nil { return err } @@ -112,6 +116,8 @@ Examples: cmd.Flags().Bool("pr", false, "Alias for --create-pull-request") _ = cmd.Flags().MarkHidden("pr") // Hide the short alias from help output cmd.Flags().Bool("audit", false, "Check dependency health without performing upgrades") + cmd.Flags().Bool("skip-extension-upgrade", false, "Skip automatic extension upgrade (used internally to prevent recursion after upgrade)") + _ = cmd.Flags().MarkHidden("skip-extension-upgrade") addJSONFlag(cmd) // Register completions @@ -140,15 +146,32 @@ func runDependencyAudit(verbose bool, jsonOutput bool) error { } // runUpgradeCommand executes the upgrade process -func runUpgradeCommand(verbose bool, workflowDir string, noFix bool, noCompile bool, noActions bool) error { - upgradeLog.Printf("Running upgrade command: verbose=%v, workflowDir=%s, noFix=%v, noCompile=%v, noActions=%v", - verbose, workflowDir, noFix, noCompile, noActions) - - // Step 0b: Ensure gh-aw extension is on the latest version - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Checking gh-aw extension version...")) - if err := ensureLatestExtensionVersion(verbose); err != nil { - upgradeLog.Printf("Extension version check failed: %v", err) - return err +func runUpgradeCommand(verbose bool, workflowDir string, noFix bool, noCompile bool, noActions bool, skipExtensionUpgrade bool) error { + upgradeLog.Printf("Running upgrade command: verbose=%v, workflowDir=%s, noFix=%v, noCompile=%v, noActions=%v, skipExtensionUpgrade=%v", + verbose, workflowDir, noFix, noCompile, noActions, skipExtensionUpgrade) + + // Step 0b: Ensure gh-aw extension is on the latest version. + // If the extension was just upgraded, re-launch the freshly-installed binary + // with the same flags so that all subsequent steps (e.g. lock-file compilation) + // use the correct new version string. The hidden --skip-extension-upgrade flag + // 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) + if err != nil { + upgradeLog.Printf("Extension upgrade failed: %v", err) + return err + } + 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 { + return err + } + // The child process completed all upgrade steps (including any PR creation). + // Exit the parent so we do not repeat those steps. + os.Exit(0) + } } // Step 1: Update dispatcher agent file (like init command) @@ -285,3 +308,42 @@ func updateAgentFiles(verbose bool) error { return nil } + +// relaunchWithSameArgs re-executes the current binary with the original command-line +// arguments plus the provided extraFlag. stdin/stdout/stderr are forwarded to the child +// 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 + } 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 + // the original slice backing array. + newArgs := append(append([]string(nil), os.Args[1:]...), extraFlag) + upgradeLog.Printf("Re-launching with new binary: %s %v", exe, newArgs) + + cmd := exec.Command(exe, newArgs...) + cmd.Stdin = os.Stdin + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + + if err := cmd.Run(); err != nil { + var exitErr *exec.ExitError + if errors.As(err, &exitErr) { + // Preserve the child's exit code so the caller sees the real failure. + os.Exit(exitErr.ExitCode()) + } + return err + } + return nil +}