diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6b62c44615b..eadd048f2e9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -496,10 +496,10 @@ jobs: env: GH_TOKEN: ${{ github.token }} run: | - echo "Testing update command to ensure it runs without issues..." - # Run update with verbose flag to check for updates without making changes - # The command checks for gh-aw updates, action updates, and workflow updates - ./gh-aw update --verbose --no-actions + echo "Testing update command to ensure it runs successfully..." + # Run update with verbose flag to check for and apply workflow updates from source repositories + # The command may modify workflow files if upstream updates are available + ./gh-aw update --verbose echo "✅ Update command executed successfully" >> $GITHUB_STEP_SUMMARY build: @@ -1143,7 +1143,7 @@ jobs: run: | echo "## Dependency Health Audit" >> $GITHUB_STEP_SUMMARY echo "" >> $GITHUB_STEP_SUMMARY - ./gh-aw update --audit 2>&1 | tee audit_output.txt || true + ./gh-aw upgrade --audit 2>&1 | tee audit_output.txt || true echo "\`\`\`" >> $GITHUB_STEP_SUMMARY head -100 audit_output.txt >> $GITHUB_STEP_SUMMARY echo "\`\`\`" >> $GITHUB_STEP_SUMMARY @@ -1152,7 +1152,7 @@ jobs: id: audit_json run: | # Run audit with JSON output for agent-friendly parsing - ./gh-aw update --audit --json > audit.json 2>&1 + ./gh-aw upgrade --audit --json > audit.json 2>&1 # Display summary in GitHub Actions echo "✅ Dependency audit completed" >> $GITHUB_STEP_SUMMARY diff --git a/docs/src/content/docs/guides/packaging-imports.md b/docs/src/content/docs/guides/packaging-imports.md index fa38e1f9c87..12c14971aba 100644 --- a/docs/src/content/docs/guides/packaging-imports.md +++ b/docs/src/content/docs/guides/packaging-imports.md @@ -50,7 +50,7 @@ gh aw update ci-doctor # update specific workflow gh aw update ci-doctor issue-triage # update multiple ``` -Use `--major`, `--force`, `--engine`, or `--verbose` flags to control update behavior. Semantic versions (e.g., `v1.2.3`) update to latest compatible release within same major version. Branch references update to latest commit. Updates use 3-way merge; when conflicts occur, manually resolve conflict markers and run `gh aw compile`. +Use `--major`, `--force`, `--no-merge`, `--engine`, or `--verbose` flags to control update behavior. Semantic versions (e.g., `v1.2.3`) update to latest compatible release within same major version. Branch references update to latest commit. SHA references update to the latest commit on the default branch. Updates use 3-way merge by default to preserve local changes; use `--no-merge` to replace with the upstream version. When merge conflicts occur, manually resolve conflict markers and run `gh aw compile`. ## Imports diff --git a/docs/src/content/docs/setup/cli.md b/docs/src/content/docs/setup/cli.md index e73ae7ec3f4..a15e1a7520a 100644 --- a/docs/src/content/docs/setup/cli.md +++ b/docs/src/content/docs/setup/cli.md @@ -385,15 +385,16 @@ gh aw remove my-workflow #### `update` -Update workflows based on `source` field (`owner/repo/path@ref`). Default replaces local file; `--merge` performs 3-way merge. Semantic versions update within same major version. +Update workflows based on `source` field (`owner/repo/path@ref`). By default, performs a 3-way merge to preserve local changes; use `--no-merge` to override with upstream. Semantic versions update within same major version. ```bash wrap gh aw update # Update all with source field -gh aw update ci-doctor --merge # Update with 3-way merge +gh aw update ci-doctor # Update specific workflow (3-way merge) +gh aw update ci-doctor --no-merge # Override local changes with upstream gh aw update ci-doctor --major --force # Allow major version updates ``` -**Options:** `--dir`, `--merge`, `--major`, `--force` +**Options:** `--dir`, `--no-merge`, `--major`, `--force`, `--engine`, `--no-stop-after`, `--stop-after` #### `upgrade` @@ -404,9 +405,11 @@ gh aw upgrade # Upgrade repository agent files and gh aw upgrade --no-fix # Update agent files only (skip codemods) gh aw upgrade --push # Upgrade and automatically commit/push gh aw upgrade --push --no-fix # Update agent files and push +gh aw upgrade --audit # Run dependency health audit +gh aw upgrade --audit --json # Dependency audit in JSON format ``` -**Options:** `--dir`, `--no-fix`, `--push` (see [--push flag](#the---push-flag)) +**Options:** `--dir`, `--no-fix`, `--no-actions`, `--push` (see [--push flag](#the---push-flag)), `--audit`, `--json` ### Advanced diff --git a/pkg/cli/update_command.go b/pkg/cli/update_command.go index a46d3382333..407e2372b79 100644 --- a/pkg/cli/update_command.go +++ b/pkg/cli/update_command.go @@ -2,9 +2,7 @@ package cli import ( "fmt" - "os" - "github.com/github/gh-aw/pkg/console" "github.com/github/gh-aw/pkg/constants" "github.com/github/gh-aw/pkg/logger" "github.com/spf13/cobra" @@ -16,97 +14,59 @@ var updateLog = logger.New("cli:update_command") func NewUpdateCommand(validateEngine func(string) error) *cobra.Command { cmd := &cobra.Command{ Use: "update [workflow]...", - Short: "Update agentic workflows from their source repositories and check for gh aw updates", - Long: `Update one or more workflows from their source repositories and check for gh aw updates. + Short: "Update agentic workflows from their source repositories", + Long: `Update one or more workflows from their source repositories. -The command: -1. Checks if a newer version of gh aw is available -2. Updates GitHub Actions versions in .github/aw/actions-lock.json (unless --no-actions is set) -3. Updates workflows using the 'source' field in the workflow frontmatter -4. Compiles each workflow immediately after update -5. Applies automatic fixes (codemods) to ensure workflows follow latest best practices +The update command fetches the latest version of each workflow from its source +repository, merges upstream changes with any local modifications, and recompiles. -By default, the update command replaces local workflow files with the latest version from the source -repository, overriding any local changes. Use the --merge flag to preserve local changes by performing -a 3-way merge between the base version, your local changes, and the latest upstream version. +If no workflow names are specified, all workflows with a 'source' field are updated. + +By default, the update performs a 3-way merge to preserve your local changes. +Use --no-merge to override local changes with the upstream version. For workflow updates, it fetches the latest version based on the current ref: - If the ref is a tag, it updates to the latest release (use --major for major version updates) - If the ref is a branch, it fetches the latest commit from that branch -- Otherwise, it fetches the latest commit from the default branch - -For action updates, it checks each action in .github/aw/actions-lock.json for newer releases -and updates the SHA to pin to the latest version. Use --no-actions to skip action updates. - -DEPENDENCY HEALTH AUDIT: -Use --audit to check dependency health without performing updates. This includes: -- Outdated Go dependencies with available updates -- Security advisories from GitHub Security Advisory API -- Dependency maturity analysis (v0.x vs stable versions) -- Comprehensive dependency health report +- If the ref is a commit SHA, it fetches the latest commit from the default branch -The --audit flag implies --dry-run (no updates performed). +For extension updates, action updates, agent files, and codemods, use 'gh aw upgrade'. ` + WorkflowIDExplanation + ` Examples: - ` + string(constants.CLIExtensionPrefix) + ` update # Check gh aw updates, update actions, and update all workflows - ` + string(constants.CLIExtensionPrefix) + ` update ci-doctor # Check gh aw updates, update actions, and update specific workflow - ` + string(constants.CLIExtensionPrefix) + ` update --no-actions # Skip action updates, only update workflows - ` + string(constants.CLIExtensionPrefix) + ` update ci-doctor.md # Check gh aw updates, update actions, and update specific workflow (alternative format) - ` + string(constants.CLIExtensionPrefix) + ` update ci-doctor --major # Allow major version updates - ` + string(constants.CLIExtensionPrefix) + ` update --merge # Update with 3-way merge to preserve local changes - ` + string(constants.CLIExtensionPrefix) + ` update --pr # Create PR with changes - ` + string(constants.CLIExtensionPrefix) + ` update --force # Force update even if no changes - ` + string(constants.CLIExtensionPrefix) + ` update --dir custom/workflows # Update workflows in custom directory - ` + string(constants.CLIExtensionPrefix) + ` update --audit # Check dependency health without updating - ` + string(constants.CLIExtensionPrefix) + ` update --dry-run # Show what would be updated without making changes`, + ` + string(constants.CLIExtensionPrefix) + ` update # Update all workflows from source + ` + string(constants.CLIExtensionPrefix) + ` update repo-assist # Update a specific workflow + ` + string(constants.CLIExtensionPrefix) + ` update repo-assist.md # Same (alternative format) + ` + string(constants.CLIExtensionPrefix) + ` update --no-merge # Override local changes with upstream + ` + string(constants.CLIExtensionPrefix) + ` update repo-assist --major # Allow major version updates + ` + string(constants.CLIExtensionPrefix) + ` update --force # Force update even if no changes + ` + string(constants.CLIExtensionPrefix) + ` update --dir custom/workflows # Update workflows in custom directory`, RunE: func(cmd *cobra.Command, args []string) error { majorFlag, _ := cmd.Flags().GetBool("major") forceFlag, _ := cmd.Flags().GetBool("force") engineOverride, _ := cmd.Flags().GetString("engine") verbose, _ := cmd.Flags().GetBool("verbose") - prFlag, _ := cmd.Flags().GetBool("pr") workflowDir, _ := cmd.Flags().GetString("dir") noStopAfter, _ := cmd.Flags().GetBool("no-stop-after") stopAfter, _ := cmd.Flags().GetString("stop-after") - mergeFlag, _ := cmd.Flags().GetBool("merge") - noActions, _ := cmd.Flags().GetBool("no-actions") - auditFlag, _ := cmd.Flags().GetBool("audit") - dryRunFlag, _ := cmd.Flags().GetBool("dry-run") - jsonOutput, _ := cmd.Flags().GetBool("json") + noMergeFlag, _ := cmd.Flags().GetBool("no-merge") if err := validateEngine(engineOverride); err != nil { return err } - // Handle audit mode - if auditFlag { - return runDependencyAudit(verbose, jsonOutput) - } - - // Handle dry-run mode - if dryRunFlag { - // TODO: Implement dry-run mode for workflow updates - return fmt.Errorf("--dry-run mode not yet implemented for workflow updates") - } - - return UpdateWorkflowsWithExtensionCheck(args, majorFlag, forceFlag, verbose, engineOverride, prFlag, workflowDir, noStopAfter, stopAfter, mergeFlag, noActions) + return RunUpdateWorkflows(args, majorFlag, forceFlag, verbose, engineOverride, workflowDir, noStopAfter, stopAfter, noMergeFlag) }, } cmd.Flags().Bool("major", false, "Allow major version updates when updating tagged releases") cmd.Flags().BoolP("force", "f", false, "Force update even if no changes are detected") addEngineFlag(cmd) - cmd.Flags().Bool("pr", false, "Create a pull request with the workflow changes") cmd.Flags().StringP("dir", "d", "", "Workflow directory (default: .github/workflows)") cmd.Flags().Bool("no-stop-after", false, "Remove any stop-after field from the workflow") cmd.Flags().String("stop-after", "", "Override stop-after value in the workflow (e.g., '+48h', '2025-12-31 23:59:59')") - cmd.Flags().Bool("merge", false, "Merge local changes with upstream updates instead of overriding") - cmd.Flags().Bool("no-actions", false, "Skip updating GitHub Actions versions") - cmd.Flags().Bool("audit", false, "Check dependency health without performing updates (implies --dry-run)") - cmd.Flags().Bool("dry-run", false, "Show what would be updated without making changes") - cmd.Flags().BoolP("json", "j", false, "Output audit results in JSON format (only with --audit)") + cmd.Flags().Bool("no-merge", false, "Override local changes with upstream version instead of merging") // Register completions for update command cmd.ValidArgsFunction = CompleteWorkflowNames @@ -116,72 +76,14 @@ Examples: return cmd } -// runDependencyAudit performs a dependency health audit -func runDependencyAudit(verbose bool, jsonOutput bool) error { - updateLog.Print("Running dependency health audit") - - // Generate comprehensive report - report, err := GenerateDependencyReport(verbose) - if err != nil { - return fmt.Errorf("failed to generate dependency report: %w", err) - } - - // Display the report - if jsonOutput { - return DisplayDependencyReportJSON(report) - } - DisplayDependencyReport(report) +// RunUpdateWorkflows updates workflows from their source repositories. +// Each workflow is compiled immediately after update. +func RunUpdateWorkflows(workflowNames []string, allowMajor, force, verbose bool, engineOverride string, workflowsDir string, noStopAfter bool, stopAfter string, noMerge bool) error { + updateLog.Printf("Starting update process: workflows=%v, allowMajor=%v, force=%v, noMerge=%v", workflowNames, allowMajor, force, noMerge) - return nil -} - -// UpdateWorkflowsWithExtensionCheck performs the complete update process: -// 1. Check for gh-aw extension updates -// 2. Update GitHub Actions versions (unless --no-actions flag is set) -// 3. Update workflows from source repositories (compiles each workflow after update) -// 4. Apply automatic fixes to updated workflows -// 5. Optionally create a PR -func UpdateWorkflowsWithExtensionCheck(workflowNames []string, allowMajor, force, verbose bool, engineOverride string, createPR bool, workflowsDir string, noStopAfter bool, stopAfter string, merge bool, noActions bool) error { - updateLog.Printf("Starting update process: workflows=%v, allowMajor=%v, force=%v, createPR=%v, merge=%v, noActions=%v", workflowNames, allowMajor, force, createPR, merge, noActions) - - // Step 1: Check for gh-aw extension updates - if err := checkExtensionUpdate(verbose); err != nil { - return fmt.Errorf("extension update check failed: %w", err) - } - - // Step 2: Update GitHub Actions versions (unless disabled) - if !noActions { - if err := UpdateActions(allowMajor, verbose); err != nil { - return fmt.Errorf("action update failed: %w", err) - } - } - - // Step 3: Update workflows from source repositories - // Note: Each workflow is compiled immediately after update - if err := UpdateWorkflows(workflowNames, allowMajor, force, verbose, engineOverride, workflowsDir, noStopAfter, stopAfter, merge); err != nil { + if err := UpdateWorkflows(workflowNames, allowMajor, force, verbose, engineOverride, workflowsDir, noStopAfter, stopAfter, noMerge); err != nil { return fmt.Errorf("workflow update failed: %w", err) } - // Step 4: Apply automatic fixes to updated workflows - fixConfig := FixConfig{ - WorkflowIDs: workflowNames, - Write: true, - Verbose: verbose, - } - if err := RunFix(fixConfig); err != nil { - updateLog.Printf("Fix command failed (non-fatal): %v", err) - // Don't fail the update if fix fails - this is non-critical - if verbose { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Warning: automatic fixes failed: %v", err))) - } - } - - // Step 5: Optionally create PR if flag is set - if createPR { - if err := createUpdatePR(verbose); err != nil { - return fmt.Errorf("failed to create PR: %w", err) - } - } - return nil } diff --git a/pkg/cli/update_command_test.go b/pkg/cli/update_command_test.go index 4fb2eec51a8..c58c8e91c51 100644 --- a/pkg/cli/update_command_test.go +++ b/pkg/cli/update_command_test.go @@ -9,6 +9,8 @@ import ( "testing" "github.com/github/gh-aw/pkg/testutil" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) // TestMergeWorkflowContent_CleanMerge tests a merge with truly non-overlapping changes @@ -699,44 +701,22 @@ This is a test workflow. } // TestUpdateWorkflow_OverrideMode tests the default override mode behavior -func TestUpdateWorkflow_OverrideMode(t *testing.T) { - // In override mode (default), local changes should be replaced with the new version - // This simulates the scenario where: - // - Local file has custom modifications - // - Upstream has different content - // - Without --merge flag, local changes should be discarded - - t.Run("override mode discards local changes", func(t *testing.T) { - // This test verifies that in override mode (merge=false), - // the update function would replace local content with upstream content - // We're testing the logic path, not the full integration - - merge := false // Default override mode - - if merge { - t.Error("Expected merge to be false in override mode") - } +func TestUpdateWorkflow_DefaultMergeMode(t *testing.T) { + // By default, merge mode is on. Local changes should be preserved via 3-way merge. + t.Run("merge is default when noMerge is false", func(t *testing.T) { + noMerge := false // default + merge := !noMerge + assert.True(t, merge, "merge should be true by default") }) } -// TestUpdateWorkflow_MergeMode tests the merge mode behavior with --merge flag -func TestUpdateWorkflow_MergeMode(t *testing.T) { - // In merge mode (--merge flag), local changes should be preserved via 3-way merge - // This simulates the scenario where: - // - Local file has custom modifications - // - Upstream has different content - // - With --merge flag, local changes should be merged with upstream - - t.Run("merge mode preserves local changes", func(t *testing.T) { - // This test verifies that in merge mode (merge=true), - // the update function would perform a 3-way merge - // We're testing the logic path, not the full integration - - merge := true // Merge mode enabled - - if !merge { - t.Error("Expected merge to be true in merge mode") - } +// TestUpdateWorkflow_NoMergeMode tests that --no-merge disables merge mode +func TestUpdateWorkflow_NoMergeMode(t *testing.T) { + // With --no-merge, local changes should be overridden with upstream. + t.Run("no-merge overrides local changes", func(t *testing.T) { + noMerge := true // --no-merge + merge := !noMerge + assert.False(t, merge, "merge should be false when --no-merge is set") }) } @@ -903,17 +883,18 @@ func TestUpdateActions_InvalidJSON(t *testing.T) { } } -// TestResolveLatestRef_CommitSHA tests that commit SHAs are returned as-is +// TestResolveLatestRef_CommitSHA tests that commit SHAs are correctly identified +// and trigger default branch resolution (which requires API access). func TestResolveLatestRef_CommitSHA(t *testing.T) { - // Test with a valid commit SHA sha := "ea350161ad5dcc9624cf510f134c6a9e39a6f94d" - result, err := resolveLatestRef("test/repo", sha, false, false) - if err != nil { - t.Fatalf("Expected no error for commit SHA, got: %v", err) - } - if result != sha { - t.Errorf("Expected commit SHA to be returned as-is, got: %s", result) - } + assert.True(t, IsCommitSHA(sha), "Should be recognized as a commit SHA") + + // resolveLatestRef for a SHA requires GitHub API access to look up the + // default branch. In environments without API access it will error; + // in authenticated environments it will succeed. Either outcome is + // acceptable — the key invariant is that the SHA is correctly + // identified (tested above) and the function does not panic. + _, _ = resolveLatestRef("test/repo", sha, false, false) } // TestResolveLatestRef_NotCommitSHA tests that non-SHA refs are handled appropriately @@ -941,104 +922,82 @@ func TestResolveLatestRef_NotCommitSHA(t *testing.T) { } } -// TestUpdateWorkflowsWithExtensionCheck_FixIntegration tests that fix is called during update -func TestUpdateWorkflowsWithExtensionCheck_FixIntegration(t *testing.T) { - // This test verifies that the fix functionality is integrated into the update flow - // We create a workflow with a deprecated field, update it, and verify the fix is applied - - // Create a temporary directory - tmpDir := testutil.TempDir(t, "test-*") - originalDir, _ := os.Getwd() - defer os.Chdir(originalDir) - - // Create .github/workflows directory - workflowsDir := filepath.Join(tmpDir, ".github", "workflows") - if err := os.MkdirAll(workflowsDir, 0755); err != nil { - t.Fatalf("Failed to create workflows directory: %v", err) - } - - // Create a workflow file with deprecated field - workflowContent := `--- -on: - workflow_dispatch: - -timeout_minutes: 30 - -permissions: - contents: read ---- - -# Test Workflow - -This is a test workflow with deprecated field. -` - - workflowPath := filepath.Join(workflowsDir, "test-workflow.md") - if err := os.WriteFile(workflowPath, []byte(workflowContent), 0644); err != nil { - t.Fatalf("Failed to write workflow file: %v", err) - } - - os.Chdir(tmpDir) - - // Test that fix config is created properly - fixConfig := FixConfig{ - WorkflowIDs: []string{}, - Write: true, - Verbose: false, +// TestShortRef tests the shortRef helper for abbreviating refs in messages +func TestShortRef(t *testing.T) { + tests := []struct { + name string + ref string + expected string + }{ + {"commit SHA", "ea350161ad5dcc9624cf510f134c6a9e39a6f94d", "ea35016"}, + {"branch name", "main", "main"}, + {"version tag", "v1.2.3", "v1.2.3"}, + {"short string", "abc", "abc"}, + {"empty string", "", ""}, } - // Verify the config has expected fields - if fixConfig.Write != true { - t.Error("Expected Write to be true") + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := shortRef(tt.ref) + assert.Equal(t, tt.expected, result, "shortRef(%q) should return %q", tt.ref, tt.expected) + }) } +} - // Test running fix on the workflow - err := RunFix(fixConfig) - if err != nil { - t.Logf("Fix command returned error (may be expected in test environment): %v", err) +// TestIsBranchRef tests the isBranchRef helper that distinguishes branch names +// from semantic version tags and commit SHAs +func TestIsBranchRef(t *testing.T) { + tests := []struct { + name string + ref string + expected bool + }{ + {"branch main", "main", true}, + {"branch develop", "develop", true}, + {"branch with slash", "feature/foo", true}, + {"semver tag", "v1.2.3", false}, + {"semver tag with v", "v0.1.0", false}, + {"commit SHA", "ea350161ad5dcc9624cf510f134c6a9e39a6f94d", false}, } - // Read the workflow file to check if fix was attempted - updatedContent, err := os.ReadFile(workflowPath) - if err != nil { - t.Fatalf("Failed to read updated workflow file: %v", err) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := isBranchRef(tt.ref) + assert.Equal(t, tt.expected, result, "isBranchRef(%q) should return %v", tt.ref, tt.expected) + }) } +} - updatedStr := string(updatedContent) +// TestRunUpdateWorkflows_NoSourceWorkflows tests that RunUpdateWorkflows errors when no source workflows exist +func TestRunUpdateWorkflows_NoSourceWorkflows(t *testing.T) { + tmpDir := testutil.TempDir(t, "test-*") + originalDir, _ := os.Getwd() + defer os.Chdir(originalDir) - // Check if the deprecated field was replaced - // Note: This test may not apply the fix if the fix system isn't fully initialized, - // but we're testing that the integration code path exists and doesn't error - if strings.Contains(updatedStr, "timeout_minutes:") { - t.Logf("Deprecated field still present (fix may not have been applied in test environment)") - } + // Create empty .github/workflows directory + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + require.NoError(t, os.MkdirAll(workflowsDir, 0755)) + os.Chdir(tmpDir) - if strings.Contains(updatedStr, "timeout-minutes:") { - t.Log("Fix was successfully applied - deprecated field was replaced") - } + // Running update with no source workflows should fail + err := RunUpdateWorkflows(nil, false, false, false, "", "", false, "", false) + require.Error(t, err, "Should error when no workflows with source field exist") + assert.Contains(t, err.Error(), "no workflows found with source field") } -// TestUpdateWorkflowsWithExtensionCheck_FixNonFatal tests that update continues if fix fails -func TestUpdateWorkflowsWithExtensionCheck_FixNonFatal(t *testing.T) { - // This test verifies that if the fix step fails, the update process continues - // and doesn't fail the entire update operation - - // Create a fix config that would process workflows - fixConfig := FixConfig{ - WorkflowIDs: []string{}, - Write: true, - Verbose: false, - } +// TestRunUpdateWorkflows_SpecificWorkflowNotFound tests that RunUpdateWorkflows errors for unknown workflow name +func TestRunUpdateWorkflows_SpecificWorkflowNotFound(t *testing.T) { + tmpDir := testutil.TempDir(t, "test-*") + originalDir, _ := os.Getwd() + defer os.Chdir(originalDir) - // The fix should handle missing workflows gracefully - err := RunFix(fixConfig) + // Create empty .github/workflows directory + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + require.NoError(t, os.MkdirAll(workflowsDir, 0755)) + os.Chdir(tmpDir) - // The error might be about no workflows found, which is acceptable - if err != nil { - if !strings.Contains(err.Error(), "No workflow files found") { - // If it's a different error, that's fine too - we just want to ensure - // the function can be called and returns - t.Logf("Fix returned error (expected in test environment): %v", err) - } - } + // Running update with a specific name that doesn't exist should fail + err := RunUpdateWorkflows([]string{"nonexistent"}, false, false, false, "", "", false, "", false) + require.Error(t, err, "Should error when specified workflow not found") + assert.Contains(t, err.Error(), "no workflows found matching the specified names") } diff --git a/pkg/cli/update_git.go b/pkg/cli/update_git.go deleted file mode 100644 index 8f3f390f439..00000000000 --- a/pkg/cli/update_git.go +++ /dev/null @@ -1,101 +0,0 @@ -package cli - -import ( - "fmt" - "math/rand" - "os" - "os/exec" - "strings" - - "github.com/github/gh-aw/pkg/console" - "github.com/github/gh-aw/pkg/logger" - "github.com/github/gh-aw/pkg/workflow" -) - -var updateGitLog = logger.New("cli:update_git") - -// hasGitChanges checks if there are any uncommitted changes -func hasGitChanges() (bool, error) { - cmd := exec.Command("git", "status", "--porcelain") - output, err := cmd.Output() - if err != nil { - return false, fmt.Errorf("failed to check git status: %w", err) - } - - hasChanges := len(strings.TrimSpace(string(output))) > 0 - updateGitLog.Printf("Git changes detected: %t", hasChanges) - return hasChanges, nil -} - -// runGitCommand runs a git command with the specified arguments -func runGitCommand(args ...string) error { - cmd := exec.Command("git", args...) - if err := cmd.Run(); err != nil { - return fmt.Errorf("git %s failed: %w", strings.Join(args, " "), err) - } - return nil -} - -// createUpdatePR creates a pull request with the workflow changes -func createUpdatePR(verbose bool) error { - updateGitLog.Print("Creating update PR for workflow changes") - // Check if GitHub CLI is available - if !isGHCLIAvailable() { - return fmt.Errorf("GitHub CLI (gh) is required for PR creation but not found in PATH") - } - - // Check if there are any changes to commit - hasChanges, err := hasGitChanges() - if err != nil { - return fmt.Errorf("failed to check git status: %w", err) - } - - if !hasChanges { - updateGitLog.Print("No git changes found, skipping PR creation") - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("No changes to create PR for")) - return nil - } - - if verbose { - fmt.Fprintln(os.Stderr, console.FormatVerboseMessage("Creating pull request with workflow updates...")) - } - - // Create a branch name with timestamp - randomNum := rand.Intn(9000) + 1000 // Generate number between 1000-9999 - branchName := fmt.Sprintf("update-workflows-%d", randomNum) - updateGitLog.Printf("Creating branch: %s", branchName) - - // Create and checkout new branch - if err := runGitCommand("checkout", "-b", branchName); err != nil { - return fmt.Errorf("failed to create branch: %w", err) - } - - // Add all changes - if err := runGitCommand("add", "."); err != nil { - return fmt.Errorf("failed to add changes: %w", err) - } - - // Commit changes - commitMsg := "Update workflows and recompile" - if err := runGitCommand("commit", "-m", commitMsg); err != nil { - return fmt.Errorf("failed to commit changes: %w", err) - } - - // Push branch - if err := runGitCommand("push", "-u", "origin", branchName); err != nil { - return fmt.Errorf("failed to push branch: %w", err) - } - - // Create PR - output, err := workflow.RunGHCombined("Creating pull request...", "pr", "create", - "--title", "Update workflows and recompile", - "--body", "This PR updates workflows from their source repositories and recompiles them.\n\nGenerated by `gh aw update --pr`") - if err != nil { - return fmt.Errorf("failed to create PR: %w\nOutput: %s", err, string(output)) - } - - fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Successfully created pull request")) - fmt.Fprintln(os.Stderr, console.FormatInfoMessage(strings.TrimSpace(string(output)))) - - return nil -} diff --git a/pkg/cli/update_integration_test.go b/pkg/cli/update_integration_test.go new file mode 100644 index 00000000000..58ed7ca0e69 --- /dev/null +++ b/pkg/cli/update_integration_test.go @@ -0,0 +1,373 @@ +//go:build integration + +package cli + +import ( + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + + "github.com/github/gh-aw/pkg/fileutil" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// updateIntegrationTestSetup holds the setup state for update integration tests +type updateIntegrationTestSetup struct { + tempDir string + originalWd string + binaryPath string + workflowsDir string + cleanup func() +} + +// setupUpdateIntegrationTest creates a minimal test environment for update command: +// - temporary directory +// - git init (required by update command) +// - pre-built gh-aw binary +// - .github/workflows directory +func setupUpdateIntegrationTest(t *testing.T) *updateIntegrationTestSetup { + t.Helper() + + // Create a temporary directory for the test + tempDir, err := os.MkdirTemp("", "gh-aw-update-integration-*") + require.NoError(t, err, "Failed to create temp directory") + + // Save current working directory and change to temp directory + originalWd, err := os.Getwd() + require.NoError(t, err, "Failed to get current working directory") + + err = os.Chdir(tempDir) + require.NoError(t, err, "Failed to change to temp directory") + + // Initialize git repository (required by update command for merge detection) + gitInitCmd := exec.Command("git", "init") + gitInitCmd.Dir = tempDir + output, err := gitInitCmd.CombinedOutput() + require.NoError(t, err, "Failed to run git init: %s", string(output)) + + // Configure git user for commits + gitConfigName := exec.Command("git", "config", "user.name", "Test User") + gitConfigName.Dir = tempDir + _ = gitConfigName.Run() + + gitConfigEmail := exec.Command("git", "config", "user.email", "test@example.com") + gitConfigEmail.Dir = tempDir + _ = gitConfigEmail.Run() + + // Copy the pre-built binary + binaryPath := filepath.Join(tempDir, "gh-aw") + err = fileutil.CopyFile(globalBinaryPath, binaryPath) + require.NoError(t, err, "Failed to copy gh-aw binary to temp directory") + + err = os.Chmod(binaryPath, 0755) + require.NoError(t, err, "Failed to make binary executable") + + // Create .github/workflows directory + workflowsDir := filepath.Join(tempDir, ".github", "workflows") + require.NoError(t, os.MkdirAll(workflowsDir, 0755), "Failed to create workflows directory") + + cleanup := func() { + _ = os.Chdir(originalWd) + _ = os.RemoveAll(tempDir) + } + + return &updateIntegrationTestSetup{ + tempDir: tempDir, + originalWd: originalWd, + binaryPath: binaryPath, + workflowsDir: workflowsDir, + cleanup: cleanup, + } +} + +// skipWithoutGitHubAuth skips the test if GitHub authentication is not available +func skipWithoutGitHubAuth(t *testing.T) { + t.Helper() + authCmd := exec.Command("gh", "auth", "status") + if err := authCmd.Run(); err != nil { + t.Skip("Skipping test: GitHub authentication not available (gh auth status failed)") + } +} + +// --- Ref Resolution Integration Tests --- + +// TestResolveLatestRef_TagIntegration verifies that a semantic version tag +// resolves to the latest release (within the same major version by default). +func TestResolveLatestRef_TagIntegration(t *testing.T) { + skipWithoutGitHubAuth(t) + + // Use a well-known public repo with releases + // actions/checkout has many tagged releases (v3.x, v4.x, etc.) + latestRef, err := resolveLatestRef("actions/checkout", "v4.0.0", false, true) + require.NoError(t, err, "Should resolve latest release for actions/checkout v4.x") + + // The resolved ref should be a newer v4.x tag + assert.True(t, isSemanticVersionTag(latestRef), "Resolved ref should be a semantic version tag, got: %s", latestRef) + assert.True(t, strings.HasPrefix(latestRef, "v4."), "Should stay within v4.x major version, got: %s", latestRef) +} + +// TestResolveLatestRef_TagMajorUpdateIntegration verifies that --major allows cross-major updates. +func TestResolveLatestRef_TagMajorUpdateIntegration(t *testing.T) { + skipWithoutGitHubAuth(t) + + // With allowMajor=true, it should resolve to the latest release across all major versions + latestRef, err := resolveLatestRef("actions/checkout", "v3.0.0", true, true) + require.NoError(t, err, "Should resolve latest release with major updates allowed") + + assert.True(t, isSemanticVersionTag(latestRef), "Resolved ref should be a semantic version tag, got: %s", latestRef) + // With major updates allowed, it might return v4.x or later +} + +// TestResolveLatestRef_BranchIntegration verifies that a branch name resolves +// to the latest commit SHA for that branch. +func TestResolveLatestRef_BranchIntegration(t *testing.T) { + skipWithoutGitHubAuth(t) + + // Use a well-known branch on a public repo + latestRef, err := resolveLatestRef("actions/checkout", "main", false, true) + require.NoError(t, err, "Should resolve latest commit for branch 'main'") + + // The result should be a 40-char commit SHA + assert.True(t, IsCommitSHA(latestRef), "Branch resolution should return a commit SHA, got: %s", latestRef) +} + +// TestResolveLatestRef_CommitSHAIntegration verifies that a commit SHA resolves +// to the latest commit from the default branch. +func TestResolveLatestRef_CommitSHAIntegration(t *testing.T) { + skipWithoutGitHubAuth(t) + + // Use a known commit SHA from actions/checkout + // This is an older commit — the resolution should return the latest commit on the default branch + oldSHA := "f43a0e5ff2bd294095638e18286ca9a3d1956744" // Known old commit + + latestRef, err := resolveLatestRef("actions/checkout", oldSHA, false, true) + require.NoError(t, err, "Should resolve latest commit from default branch") + + // The result should be a 40-char commit SHA (the latest on main) + assert.True(t, IsCommitSHA(latestRef), "SHA resolution should return a commit SHA, got: %s", latestRef) + // The result should be different from the old SHA if there have been newer commits + // (actions/checkout is actively maintained, so this should be true) + assert.NotEqual(t, oldSHA, latestRef, "Should resolve to a newer commit than the old SHA") +} + +// --- Update Command Integration Tests --- + +// TestUpdateCommand_NoMergeFlag verifies that --no-merge flag is recognized. +func TestUpdateCommand_NoMergeFlag(t *testing.T) { + setup := setupUpdateIntegrationTest(t) + defer setup.cleanup() + + // Run the update command with --no-merge flag — should be accepted + cmd := exec.Command(setup.binaryPath, "update", "--no-merge", "--verbose") + cmd.Dir = setup.tempDir + output, err := cmd.CombinedOutput() + outputStr := string(output) + + // It should fail because there are no workflows with source, not because --no-merge is unknown + assert.Error(t, err, "Should fail (no source workflows), output: %s", outputStr) + assert.NotContains(t, outputStr, "unknown flag", "The --no-merge flag should be recognized") + assert.Contains(t, outputStr, "no workflows found", "Should report no workflows found") +} + +// TestUpdateCommand_RemovedFlags verifies that old flags are no longer accepted. +func TestUpdateCommand_RemovedFlags(t *testing.T) { + setup := setupUpdateIntegrationTest(t) + defer setup.cleanup() + + removedFlags := []string{"--merge", "--pr", "--no-actions", "--audit", "--dry-run", "--json"} + + for _, flag := range removedFlags { + t.Run(flag, func(t *testing.T) { + cmd := exec.Command(setup.binaryPath, "update", flag) + cmd.Dir = setup.tempDir + output, err := cmd.CombinedOutput() + outputStr := string(output) + + assert.Error(t, err, "Should reject removed flag %s", flag) + assert.Contains(t, outputStr, "unknown flag", "Should report %s as unknown flag, output: %s", flag, outputStr) + }) + } +} + +// TestUpdateCommand_HelpText verifies the update command help text is correct. +func TestUpdateCommand_HelpText(t *testing.T) { + setup := setupUpdateIntegrationTest(t) + defer setup.cleanup() + + cmd := exec.Command(setup.binaryPath, "update", "--help") + cmd.Dir = setup.tempDir + output, err := cmd.CombinedOutput() + require.NoError(t, err, "Help should not return error") + + outputStr := string(output) + + // Should mention merge behavior + assert.Contains(t, outputStr, "no-merge", "Help should document --no-merge flag") + assert.Contains(t, outputStr, "3-way merge", "Help should explain merge behavior") + + // Should reference upgrade for other features + assert.Contains(t, outputStr, "upgrade", "Help should reference 'gh aw upgrade' for other features") + + // Should NOT mention removed flags + assert.NotContains(t, outputStr, "--pr", "Help should not mention removed --pr flag") + assert.NotContains(t, outputStr, "--audit", "Help should not mention removed --audit flag") + assert.NotContains(t, outputStr, "--dry-run", "Help should not mention removed --dry-run flag") +} + +// --- Merge Behavior Integration Tests --- + +// TestUpdateCommand_MergeIsDefault verifies that merge is the default behavior +// when a workflow has local modifications. +func TestUpdateCommand_MergeIsDefault(t *testing.T) { + skipWithoutGitHubAuth(t) + + setup := setupUpdateIntegrationTest(t) + defer setup.cleanup() + + // Create a workflow with a source field that can be fetched + workflowContent := `--- +source: github/gh-aw/.github/workflows/smoke-test-push.md@main +on: + workflow_dispatch: + +permissions: + contents: read +--- + +# Test Workflow + +Local modification that should be preserved during merge. +` + workflowPath := filepath.Join(setup.workflowsDir, "test-update.md") + require.NoError(t, os.WriteFile(workflowPath, []byte(workflowContent), 0644)) + + // Commit the workflow to git so hasLocalModifications can detect changes + gitAdd := exec.Command("git", "add", "-A") + gitAdd.Dir = setup.tempDir + require.NoError(t, gitAdd.Run(), "Failed to git add") + + gitCommit := exec.Command("git", "commit", "-m", "initial commit") + gitCommit.Dir = setup.tempDir + require.NoError(t, gitCommit.Run(), "Failed to git commit") + + // Make a local modification + modifiedContent := strings.Replace(workflowContent, + "Local modification that should be preserved during merge.", + "This is my custom local addition.\n\nLocal modification that should be preserved during merge.", + 1) + require.NoError(t, os.WriteFile(workflowPath, []byte(modifiedContent), 0644)) + + // Run update with default merge behavior (no --no-merge flag) + cmd := exec.Command(setup.binaryPath, "update", "test-update", "--verbose") + cmd.Dir = setup.tempDir + output, _ := cmd.CombinedOutput() + outputStr := string(output) + + // The command should produce output (may succeed or fail depending on + // source repo accessibility, but must attempt merge, not override). + t.Logf("Update output: %s", outputStr) + assert.NotEmpty(t, outputStr, "Update command should produce output") + + // Verify it does NOT report "override mode" — merge is the default + assert.NotContains(t, outputStr, "Using override mode", + "Default behavior should use merge, not override") + + // Read the resulting workflow file to verify it still exists + updatedContent, err := os.ReadFile(filepath.Join(setup.workflowsDir, "test-update.md")) + require.NoError(t, err, "Workflow file should still exist after update") + assert.Contains(t, string(updatedContent), "Local modification", + "Local content should be preserved when merge is the default") +} + +// --- ParseSourceSpec Integration Tests --- + +// TestParseSourceSpec_Integration verifies source spec parsing for various formats. +func TestParseSourceSpec_Integration(t *testing.T) { + tests := []struct { + name string + source string + expectedRepo string + expectedPath string + expectedRef string + }{ + { + name: "standard format with tag", + source: "github/gh-aw/.github/workflows/workflow.md@v1.2.3", + expectedRepo: "github/gh-aw", + expectedPath: ".github/workflows/workflow.md", + expectedRef: "v1.2.3", + }, + { + name: "standard format with branch", + source: "githubnext/agentics/workflows/repo-assist.md@main", + expectedRepo: "githubnext/agentics", + expectedPath: "workflows/repo-assist.md", + expectedRef: "main", + }, + { + name: "standard format with commit SHA", + source: "githubnext/agentics/workflows/repo-assist.md@6c79ed2ea350161ad5dcc9624cf510f134c6a9e3", + expectedRepo: "githubnext/agentics", + expectedPath: "workflows/repo-assist.md", + expectedRef: "6c79ed2ea350161ad5dcc9624cf510f134c6a9e3", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + spec, err := parseSourceSpec(tt.source) + require.NoError(t, err, "Should parse source spec: %s", tt.source) + + assert.Equal(t, tt.expectedRepo, spec.Repo, "Repo should match") + assert.Equal(t, tt.expectedPath, spec.Path, "Path should match") + assert.Equal(t, tt.expectedRef, spec.Ref, "Ref should match") + }) + } +} + +// TestIsCommitSHA_Integration tests commit SHA identification with real-world examples. +func TestIsCommitSHA_Integration(t *testing.T) { + tests := []struct { + name string + ref string + expected bool + }{ + {"real commit SHA", "6c79ed2ea350161ad5dcc9624cf510f134c6a9e3", true}, + {"another SHA", "f43a0e5ff2bd294095638e18286ca9a3d1956744", true}, + {"v-prefixed tag", "v1.2.3", false}, + {"branch name", "main", false}, + {"short SHA", "6c79ed2", false}, + {"39 chars", "6c79ed2ea350161ad5dcc9624cf510f134c6a9e", false}, + {"41 chars", "6c79ed2ea350161ad5dcc9624cf510f134c6a9e39", false}, + {"non-hex 40 chars", "ghijklmnopqrstuvwxyz12345678901234567890", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, IsCommitSHA(tt.ref), "IsCommitSHA(%q)", tt.ref) + }) + } +} + +// --- Helper Function Tests --- + +// TestGetRepoDefaultBranch_Integration verifies fetching the default branch via GitHub API. +func TestGetRepoDefaultBranch_Integration(t *testing.T) { + skipWithoutGitHubAuth(t) + + branch, err := getRepoDefaultBranch("actions/checkout") + require.NoError(t, err, "Should fetch default branch for actions/checkout") + assert.Equal(t, "main", branch, "actions/checkout default branch should be 'main'") +} + +// TestGetLatestBranchCommitSHA_Integration verifies fetching the latest commit SHA for a branch. +func TestGetLatestBranchCommitSHA_Integration(t *testing.T) { + skipWithoutGitHubAuth(t) + + sha, err := getLatestBranchCommitSHA("actions/checkout", "main") + require.NoError(t, err, "Should fetch latest commit SHA for actions/checkout main branch") + assert.True(t, IsCommitSHA(sha), "Result should be a 40-char commit SHA, got: %s", sha) +} diff --git a/pkg/cli/update_workflows.go b/pkg/cli/update_workflows.go index cde48d5be7b..3379b8c19f3 100644 --- a/pkg/cli/update_workflows.go +++ b/pkg/cli/update_workflows.go @@ -2,6 +2,7 @@ package cli import ( "fmt" + "net/url" "os" "path/filepath" "strings" @@ -12,8 +13,8 @@ import ( ) // UpdateWorkflows updates workflows from their source repositories -func UpdateWorkflows(workflowNames []string, allowMajor, force, verbose bool, engineOverride string, workflowsDir string, noStopAfter bool, stopAfter string, merge bool) error { - updateLog.Printf("Scanning for workflows with source field: dir=%s, filter=%v, merge=%v", workflowsDir, workflowNames, merge) +func UpdateWorkflows(workflowNames []string, allowMajor, force, verbose bool, engineOverride string, workflowsDir string, noStopAfter bool, stopAfter string, noMerge bool) error { + updateLog.Printf("Scanning for workflows with source field: dir=%s, filter=%v, noMerge=%v", workflowsDir, workflowNames, noMerge) // Use provided workflows directory or default if workflowsDir == "" { @@ -44,7 +45,7 @@ func UpdateWorkflows(workflowNames []string, allowMajor, force, verbose bool, en // Update each workflow for _, wf := range workflows { updateLog.Printf("Updating workflow: %s (source: %s)", wf.Name, wf.SourceSpec) - if err := updateWorkflow(wf, allowMajor, force, verbose, engineOverride, noStopAfter, stopAfter, merge); err != nil { + if err := updateWorkflow(wf, allowMajor, force, verbose, engineOverride, noStopAfter, stopAfter, noMerge); err != nil { updateLog.Printf("Failed to update workflow %s: %v", wf.Name, err) failedUpdates = append(failedUpdates, updateFailure{ Name: wf.Name, @@ -168,12 +169,10 @@ func resolveLatestRef(repo, currentRef string, allowMajor, verbose bool) (string // Check if current ref is a commit SHA (40-character hex string) if IsCommitSHA(currentRef) { - updateLog.Printf("Current ref is a commit SHA: %s, returning as-is", currentRef) - if verbose { - fmt.Fprintln(os.Stderr, console.FormatVerboseMessage(fmt.Sprintf("Ref %s is a commit SHA, already pinned to specific commit", currentRef))) - } - // Commit SHAs are already pinned to a specific commit, no need to resolve - return currentRef, nil + updateLog.Printf("Current ref is a commit SHA: %s, fetching latest from default branch", currentRef) + // The source field only contains a pinned SHA with no branch information. + // Fetch the latest commit from the default branch to check for updates. + return resolveLatestCommitFromDefaultBranch(repo, currentRef, verbose) } // Otherwise, treat as branch and get latest commit @@ -182,17 +181,76 @@ func resolveLatestRef(repo, currentRef string, allowMajor, verbose bool) (string } // Get the latest commit SHA for the branch - output, err := workflow.RunGH("Fetching branch info...", "api", fmt.Sprintf("/repos/%s/branches/%s", repo, currentRef), "--jq", ".commit.sha") + latestSHA, err := getLatestBranchCommitSHA(repo, currentRef) if err != nil { return "", fmt.Errorf("failed to get latest commit for branch %s: %w", currentRef, err) } - latestSHA := strings.TrimSpace(string(output)) updateLog.Printf("Latest commit for branch %s: %s", currentRef, latestSHA) - // For branches, we return the branch name, not the SHA - // The source spec will remain as branch@branchname - return currentRef, nil + // Return the SHA for comparison so we can detect upstream changes. + // The caller (updateWorkflow) preserves the branch name in the source + // field to avoid SHA-pinning — see isBranchRef() usage there. + return latestSHA, nil +} + +// resolveLatestCommitFromDefaultBranch fetches the latest commit SHA from +// the default branch of a repo. This is used when the source field is pinned +// to a commit SHA with no branch information — in that case we can only +// logically track the default branch. +func resolveLatestCommitFromDefaultBranch(repo, currentSHA string, verbose bool) (string, error) { + // Get the default branch name + defaultBranch, err := getRepoDefaultBranch(repo) + if err != nil { + return "", fmt.Errorf("failed to get default branch for %s: %w", repo, err) + } + + updateLog.Printf("Source is pinned to commit SHA, tracking default branch %q of %s", defaultBranch, repo) + if verbose { + fmt.Fprintln(os.Stderr, console.FormatVerboseMessage(fmt.Sprintf("Source is pinned to commit SHA, checking default branch %q for updates", defaultBranch))) + } + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Source has no branch ref, tracking default branch %q", defaultBranch))) + + // Get the latest commit SHA from the default branch + latestSHA, err := getLatestBranchCommitSHA(repo, defaultBranch) + if err != nil { + return "", fmt.Errorf("failed to get latest commit for default branch %s: %w", defaultBranch, err) + } + + updateLog.Printf("Latest commit on default branch %s: %s (current: %s)", defaultBranch, latestSHA, currentSHA) + + return latestSHA, nil +} + +// getRepoDefaultBranch fetches the default branch name for a repository. +func getRepoDefaultBranch(repo string) (string, error) { + output, err := workflow.RunGH("Fetching repo info...", "api", fmt.Sprintf("/repos/%s", repo), "--jq", ".default_branch") + if err != nil { + return "", err + } + + branch := strings.TrimSpace(string(output)) + if branch == "" { + return "", fmt.Errorf("empty default branch returned for %s", repo) + } + + return branch, nil +} + +// getLatestBranchCommitSHA fetches the latest commit SHA for a given branch. +func getLatestBranchCommitSHA(repo, branch string) (string, error) { + // URL-encode the branch name since it may contain slashes (e.g. "feature/foo") + output, err := workflow.RunGH("Fetching branch info...", "api", fmt.Sprintf("/repos/%s/branches/%s", repo, url.PathEscape(branch)), "--jq", ".commit.sha") + if err != nil { + return "", err + } + + sha := strings.TrimSpace(string(output)) + if sha == "" { + return "", fmt.Errorf("empty commit SHA returned for branch %s", branch) + } + + return sha, nil } // resolveLatestRelease resolves the latest compatible release for a workflow source @@ -259,8 +317,8 @@ func resolveLatestRelease(repo, currentRef string, allowMajor, verbose bool) (st } // updateWorkflow updates a single workflow from its source -func updateWorkflow(wf *workflowWithSource, allowMajor, force, verbose bool, engineOverride string, noStopAfter bool, stopAfter string, merge bool) error { - updateLog.Printf("Updating workflow: name=%s, source=%s, force=%v, merge=%v", wf.Name, wf.SourceSpec, force, merge) +func updateWorkflow(wf *workflowWithSource, allowMajor, force, verbose bool, engineOverride string, noStopAfter bool, stopAfter string, noMerge bool) error { + updateLog.Printf("Updating workflow: name=%s, source=%s, force=%v, noMerge=%v", wf.Name, wf.SourceSpec, force, noMerge) if verbose { fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("\nUpdating workflow: %s", wf.Name))) @@ -286,6 +344,14 @@ func updateWorkflow(wf *workflowWithSource, allowMajor, force, verbose bool, eng return fmt.Errorf("failed to resolve latest ref: %w", err) } + // For branch refs, resolveLatestRef returns the branch-head SHA so that + // we can detect upstream changes (currentRef != latestRef). However the + // source field must keep the branch *name* to avoid SHA-pinning. + sourceFieldRef := latestRef + if isBranchRef(currentRef) { + sourceFieldRef = currentRef + } + if verbose { fmt.Fprintln(os.Stderr, console.FormatVerboseMessage(fmt.Sprintf("Current ref: %s", currentRef))) fmt.Fprintln(os.Stderr, console.FormatVerboseMessage(fmt.Sprintf("Latest ref: %s", latestRef))) @@ -302,7 +368,7 @@ func updateWorkflow(wf *workflowWithSource, allowMajor, force, verbose bool, eng if verbose { fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to download source for comparison: %v", err))) } - fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Workflow %s is already up to date (%s)", wf.Name, currentRef))) + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Workflow %s is already up to date (%s)", wf.Name, shortRef(currentRef)))) return nil } @@ -315,13 +381,13 @@ func updateWorkflow(wf *workflowWithSource, allowMajor, force, verbose bool, eng // Check if local file differs from source if hasLocalModifications(string(sourceContent), string(currentContent), wf.SourceSpec, verbose) { updateLog.Printf("Local modifications detected in workflow: %s", wf.Name) - fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Workflow %s is already up to date (%s)", wf.Name, currentRef))) + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Workflow %s is already up to date (%s)", wf.Name, shortRef(currentRef)))) fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("⚠️ Local copy of %s has been modified from source", wf.Name))) return nil } updateLog.Printf("Workflow %s is up to date with no local modifications", wf.Name) - fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Workflow %s is already up to date (%s)", wf.Name, currentRef))) + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Workflow %s is already up to date (%s)", wf.Name, shortRef(currentRef)))) return nil } @@ -335,6 +401,27 @@ func updateWorkflow(wf *workflowWithSource, allowMajor, force, verbose bool, eng return fmt.Errorf("failed to download workflow: %w", err) } + // Determine merge mode. Merge is the default behaviour — it detects + // local modifications and performs a 3-way merge to preserve them. + // When --no-merge is used, local changes are overridden with upstream. + merge := !noMerge + + // When merge mode is on, detect local modifications to confirm we + // actually need to merge (if no local mods, override is fine either way). + if merge { + baseContent, dlErr := downloadWorkflowContent(sourceSpec.Repo, sourceSpec.Path, currentRef, verbose) + if dlErr == nil { + localContent, readErr := os.ReadFile(wf.Path) + if readErr == nil && hasLocalModifications(string(baseContent), string(localContent), wf.SourceSpec, verbose) { + updateLog.Printf("Local modifications detected in %s, merging to preserve changes", wf.Name) + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Local modifications detected in %s, merging to preserve your changes", wf.Name))) + } else { + // No local modifications — no need to merge, just override + merge = false + } + } + } + var finalContent string var hasConflicts bool @@ -363,7 +450,7 @@ func updateWorkflow(wf *workflowWithSource, allowMajor, force, verbose bool, eng // Perform 3-way merge using git merge-file updateLog.Printf("Performing 3-way merge for workflow: %s", wf.Name) - mergedContent, conflicts, err := MergeWorkflowContent(string(baseContent), string(currentContent), string(newContent), wf.SourceSpec, latestRef, verbose) + mergedContent, conflicts, err := MergeWorkflowContent(string(baseContent), string(currentContent), string(newContent), wf.SourceSpec, sourceFieldRef, verbose) if err != nil { updateLog.Printf("Merge failed for workflow %s: %v", wf.Name, err) return fmt.Errorf("failed to merge workflow content: %w", err) @@ -382,7 +469,7 @@ func updateWorkflow(wf *workflowWithSource, allowMajor, force, verbose bool, eng } // Update the source field in the new content with the new ref - newWithUpdatedSource, err := UpdateFieldInFrontmatter(string(newContent), "source", fmt.Sprintf("%s/%s@%s", sourceSpec.Repo, sourceSpec.Path, latestRef)) + newWithUpdatedSource, err := UpdateFieldInFrontmatter(string(newContent), "source", fmt.Sprintf("%s/%s@%s", sourceSpec.Repo, sourceSpec.Path, sourceFieldRef)) if err != nil { if verbose { fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to update source in new content: %v", err))) @@ -448,12 +535,12 @@ func updateWorkflow(wf *workflowWithSource, allowMajor, force, verbose bool, eng } if hasConflicts { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Updated %s from %s to %s with CONFLICTS - please review and resolve manually", wf.Name, currentRef, latestRef))) + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Updated %s from %s to %s with CONFLICTS - please review and resolve manually", wf.Name, shortRef(currentRef), shortRef(latestRef)))) return nil // Not an error, but user needs to resolve conflicts } updateLog.Printf("Successfully updated workflow %s from %s to %s", wf.Name, currentRef, latestRef) - fmt.Fprintln(os.Stderr, console.FormatSuccessMessage(fmt.Sprintf("Updated %s from %s to %s", wf.Name, currentRef, latestRef))) + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage(fmt.Sprintf("Updated %s from %s to %s", wf.Name, shortRef(currentRef), shortRef(latestRef)))) // Compile the updated workflow with refreshStopTime enabled updateLog.Printf("Compiling updated workflow: %s", wf.Name) @@ -464,3 +551,18 @@ func updateWorkflow(wf *workflowWithSource, allowMajor, force, verbose bool, eng return nil } + +// isBranchRef returns true when the ref is a branch name — i.e. it is +// neither a semantic-version tag nor a full commit SHA. +func isBranchRef(ref string) bool { + return !isSemanticVersionTag(ref) && !IsCommitSHA(ref) +} + +// shortRef abbreviates a ref for display. Commit SHAs are truncated to 7 characters; +// other refs (branch names, tags) are returned as-is. +func shortRef(ref string) string { + if IsCommitSHA(ref) { + return ref[:7] + } + return ref +} diff --git a/pkg/cli/upgrade_command.go b/pkg/cli/upgrade_command.go index c452ed4b0c9..07f41a38ac3 100644 --- a/pkg/cli/upgrade_command.go +++ b/pkg/cli/upgrade_command.go @@ -20,10 +20,15 @@ type UpgradeConfig struct { NoFix bool Push bool NoActions bool + Audit bool + JSON bool } // RunUpgrade runs the upgrade command with the given configuration func RunUpgrade(config UpgradeConfig) error { + if config.Audit { + return runDependencyAudit(config.Verbose, config.JSON) + } return runUpgradeCommand(config.Verbose, config.WorkflowDir, config.NoFix, false, config.Push, config.NoActions) } @@ -40,6 +45,15 @@ This command: 3. Updates GitHub Actions versions in .github/aw/actions-lock.json (unless --no-actions is set) 4. Compiles all workflows to generate lock files (like 'compile' command) +DEPENDENCY HEALTH AUDIT: +Use --audit to check dependency health without performing upgrades. This includes: +- Outdated Go dependencies with available updates +- Security advisories from GitHub Security Advisory API +- Dependency maturity analysis (v0.x vs stable versions) +- Comprehensive dependency health report + +The --audit flag skips the normal upgrade process. + The upgrade process ensures: - Dispatcher agent is current (.github/agents/agentic-workflows.agent.md) - All workflow prompts exist in .github/aw/ (create, update, debug, upgrade) @@ -55,7 +69,9 @@ Examples: ` + string(constants.CLIExtensionPrefix) + ` upgrade --no-fix # Update agent files only (skip codemods, actions, and compilation) ` + string(constants.CLIExtensionPrefix) + ` upgrade --no-actions # Skip updating GitHub Actions versions ` + string(constants.CLIExtensionPrefix) + ` upgrade --push # Upgrade and automatically commit/push changes - ` + string(constants.CLIExtensionPrefix) + ` upgrade --dir custom/workflows # Upgrade workflows in custom directory`, + ` + string(constants.CLIExtensionPrefix) + ` upgrade --dir custom/workflows # Upgrade workflows in custom directory + ` + string(constants.CLIExtensionPrefix) + ` upgrade --audit # Check dependency health without upgrading + ` + string(constants.CLIExtensionPrefix) + ` upgrade --audit --json # Output audit results in JSON format`, Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { verbose, _ := cmd.Flags().GetBool("verbose") @@ -63,6 +79,13 @@ Examples: noFix, _ := cmd.Flags().GetBool("no-fix") push, _ := cmd.Flags().GetBool("push") noActions, _ := cmd.Flags().GetBool("no-actions") + auditFlag, _ := cmd.Flags().GetBool("audit") + jsonOutput, _ := cmd.Flags().GetBool("json") + + // Handle audit mode + if auditFlag { + return runDependencyAudit(verbose, jsonOutput) + } return runUpgradeCommand(verbose, dir, noFix, false, push, noActions) }, @@ -72,6 +95,8 @@ Examples: cmd.Flags().Bool("no-fix", false, "Skip applying codemods, action updates, and compiling workflows (only update agent files)") cmd.Flags().Bool("no-actions", false, "Skip updating GitHub Actions versions") cmd.Flags().Bool("push", false, "Automatically commit and push changes after successful upgrade") + cmd.Flags().Bool("audit", false, "Check dependency health without performing upgrades") + addJSONFlag(cmd) // Register completions RegisterDirFlagCompletion(cmd, "dir") @@ -79,6 +104,25 @@ Examples: return cmd } +// runDependencyAudit performs a dependency health audit +func runDependencyAudit(verbose bool, jsonOutput bool) error { + upgradeLog.Print("Running dependency health audit") + + // Generate comprehensive report + report, err := GenerateDependencyReport(verbose) + if err != nil { + return fmt.Errorf("failed to generate dependency report: %w", err) + } + + // Display the report + if jsonOutput { + return DisplayDependencyReportJSON(report) + } + DisplayDependencyReport(report) + + return nil +} + // runUpgradeCommand executes the upgrade process func runUpgradeCommand(verbose bool, workflowDir string, noFix bool, noCompile bool, push bool, noActions bool) error { upgradeLog.Printf("Running upgrade command: verbose=%v, workflowDir=%s, noFix=%v, noCompile=%v, push=%v, noActions=%v",