From f05b2798a79a8c461f5522d2c4e82824f23bf569 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 5 Jan 2026 05:27:09 +0000 Subject: [PATCH 01/18] Initial plan From 5c89bfdb5ac8e1bac240f1f36338b7324e6c710d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 5 Jan 2026 05:42:25 +0000 Subject: [PATCH 02/18] Add --push flag to run command with transitive imports support Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- cmd/gh-aw/main.go | 7 +- pkg/cli/commands_test.go | 20 +- pkg/cli/context_cancellation_test.go | 6 +- pkg/cli/run_command.go | 34 +++- pkg/cli/run_push.go | 268 +++++++++++++++++++++++++ pkg/cli/run_push_test.go | 286 +++++++++++++++++++++++++++ 6 files changed, 602 insertions(+), 19 deletions(-) create mode 100644 pkg/cli/run_push.go create mode 100644 pkg/cli/run_push_test.go diff --git a/cmd/gh-aw/main.go b/cmd/gh-aw/main.go index 0a8ee0843ed..9a06d6aac2f 100644 --- a/cmd/gh-aw/main.go +++ b/cmd/gh-aw/main.go @@ -305,7 +305,8 @@ Examples: gh aw run daily-perf-improver --repeat 3 # Run 3 times total gh aw run daily-perf-improver --enable-if-needed # Enable if disabled, run, then restore state gh aw run daily-perf-improver --auto-merge-prs # Auto-merge any PRs created during execution - gh aw run daily-perf-improver -f name=value -f env=prod # Pass workflow inputs`, + gh aw run daily-perf-improver -f name=value -f env=prod # Pass workflow inputs + gh aw run daily-perf-improver --push # Commit and push workflow files before running`, Args: cobra.MinimumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { repeatCount, _ := cmd.Flags().GetInt("repeat") @@ -316,12 +317,13 @@ Examples: autoMergePRs, _ := cmd.Flags().GetBool("auto-merge-prs") pushSecrets, _ := cmd.Flags().GetBool("use-local-secrets") inputs, _ := cmd.Flags().GetStringArray("raw-field") + push, _ := cmd.Flags().GetBool("push") if err := validateEngine(engineOverride); err != nil { return err } - return cli.RunWorkflowsOnGitHub(cmd.Context(), args, repeatCount, enable, engineOverride, repoOverride, refOverride, autoMergePRs, pushSecrets, inputs, verboseFlag) + return cli.RunWorkflowsOnGitHub(cmd.Context(), args, repeatCount, enable, engineOverride, repoOverride, refOverride, autoMergePRs, pushSecrets, push, inputs, verboseFlag) }, } @@ -512,6 +514,7 @@ Use "` + string(constants.CLIExtensionPrefix) + ` help all" to show help for all runCmd.Flags().Bool("auto-merge-prs", false, "Auto-merge any pull requests created during the workflow execution") runCmd.Flags().Bool("use-local-secrets", false, "Use local environment API key secrets for workflow execution (pushes and cleans up secrets in repository)") runCmd.Flags().StringArrayP("raw-field", "F", []string{}, "Add a string parameter in key=value format (can be used multiple times)") + runCmd.Flags().Bool("push", false, "Commit and push workflow files (including transitive imports) before running") // Register completions for run command runCmd.ValidArgsFunction = cli.CompleteWorkflowNames cli.RegisterEngineFlagCompletion(runCmd) diff --git a/pkg/cli/commands_test.go b/pkg/cli/commands_test.go index 45c6550a0f9..d276e353db3 100644 --- a/pkg/cli/commands_test.go +++ b/pkg/cli/commands_test.go @@ -337,13 +337,13 @@ func TestDisableWorkflowsFailureScenarios(t *testing.T) { func TestRunWorkflowOnGitHub(t *testing.T) { // Test with empty workflow name - err := RunWorkflowOnGitHub(context.Background(), "", false, "", "", "", false, false, false, []string{}, false) + err := RunWorkflowOnGitHub(context.Background(), "", false, "", "", "", false, false, false, false, []string{}, false) if err == nil { t.Error("RunWorkflowOnGitHub should return error for empty workflow name") } // Test with nonexistent workflow (this will fail but gracefully) - err = RunWorkflowOnGitHub(context.Background(), "nonexistent-workflow", false, "", "", "", false, false, false, []string{}, false) + err = RunWorkflowOnGitHub(context.Background(), "nonexistent-workflow", false, "", "", "", false, false, false, false, []string{}, false) if err == nil { t.Error("RunWorkflowOnGitHub should return error for non-existent workflow") } @@ -351,25 +351,25 @@ func TestRunWorkflowOnGitHub(t *testing.T) { func TestRunWorkflowsOnGitHub(t *testing.T) { // Test with empty workflow list - err := RunWorkflowsOnGitHub(context.Background(), []string{}, 0, false, "", "", "", false, false, []string{}, false) + err := RunWorkflowsOnGitHub(context.Background(), []string{}, 0, false, "", "", "", false, false, false, []string{}, false) if err == nil { t.Error("RunWorkflowsOnGitHub should return error for empty workflow list") } // Test with workflow list containing empty name - err = RunWorkflowsOnGitHub(context.Background(), []string{"valid-workflow", ""}, 0, false, "", "", "", false, false, []string{}, false) + err = RunWorkflowsOnGitHub(context.Background(), []string{"valid-workflow", ""}, 0, false, "", "", "", false, false, false, []string{}, false) if err == nil { t.Error("RunWorkflowsOnGitHub should return error for workflow list containing empty name") } // Test with nonexistent workflows (this will fail but gracefully) - err = RunWorkflowsOnGitHub(context.Background(), []string{"nonexistent-workflow1", "nonexistent-workflow2"}, 0, false, "", "", "", false, false, []string{}, false) + err = RunWorkflowsOnGitHub(context.Background(), []string{"nonexistent-workflow1", "nonexistent-workflow2"}, 0, false, "", "", "", false, false, false, []string{}, false) if err == nil { t.Error("RunWorkflowsOnGitHub should return error for non-existent workflows") } // Test with negative repeat seconds (should work as 0) - err = RunWorkflowsOnGitHub(context.Background(), []string{"nonexistent-workflow"}, -1, false, "", "", "", false, false, []string{}, false) + err = RunWorkflowsOnGitHub(context.Background(), []string{"nonexistent-workflow"}, -1, false, "", "", "", false, false, false, []string{}, false) if err == nil { t.Error("RunWorkflowsOnGitHub should return error for non-existent workflow regardless of repeat value") } @@ -427,10 +427,10 @@ Test workflow for command existence.` {func() error { return EnableWorkflows("nonexistent") }, true, "EnableWorkflows"}, // Should now error when no workflows found to enable {func() error { return DisableWorkflows("nonexistent") }, true, "DisableWorkflows"}, // Should now also error when no workflows found to disable {func() error { - return RunWorkflowOnGitHub(context.Background(), "", false, "", "", "", false, false, false, []string{}, false) + return RunWorkflowOnGitHub(context.Background(), "", false, "", "", "", false, false, false, false, []string{}, false) }, true, "RunWorkflowOnGitHub"}, // Should error with empty workflow name {func() error { - return RunWorkflowsOnGitHub(context.Background(), []string{}, 0, false, "", "", "", false, false, []string{}, false) + return RunWorkflowsOnGitHub(context.Background(), []string{}, 0, false, "", "", "", false, false, false, []string{}, false) }, true, "RunWorkflowsOnGitHub"}, // Should error with empty workflow list } @@ -1078,13 +1078,13 @@ func TestCalculateTimeRemaining(t *testing.T) { func TestRunWorkflowOnGitHubWithEnable(t *testing.T) { // Test with enable flag enabled (should not error for basic validation) - err := RunWorkflowOnGitHub(context.Background(), "nonexistent-workflow", true, "", "", "", false, false, false, []string{}, false) + err := RunWorkflowOnGitHub(context.Background(), "nonexistent-workflow", true, "", "", "", false, false, false, false, []string{}, false) if err == nil { t.Error("RunWorkflowOnGitHub should return error for non-existent workflow even with enable flag") } // Test with empty workflow name and enable flag - err = RunWorkflowOnGitHub(context.Background(), "", true, "", "", "", false, false, false, []string{}, false) + err = RunWorkflowOnGitHub(context.Background(), "", true, "", "", "", false, false, false, false, []string{}, false) if err == nil { t.Error("RunWorkflowOnGitHub should return error for empty workflow name regardless of enable flag") } diff --git a/pkg/cli/context_cancellation_test.go b/pkg/cli/context_cancellation_test.go index 74290a6728a..b588bac07f2 100644 --- a/pkg/cli/context_cancellation_test.go +++ b/pkg/cli/context_cancellation_test.go @@ -15,7 +15,7 @@ func TestRunWorkflowOnGitHubWithCancellation(t *testing.T) { cancel() // Try to run a workflow with a cancelled context - err := RunWorkflowOnGitHub(ctx, "test-workflow", false, "", "", "", false, false, false, []string{}, false) + err := RunWorkflowOnGitHub(ctx, "test-workflow", false, "", "", "", false, false, false, false, []string{}, false) // Should return context.Canceled error assert.ErrorIs(t, err, context.Canceled, "Should return context.Canceled error when context is cancelled") @@ -28,7 +28,7 @@ func TestRunWorkflowsOnGitHubWithCancellation(t *testing.T) { cancel() // Try to run workflows with a cancelled context - err := RunWorkflowsOnGitHub(ctx, []string{"test-workflow"}, 0, false, "", "", "", false, false, []string{}, false) + err := RunWorkflowsOnGitHub(ctx, []string{"test-workflow"}, 0, false, "", "", "", false, false, false, []string{}, false) // Should return context.Canceled error assert.ErrorIs(t, err, context.Canceled, "Should return context.Canceled error when context is cancelled") @@ -96,7 +96,7 @@ func TestRunWorkflowsOnGitHubCancellationDuringExecution(t *testing.T) { // Try to run multiple workflows that would take a long time // This should fail validation before timeout, but if it gets past validation, // it should respect the context cancellation - err := RunWorkflowsOnGitHub(ctx, []string{"nonexistent-workflow-1", "nonexistent-workflow-2"}, 0, false, "", "", "", false, false, []string{}, false) + err := RunWorkflowsOnGitHub(ctx, []string{"nonexistent-workflow-1", "nonexistent-workflow-2"}, 0, false, "", "", "", false, false, false, []string{}, false) // Should return an error (either validation error or context error) assert.Error(t, err, "Should return an error") diff --git a/pkg/cli/run_command.go b/pkg/cli/run_command.go index 77d299d877f..ac21edaf7fa 100644 --- a/pkg/cli/run_command.go +++ b/pkg/cli/run_command.go @@ -22,8 +22,8 @@ import ( var runLog = logger.New("cli:run_command") // RunWorkflowOnGitHub runs an agentic workflow on GitHub Actions -func RunWorkflowOnGitHub(ctx context.Context, workflowIdOrName string, enable bool, engineOverride string, repoOverride string, refOverride string, autoMergePRs bool, pushSecrets bool, waitForCompletion bool, inputs []string, verbose bool) error { - runLog.Printf("Starting workflow run: workflow=%s, enable=%v, engineOverride=%s, repo=%s, ref=%s, wait=%v, inputs=%v", workflowIdOrName, enable, engineOverride, repoOverride, refOverride, waitForCompletion, inputs) +func RunWorkflowOnGitHub(ctx context.Context, workflowIdOrName string, enable bool, engineOverride string, repoOverride string, refOverride string, autoMergePRs bool, pushSecrets bool, push bool, waitForCompletion bool, inputs []string, verbose bool) error { + runLog.Printf("Starting workflow run: workflow=%s, enable=%v, engineOverride=%s, repo=%s, ref=%s, push=%v, wait=%v, inputs=%v", workflowIdOrName, enable, engineOverride, repoOverride, refOverride, push, waitForCompletion, inputs) // Check context cancellation at the start select { @@ -225,6 +225,32 @@ func RunWorkflowOnGitHub(ctx context.Context, workflowIdOrName string, enable bo fmt.Printf("Using lock file: %s\n", lockFileName) } + // Handle --push flag: commit and push workflow files before running + if push { + // Only valid for local workflows + if repoOverride != "" { + return fmt.Errorf("--push flag is only supported for local workflows, not remote repositories") + } + + if verbose { + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Collecting workflow files for push...")) + } + + // Collect the workflow .md file, .lock.yml file, and transitive imports + workflowMarkdownPath := strings.TrimSuffix(lockFilePath, ".lock.yml") + ".md" + files, err := collectWorkflowFiles(workflowMarkdownPath, verbose) + if err != nil { + return fmt.Errorf("failed to collect workflow files: %w", err) + } + + // Commit and push the files + if err := pushWorkflowFiles(workflowIdOrName, files, verbose); err != nil { + return fmt.Errorf("failed to push workflow files: %w", err) + } + + fmt.Println(console.FormatSuccessMessage(fmt.Sprintf("Successfully pushed %d file(s) for workflow %s", len(files), workflowIdOrName))) + } + // Handle secret pushing if requested var secretTracker *TrialSecretTracker if pushSecrets { @@ -472,7 +498,7 @@ func RunWorkflowOnGitHub(ctx context.Context, workflowIdOrName string, enable bo } // RunWorkflowsOnGitHub runs multiple agentic workflows on GitHub Actions, optionally repeating a specified number of times -func RunWorkflowsOnGitHub(ctx context.Context, workflowNames []string, repeatCount int, enable bool, engineOverride string, repoOverride string, refOverride string, autoMergePRs bool, pushSecrets bool, inputs []string, verbose bool) error { +func RunWorkflowsOnGitHub(ctx context.Context, workflowNames []string, repeatCount int, enable bool, engineOverride string, repoOverride string, refOverride string, autoMergePRs bool, pushSecrets bool, push bool, inputs []string, verbose bool) error { if len(workflowNames) == 0 { return fmt.Errorf("at least one workflow name or ID is required") } @@ -535,7 +561,7 @@ func RunWorkflowsOnGitHub(ctx context.Context, workflowNames []string, repeatCou fmt.Println(console.FormatProgressMessage(fmt.Sprintf("Running workflow %d/%d: %s", i+1, len(workflowNames), workflowName))) } - if err := RunWorkflowOnGitHub(ctx, workflowName, enable, engineOverride, repoOverride, refOverride, autoMergePRs, pushSecrets, waitForCompletion, inputs, verbose); err != nil { + if err := RunWorkflowOnGitHub(ctx, workflowName, enable, engineOverride, repoOverride, refOverride, autoMergePRs, pushSecrets, push, waitForCompletion, inputs, verbose); err != nil { return fmt.Errorf("failed to run workflow '%s': %w", workflowName, err) } diff --git a/pkg/cli/run_push.go b/pkg/cli/run_push.go new file mode 100644 index 00000000000..25896303a7e --- /dev/null +++ b/pkg/cli/run_push.go @@ -0,0 +1,268 @@ +package cli + +import ( + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + + "github.com/githubnext/gh-aw/pkg/console" + "github.com/githubnext/gh-aw/pkg/logger" + "github.com/githubnext/gh-aw/pkg/parser" +) + +var runPushLog = logger.New("cli:run_push") + +// collectWorkflowFiles collects the workflow .md file, its corresponding .lock.yml file, +// and the transitive closure of all imported files +func collectWorkflowFiles(workflowPath string, verbose bool) ([]string, error) { + runPushLog.Printf("Collecting files for workflow: %s", workflowPath) + + files := make(map[string]bool) // Use map to avoid duplicates + visited := make(map[string]bool) + + // Get absolute path for the workflow + absWorkflowPath, err := filepath.Abs(workflowPath) + if err != nil { + return nil, fmt.Errorf("failed to get absolute path for workflow: %w", err) + } + + // Add the workflow .md file + files[absWorkflowPath] = true + runPushLog.Printf("Added workflow file: %s", absWorkflowPath) + + // Add the corresponding .lock.yml file + lockFilePath := strings.TrimSuffix(absWorkflowPath, ".md") + ".lock.yml" + if _, err := os.Stat(lockFilePath); err == nil { + files[lockFilePath] = true + runPushLog.Printf("Added lock file: %s", lockFilePath) + } else if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Lock file not found: %s", lockFilePath))) + } + + // Collect transitive closure of imported files + if err := collectImports(absWorkflowPath, files, visited, verbose); err != nil { + return nil, fmt.Errorf("failed to collect imports: %w", err) + } + + // Convert map to slice + result := make([]string, 0, len(files)) + for file := range files { + result = append(result, file) + } + + runPushLog.Printf("Collected %d files total", len(result)) + return result, nil +} + +// collectImports recursively collects all imported files (transitive closure) +func collectImports(workflowPath string, files map[string]bool, visited map[string]bool, verbose bool) error { + // Avoid processing the same file multiple times + if visited[workflowPath] { + return nil + } + visited[workflowPath] = true + + runPushLog.Printf("Processing imports for: %s", workflowPath) + + // Read the workflow file + content, err := os.ReadFile(workflowPath) + if err != nil { + return fmt.Errorf("failed to read workflow file %s: %w", workflowPath, err) + } + + // Extract frontmatter to get imports field + result, err := parser.ExtractFrontmatterFromContent(string(content)) + if err != nil { + // No frontmatter is okay - might be a simple file + runPushLog.Printf("No frontmatter in %s, skipping imports extraction", workflowPath) + return nil + } + + // Get imports from frontmatter + importsField, exists := result.Frontmatter["imports"] + if !exists { + runPushLog.Printf("No imports field in %s", workflowPath) + return nil + } + + // Parse imports field - can be array of strings or objects with path + workflowDir := filepath.Dir(workflowPath) + var imports []string + + switch v := importsField.(type) { + case []any: + for _, item := range v { + switch importItem := item.(type) { + case string: + // Simple string import + imports = append(imports, importItem) + case map[string]any: + // Object import with path field + if pathValue, hasPath := importItem["path"]; hasPath { + if pathStr, ok := pathValue.(string); ok { + imports = append(imports, pathStr) + } + } + } + } + case []string: + imports = v + } + + runPushLog.Printf("Found %d imports in %s", len(imports), workflowPath) + + // Process each import + for _, importPath := range imports { + // Resolve the import path + resolvedPath := resolveImportPathLocal(importPath, workflowDir) + if resolvedPath == "" { + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Could not resolve import: %s", importPath))) + } + continue + } + + // Get absolute path + var absImportPath string + if filepath.IsAbs(resolvedPath) { + absImportPath = resolvedPath + } else { + absImportPath = filepath.Join(workflowDir, resolvedPath) + } + + // Check if file exists + if _, err := os.Stat(absImportPath); err != nil { + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Import file not found: %s", absImportPath))) + } + continue + } + + // Add the import file + files[absImportPath] = true + runPushLog.Printf("Added import file: %s", absImportPath) + + // Recursively collect imports from this file + if err := collectImports(absImportPath, files, visited, verbose); err != nil { + return err + } + } + + return nil +} + +// resolveImportPathLocal is a local version of resolveImportPath for push functionality +// This is needed to avoid circular dependencies with imports.go +func resolveImportPathLocal(importPath, baseDir string) string { + // Handle section references (file.md#Section) - strip the section part + if strings.Contains(importPath, "#") { + parts := strings.SplitN(importPath, "#", 2) + importPath = parts[0] + } + + // Skip workflowspec format imports (owner/repo/path@sha) + if strings.Contains(importPath, "@") || isWorkflowSpecFormatLocal(importPath) { + runPushLog.Printf("Skipping workflowspec format import: %s", importPath) + return "" + } + + // If the import path is absolute (starts with /), use it relative to repo root + if strings.HasPrefix(importPath, "/") { + // Find git root + gitRoot, err := findGitRoot() + if err != nil { + return "" + } + return filepath.Join(gitRoot, strings.TrimPrefix(importPath, "/")) + } + + // Otherwise, resolve relative to the workflow file's directory + return filepath.Join(baseDir, importPath) +} + +// isWorkflowSpecFormatLocal is a local version of isWorkflowSpecFormat for push functionality +// This is duplicated from imports.go to avoid circular dependencies +func isWorkflowSpecFormatLocal(path string) bool { + // Check if it contains @ (ref separator) or looks like owner/repo/path + if strings.Contains(path, "@") { + return true + } + + // Remove section reference if present + cleanPath := path + if idx := strings.Index(path, "#"); idx != -1 { + cleanPath = path[:idx] + } + + // Check if it has at least 3 parts and doesn't start with . or / + parts := strings.Split(cleanPath, "/") + if len(parts) >= 3 && !strings.HasPrefix(cleanPath, ".") && !strings.HasPrefix(cleanPath, "/") { + return true + } + + return false +} + +// pushWorkflowFiles commits and pushes the workflow files to the repository +func pushWorkflowFiles(workflowName string, files []string, verbose bool) error { + runPushLog.Printf("Pushing %d files for workflow: %s", len(files), workflowName) + + if verbose { + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Staging %d files for commit", len(files)))) + for _, file := range files { + fmt.Fprintf(os.Stderr, " - %s\n", file) + } + } + + // Stage all files + gitArgs := append([]string{"add"}, files...) + cmd := exec.Command("git", gitArgs...) + if output, err := cmd.CombinedOutput(); err != nil { + runPushLog.Printf("Failed to stage files: %v, output: %s", err, string(output)) + return fmt.Errorf("failed to stage files: %w\nOutput: %s", err, string(output)) + } + + if verbose { + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Files staged successfully")) + } + + // Create commit message + commitMessage := fmt.Sprintf("Updated agentic workflow %s", workflowName) + runPushLog.Printf("Creating commit with message: %s", commitMessage) + + // Commit the changes + cmd = exec.Command("git", "commit", "-m", commitMessage) + if output, err := cmd.CombinedOutput(); err != nil { + // Check if there are no changes to commit + if strings.Contains(string(output), "nothing to commit") { + if verbose { + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("No changes to commit")) + } + runPushLog.Print("No changes to commit") + return nil + } + runPushLog.Printf("Failed to commit: %v, output: %s", err, string(output)) + return fmt.Errorf("failed to commit changes: %w\nOutput: %s", err, string(output)) + } + + if verbose { + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Changes committed successfully")) + } + + // Push the changes + runPushLog.Print("Pushing changes to remote") + cmd = exec.Command("git", "push") + if output, err := cmd.CombinedOutput(); err != nil { + runPushLog.Printf("Failed to push: %v, output: %s", err, string(output)) + return fmt.Errorf("failed to push changes: %w\nOutput: %s", err, string(output)) + } + + if verbose { + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Changes pushed to remote")) + } + + runPushLog.Print("Push completed successfully") + return nil +} diff --git a/pkg/cli/run_push_test.go b/pkg/cli/run_push_test.go new file mode 100644 index 00000000000..e511325ee92 --- /dev/null +++ b/pkg/cli/run_push_test.go @@ -0,0 +1,286 @@ +package cli + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestCollectWorkflowFiles_SimpleWorkflow(t *testing.T) { + // Create a temporary directory for testing + tmpDir := t.TempDir() + + // Create a simple workflow file + workflowPath := filepath.Join(tmpDir, "test-workflow.md") + workflowContent := `--- +name: Test Workflow +on: workflow_dispatch +--- +# Test Workflow +This is a test workflow. +` + err := os.WriteFile(workflowPath, []byte(workflowContent), 0644) + require.NoError(t, err) + + // Create the corresponding lock file + lockFilePath := filepath.Join(tmpDir, "test-workflow.lock.yml") + lockContent := `name: Test Workflow +on: workflow_dispatch +` + err = os.WriteFile(lockFilePath, []byte(lockContent), 0644) + require.NoError(t, err) + + // Test collecting files + files, err := collectWorkflowFiles(workflowPath, false) + require.NoError(t, err) + assert.Len(t, files, 2, "Should collect workflow .md and .lock.yml files") + + // Check that both files are in the result + fileSet := make(map[string]bool) + for _, file := range files { + fileSet[file] = true + } + assert.True(t, fileSet[workflowPath], "Should include workflow .md file") + assert.True(t, fileSet[lockFilePath], "Should include lock .yml file") +} + +func TestCollectWorkflowFiles_WithImports(t *testing.T) { + // Create a temporary directory for testing + tmpDir := t.TempDir() + + // Create a shared file + sharedPath := filepath.Join(tmpDir, "shared.md") + sharedContent := `# Shared Content +This is shared content. +` + err := os.WriteFile(sharedPath, []byte(sharedContent), 0644) + require.NoError(t, err) + + // Create a workflow file that imports the shared file + workflowPath := filepath.Join(tmpDir, "test-workflow.md") + workflowContent := `--- +name: Test Workflow +on: workflow_dispatch +imports: + - shared.md +--- +# Test Workflow +This workflow imports shared content. +` + err = os.WriteFile(workflowPath, []byte(workflowContent), 0644) + require.NoError(t, err) + + // Create the corresponding lock file + lockFilePath := filepath.Join(tmpDir, "test-workflow.lock.yml") + lockContent := `name: Test Workflow +on: workflow_dispatch +` + err = os.WriteFile(lockFilePath, []byte(lockContent), 0644) + require.NoError(t, err) + + // Test collecting files + files, err := collectWorkflowFiles(workflowPath, false) + require.NoError(t, err) + assert.Len(t, files, 3, "Should collect workflow, lock, and imported files") + + // Check that all files are in the result + fileSet := make(map[string]bool) + for _, file := range files { + fileSet[file] = true + } + assert.True(t, fileSet[workflowPath], "Should include workflow .md file") + assert.True(t, fileSet[lockFilePath], "Should include lock .yml file") + assert.True(t, fileSet[sharedPath], "Should include imported shared.md file") +} + +func TestCollectWorkflowFiles_TransitiveImports(t *testing.T) { + // Create a temporary directory for testing + tmpDir := t.TempDir() + + // Create base shared file + baseSharedPath := filepath.Join(tmpDir, "base-shared.md") + baseSharedContent := `# Base Shared Content +This is base shared content. +` + err := os.WriteFile(baseSharedPath, []byte(baseSharedContent), 0644) + require.NoError(t, err) + + // Create intermediate shared file that imports base + intermediateSharedPath := filepath.Join(tmpDir, "intermediate-shared.md") + intermediateSharedContent := `--- +imports: + - base-shared.md +--- +# Intermediate Shared Content +This imports base shared. +` + err = os.WriteFile(intermediateSharedPath, []byte(intermediateSharedContent), 0644) + require.NoError(t, err) + + // Create a workflow file that imports the intermediate file + workflowPath := filepath.Join(tmpDir, "test-workflow.md") + workflowContent := `--- +name: Test Workflow +on: workflow_dispatch +imports: + - intermediate-shared.md +--- +# Test Workflow +This workflow imports intermediate shared content. +` + err = os.WriteFile(workflowPath, []byte(workflowContent), 0644) + require.NoError(t, err) + + // Create the corresponding lock file + lockFilePath := filepath.Join(tmpDir, "test-workflow.lock.yml") + lockContent := `name: Test Workflow +on: workflow_dispatch +` + err = os.WriteFile(lockFilePath, []byte(lockContent), 0644) + require.NoError(t, err) + + // Test collecting files + files, err := collectWorkflowFiles(workflowPath, false) + require.NoError(t, err) + assert.Len(t, files, 4, "Should collect workflow, lock, and all transitive imports") + + // Check that all files are in the result + fileSet := make(map[string]bool) + for _, file := range files { + fileSet[file] = true + } + assert.True(t, fileSet[workflowPath], "Should include workflow .md file") + assert.True(t, fileSet[lockFilePath], "Should include lock .yml file") + assert.True(t, fileSet[intermediateSharedPath], "Should include intermediate-shared.md file") + assert.True(t, fileSet[baseSharedPath], "Should include base-shared.md file") +} + +func TestCollectWorkflowFiles_NoLockFile(t *testing.T) { + // Create a temporary directory for testing + tmpDir := t.TempDir() + + // Create a simple workflow file without a lock file + workflowPath := filepath.Join(tmpDir, "test-workflow.md") + workflowContent := `--- +name: Test Workflow +on: workflow_dispatch +--- +# Test Workflow +This is a test workflow without a lock file. +` + err := os.WriteFile(workflowPath, []byte(workflowContent), 0644) + require.NoError(t, err) + + // Test collecting files (should not fail even without lock file) + files, err := collectWorkflowFiles(workflowPath, false) + require.NoError(t, err) + assert.Len(t, files, 1, "Should collect only workflow .md file when lock file is missing") + + // Check that workflow file is in the result + fileSet := make(map[string]bool) + for _, file := range files { + fileSet[file] = true + } + assert.True(t, fileSet[workflowPath], "Should include workflow .md file") +} + +func TestIsWorkflowSpecFormatLocal(t *testing.T) { + tests := []struct { + name string + path string + expected bool + }{ + { + name: "workflowspec with SHA", + path: "owner/repo/path/file.md@abc123", + expected: true, + }, + { + name: "workflowspec without SHA", + path: "owner/repo/path/file.md", + expected: true, + }, + { + name: "relative path with ./", + path: "./shared/file.md", + expected: false, + }, + { + name: "relative path without ./", + path: "shared/file.md", + expected: false, + }, + { + name: "absolute path", + path: "/shared/file.md", + expected: false, + }, + { + name: "workflowspec with section", + path: "owner/repo/path/file.md#section", + expected: true, + }, + { + name: "simple filename", + path: "file.md", + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := isWorkflowSpecFormatLocal(tt.path) + assert.Equal(t, tt.expected, result, "isWorkflowSpecFormatLocal(%q) = %v, want %v", tt.path, result, tt.expected) + }) + } +} + +func TestResolveImportPathLocal(t *testing.T) { + // Create a temporary directory for testing + tmpDir := t.TempDir() + baseDir := filepath.Join(tmpDir, "workflows") + err := os.MkdirAll(baseDir, 0755) + require.NoError(t, err) + + tests := []struct { + name string + importPath string + baseDir string + expected string + }{ + { + name: "relative path", + importPath: "shared/file.md", + baseDir: baseDir, + expected: filepath.Join(baseDir, "shared/file.md"), + }, + { + name: "path with section", + importPath: "shared/file.md#section", + baseDir: baseDir, + expected: filepath.Join(baseDir, "shared/file.md"), + }, + { + name: "workflowspec format with @", + importPath: "owner/repo/path/file.md@abc123", + baseDir: baseDir, + expected: "", + }, + { + name: "workflowspec format without @", + importPath: "owner/repo/path/file.md", + baseDir: baseDir, + expected: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := resolveImportPathLocal(tt.importPath, tt.baseDir) + assert.Equal(t, tt.expected, result, "resolveImportPathLocal(%q, %q) = %v, want %v", tt.importPath, tt.baseDir, result, tt.expected) + }) + } +} From 4fe52fc1bd88a1726e6b71a35b3d831917dde2bc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 5 Jan 2026 05:45:37 +0000 Subject: [PATCH 03/18] Complete implementation and testing of --push flag Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/run_push_test.go | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/pkg/cli/run_push_test.go b/pkg/cli/run_push_test.go index e511325ee92..8036e5a35c8 100644 --- a/pkg/cli/run_push_test.go +++ b/pkg/cli/run_push_test.go @@ -246,34 +246,34 @@ func TestResolveImportPathLocal(t *testing.T) { require.NoError(t, err) tests := []struct { - name string - importPath string - baseDir string - expected string + name string + importPath string + baseDir string + expected string }{ { - name: "relative path", - importPath: "shared/file.md", - baseDir: baseDir, - expected: filepath.Join(baseDir, "shared/file.md"), + name: "relative path", + importPath: "shared/file.md", + baseDir: baseDir, + expected: filepath.Join(baseDir, "shared/file.md"), }, { - name: "path with section", - importPath: "shared/file.md#section", - baseDir: baseDir, - expected: filepath.Join(baseDir, "shared/file.md"), + name: "path with section", + importPath: "shared/file.md#section", + baseDir: baseDir, + expected: filepath.Join(baseDir, "shared/file.md"), }, { - name: "workflowspec format with @", - importPath: "owner/repo/path/file.md@abc123", - baseDir: baseDir, - expected: "", + name: "workflowspec format with @", + importPath: "owner/repo/path/file.md@abc123", + baseDir: baseDir, + expected: "", }, { - name: "workflowspec format without @", - importPath: "owner/repo/path/file.md", - baseDir: baseDir, - expected: "", + name: "workflowspec format without @", + importPath: "owner/repo/path/file.md", + baseDir: baseDir, + expected: "", }, } From 30608fb0d83e48c6d62d4857a0a9b9ebfebcdd23 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 5 Jan 2026 08:09:29 +0000 Subject: [PATCH 04/18] Add auto-recompilation when lock file is outdated Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .github/workflows/test-workflow.md | 8 ++++ pkg/cli/run_push.go | 67 +++++++++++++++++++++++++++++- pkg/cli/run_push_test.go | 53 +++++++++++++++++++++-- 3 files changed, 123 insertions(+), 5 deletions(-) create mode 100644 .github/workflows/test-workflow.md diff --git a/.github/workflows/test-workflow.md b/.github/workflows/test-workflow.md new file mode 100644 index 00000000000..7cf6cce3c1d --- /dev/null +++ b/.github/workflows/test-workflow.md @@ -0,0 +1,8 @@ +--- +name: Test Workflow (Modified) +on: workflow_dispatch +permissions: + contents: read +--- +# Test Workflow +This is a MODIFIED test workflow. diff --git a/pkg/cli/run_push.go b/pkg/cli/run_push.go index 25896303a7e..0a8a255077e 100644 --- a/pkg/cli/run_push.go +++ b/pkg/cli/run_push.go @@ -1,6 +1,7 @@ package cli import ( + "context" "fmt" "os" "os/exec" @@ -32,13 +33,46 @@ func collectWorkflowFiles(workflowPath string, verbose bool) ([]string, error) { files[absWorkflowPath] = true runPushLog.Printf("Added workflow file: %s", absWorkflowPath) - // Add the corresponding .lock.yml file + // Check if lock file needs recompilation lockFilePath := strings.TrimSuffix(absWorkflowPath, ".md") + ".lock.yml" + needsRecompile := false + + if lockStat, err := os.Stat(lockFilePath); err == nil { + // Lock file exists - check if it's outdated + if mdStat, err := os.Stat(absWorkflowPath); err == nil { + if mdStat.ModTime().After(lockStat.ModTime()) { + needsRecompile = true + runPushLog.Printf("Lock file is outdated (md: %v, lock: %v)", mdStat.ModTime(), lockStat.ModTime()) + if verbose { + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Detected outdated lock file, recompiling workflow...")) + } + } + } + } else if os.IsNotExist(err) { + // Lock file doesn't exist - needs compilation + needsRecompile = true + runPushLog.Printf("Lock file not found: %s", lockFilePath) + if verbose { + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Lock file not found, compiling workflow...")) + } + } + + // Recompile if needed + if needsRecompile { + if err := recompileWorkflow(absWorkflowPath, verbose); err != nil { + return nil, fmt.Errorf("failed to recompile workflow: %w", err) + } + if verbose { + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Workflow compiled successfully")) + } + } + + // Add the corresponding .lock.yml file if _, err := os.Stat(lockFilePath); err == nil { files[lockFilePath] = true runPushLog.Printf("Added lock file: %s", lockFilePath) } else if verbose { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Lock file not found: %s", lockFilePath))) + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Lock file not found after compilation: %s", lockFilePath))) } // Collect transitive closure of imported files @@ -56,6 +90,35 @@ func collectWorkflowFiles(workflowPath string, verbose bool) ([]string, error) { return result, nil } +// recompileWorkflow compiles a workflow using CompileWorkflows +func recompileWorkflow(workflowPath string, verbose bool) error { + runPushLog.Printf("Recompiling workflow: %s", workflowPath) + + config := CompileConfig{ + MarkdownFiles: []string{workflowPath}, + Verbose: verbose, + EngineOverride: "", + Validate: true, + Watch: false, + WorkflowDir: "", + SkipInstructions: false, + NoEmit: false, + Purge: false, + TrialMode: false, + TrialLogicalRepoSlug: "", + Strict: false, + } + + // Use background context for compilation + ctx := context.Background() + if _, err := CompileWorkflows(ctx, config); err != nil { + return fmt.Errorf("compilation failed: %w", err) + } + + runPushLog.Printf("Successfully recompiled workflow: %s", workflowPath) + return nil +} + // collectImports recursively collects all imported files (transitive closure) func collectImports(workflowPath string, files map[string]bool, visited map[string]bool, verbose bool) error { // Avoid processing the same file multiple times diff --git a/pkg/cli/run_push_test.go b/pkg/cli/run_push_test.go index 8036e5a35c8..130db2ce587 100644 --- a/pkg/cli/run_push_test.go +++ b/pkg/cli/run_push_test.go @@ -3,7 +3,9 @@ package cli import ( "os" "path/filepath" + "strings" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -174,17 +176,20 @@ This is a test workflow without a lock file. err := os.WriteFile(workflowPath, []byte(workflowContent), 0644) require.NoError(t, err) - // Test collecting files (should not fail even without lock file) + // Test collecting files - should now compile the workflow and create lock file files, err := collectWorkflowFiles(workflowPath, false) require.NoError(t, err) - assert.Len(t, files, 1, "Should collect only workflow .md file when lock file is missing") + assert.Len(t, files, 2, "Should collect workflow .md file and auto-generate lock file") - // Check that workflow file is in the result + // Check that both workflow file and lock file are in the result fileSet := make(map[string]bool) for _, file := range files { fileSet[file] = true } assert.True(t, fileSet[workflowPath], "Should include workflow .md file") + + lockFilePath := strings.TrimSuffix(workflowPath, ".md") + ".lock.yml" + assert.True(t, fileSet[lockFilePath], "Should include auto-generated lock .yml file") } func TestIsWorkflowSpecFormatLocal(t *testing.T) { @@ -284,3 +289,45 @@ func TestResolveImportPathLocal(t *testing.T) { }) } } + +func TestCollectWorkflowFiles_WithOutdatedLockFile(t *testing.T) { +// Create a temporary directory for testing +tmpDir := t.TempDir() + +// Create a workflow file +workflowPath := filepath.Join(tmpDir, "test-workflow.md") +workflowContent := `--- +name: Test Workflow +on: workflow_dispatch +--- +# Test Workflow +This is a test workflow. +` +err := os.WriteFile(workflowPath, []byte(workflowContent), 0644) +require.NoError(t, err) + +// Create an old lock file (simulate outdated) +lockFilePath := filepath.Join(tmpDir, "test-workflow.lock.yml") +lockContent := `name: Test Workflow +on: workflow_dispatch +` +err = os.WriteFile(lockFilePath, []byte(lockContent), 0644) +require.NoError(t, err) + +// Make the workflow file newer by sleeping and touching it +time.Sleep(100 * time.Millisecond) +currentTime := time.Now() +err = os.Chtimes(workflowPath, currentTime, currentTime) +require.NoError(t, err) + +// Verify the lock file is older +mdStat, err := os.Stat(workflowPath) +require.NoError(t, err) +lockStat, err := os.Stat(lockFilePath) +require.NoError(t, err) +assert.True(t, mdStat.ModTime().After(lockStat.ModTime()), "Workflow file should be newer than lock file") + +// Note: We can't actually test recompilation here without a full compilation setup, +// but we can verify the detection logic works +// The actual compilation would happen in an integration test +} From 97c409360dc89a2c0a9403a041e7461b0fe8a466 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 5 Jan 2026 08:10:58 +0000 Subject: [PATCH 05/18] Remove accidentally committed test file Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .github/workflows/test-workflow.md | 8 -------- 1 file changed, 8 deletions(-) delete mode 100644 .github/workflows/test-workflow.md diff --git a/.github/workflows/test-workflow.md b/.github/workflows/test-workflow.md deleted file mode 100644 index 7cf6cce3c1d..00000000000 --- a/.github/workflows/test-workflow.md +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: Test Workflow (Modified) -on: workflow_dispatch -permissions: - contents: read ---- -# Test Workflow -This is a MODIFIED test workflow. From 7fb10cdba2d43bca9fa92158c7eef4b91e0bbe7d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 5 Jan 2026 08:46:46 +0000 Subject: [PATCH 06/18] Add user confirmation prompt before commit and push Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/run_push.go | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/pkg/cli/run_push.go b/pkg/cli/run_push.go index 0a8a255077e..fa82848017d 100644 --- a/pkg/cli/run_push.go +++ b/pkg/cli/run_push.go @@ -295,6 +295,31 @@ func pushWorkflowFiles(workflowName string, files []string, verbose bool) error commitMessage := fmt.Sprintf("Updated agentic workflow %s", workflowName) runPushLog.Printf("Creating commit with message: %s", commitMessage) + // Show what will be committed and ask for confirmation + fmt.Fprintln(os.Stderr, "") + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Ready to commit and push the following files:")) + for _, file := range files { + fmt.Fprintf(os.Stderr, " - %s\n", file) + } + fmt.Fprintln(os.Stderr, "") + fmt.Fprintf(os.Stderr, console.FormatInfoMessage("Commit message: %s\n"), commitMessage) + fmt.Fprintln(os.Stderr, "") + + // Ask for confirmation + fmt.Fprint(os.Stderr, console.FormatPromptMessage("Do you want to commit and push these changes? [y/N]: ")) + + var response string + _, err := fmt.Scanln(&response) + if err != nil { + response = "n" // Default to no on error + } + + response = strings.ToLower(strings.TrimSpace(response)) + if response != "y" && response != "yes" { + runPushLog.Print("Push cancelled by user") + return fmt.Errorf("push cancelled by user") + } + // Commit the changes cmd = exec.Command("git", "commit", "-m", commitMessage) if output, err := cmd.CombinedOutput(); err != nil { From bd74addd4e65f2732ea628f0280beac9b3792c8d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 5 Jan 2026 09:14:44 +0000 Subject: [PATCH 07/18] Replace manual prompt with Bubble Tea confirmation dialog Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/run_push.go | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/pkg/cli/run_push.go b/pkg/cli/run_push.go index fa82848017d..1fa650be019 100644 --- a/pkg/cli/run_push.go +++ b/pkg/cli/run_push.go @@ -8,6 +8,7 @@ import ( "path/filepath" "strings" + "github.com/charmbracelet/huh" "github.com/githubnext/gh-aw/pkg/console" "github.com/githubnext/gh-aw/pkg/logger" "github.com/githubnext/gh-aw/pkg/parser" @@ -295,7 +296,7 @@ func pushWorkflowFiles(workflowName string, files []string, verbose bool) error commitMessage := fmt.Sprintf("Updated agentic workflow %s", workflowName) runPushLog.Printf("Creating commit with message: %s", commitMessage) - // Show what will be committed and ask for confirmation + // Show what will be committed and ask for confirmation using huh fmt.Fprintln(os.Stderr, "") fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Ready to commit and push the following files:")) for _, file := range files { @@ -305,17 +306,24 @@ func pushWorkflowFiles(workflowName string, files []string, verbose bool) error fmt.Fprintf(os.Stderr, console.FormatInfoMessage("Commit message: %s\n"), commitMessage) fmt.Fprintln(os.Stderr, "") - // Ask for confirmation - fmt.Fprint(os.Stderr, console.FormatPromptMessage("Do you want to commit and push these changes? [y/N]: ")) - - var response string - _, err := fmt.Scanln(&response) - if err != nil { - response = "n" // Default to no on error + // Ask for confirmation using huh + var confirmed bool + confirmForm := huh.NewForm( + huh.NewGroup( + huh.NewConfirm(). + Title("Do you want to commit and push these changes?"). + Affirmative("Yes, commit and push"). + Negative("No, cancel"). + Value(&confirmed), + ), + ).WithAccessible(isAccessibleMode()) + + if err := confirmForm.Run(); err != nil { + runPushLog.Printf("Confirmation failed: %v", err) + return fmt.Errorf("confirmation failed: %w", err) } - response = strings.ToLower(strings.TrimSpace(response)) - if response != "y" && response != "yes" { + if !confirmed { runPushLog.Print("Push cancelled by user") return fmt.Errorf("push cancelled by user") } From cb1afbd0e96980e8fa7ad071279bdb67db354505 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 5 Jan 2026 09:30:12 +0000 Subject: [PATCH 08/18] Move confirmation dialog to console package helper Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/run_push.go | 23 ++++------ pkg/console/confirm.go | 36 +++++++++++++++ pkg/console/confirm_test.go | 92 +++++++++++++++++++++++++++++++++++++ 3 files changed, 136 insertions(+), 15 deletions(-) create mode 100644 pkg/console/confirm.go create mode 100644 pkg/console/confirm_test.go diff --git a/pkg/cli/run_push.go b/pkg/cli/run_push.go index 1fa650be019..da85f77a47d 100644 --- a/pkg/cli/run_push.go +++ b/pkg/cli/run_push.go @@ -8,7 +8,6 @@ import ( "path/filepath" "strings" - "github.com/charmbracelet/huh" "github.com/githubnext/gh-aw/pkg/console" "github.com/githubnext/gh-aw/pkg/logger" "github.com/githubnext/gh-aw/pkg/parser" @@ -296,7 +295,7 @@ func pushWorkflowFiles(workflowName string, files []string, verbose bool) error commitMessage := fmt.Sprintf("Updated agentic workflow %s", workflowName) runPushLog.Printf("Creating commit with message: %s", commitMessage) - // Show what will be committed and ask for confirmation using huh + // Show what will be committed and ask for confirmation using console helper fmt.Fprintln(os.Stderr, "") fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Ready to commit and push the following files:")) for _, file := range files { @@ -306,19 +305,13 @@ func pushWorkflowFiles(workflowName string, files []string, verbose bool) error fmt.Fprintf(os.Stderr, console.FormatInfoMessage("Commit message: %s\n"), commitMessage) fmt.Fprintln(os.Stderr, "") - // Ask for confirmation using huh - var confirmed bool - confirmForm := huh.NewForm( - huh.NewGroup( - huh.NewConfirm(). - Title("Do you want to commit and push these changes?"). - Affirmative("Yes, commit and push"). - Negative("No, cancel"). - Value(&confirmed), - ), - ).WithAccessible(isAccessibleMode()) - - if err := confirmForm.Run(); err != nil { + // Ask for confirmation using console helper + confirmed, err := console.ConfirmAction( + "Do you want to commit and push these changes?", + "Yes, commit and push", + "No, cancel", + ) + if err != nil { runPushLog.Printf("Confirmation failed: %v", err) return fmt.Errorf("confirmation failed: %w", err) } diff --git a/pkg/console/confirm.go b/pkg/console/confirm.go new file mode 100644 index 00000000000..7e9b86f948e --- /dev/null +++ b/pkg/console/confirm.go @@ -0,0 +1,36 @@ +package console + +import ( + "os" + + "github.com/charmbracelet/huh" +) + +// isAccessibleMode detects if accessibility mode should be enabled based on environment variables +func isAccessibleMode() bool { + return os.Getenv("ACCESSIBLE") != "" || + os.Getenv("TERM") == "dumb" || + os.Getenv("NO_COLOR") != "" +} + +// ConfirmAction shows an interactive confirmation dialog using Bubble Tea (huh) +// Returns true if the user confirms, false if they cancel or an error occurs +func ConfirmAction(title, affirmative, negative string) (bool, error) { + var confirmed bool + + confirmForm := huh.NewForm( + huh.NewGroup( + huh.NewConfirm(). + Title(title). + Affirmative(affirmative). + Negative(negative). + Value(&confirmed), + ), + ).WithAccessible(isAccessibleMode()) + + if err := confirmForm.Run(); err != nil { + return false, err + } + + return confirmed, nil +} diff --git a/pkg/console/confirm_test.go b/pkg/console/confirm_test.go new file mode 100644 index 00000000000..439ed1efbef --- /dev/null +++ b/pkg/console/confirm_test.go @@ -0,0 +1,92 @@ +package console + +import ( + "os" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestIsAccessibleMode(t *testing.T) { + tests := []struct { + name string + envVars map[string]string + expected bool + }{ + { + name: "no environment variables", + envVars: map[string]string{}, + expected: false, + }, + { + name: "ACCESSIBLE set", + envVars: map[string]string{"ACCESSIBLE": "1"}, + expected: true, + }, + { + name: "TERM=dumb", + envVars: map[string]string{"TERM": "dumb"}, + expected: true, + }, + { + name: "NO_COLOR set", + envVars: map[string]string{"NO_COLOR": "1"}, + expected: true, + }, + { + name: "regular TERM", + envVars: map[string]string{"TERM": "xterm-256color"}, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Save original environment + originalEnv := make(map[string]string) + for key := range tt.envVars { + originalEnv[key] = os.Getenv(key) + } + + // Set test environment + for key, value := range tt.envVars { + if value == "" { + os.Unsetenv(key) + } else { + os.Setenv(key, value) + } + } + + // Clear other relevant env vars + for _, key := range []string{"ACCESSIBLE", "TERM", "NO_COLOR"} { + if _, exists := tt.envVars[key]; !exists { + os.Unsetenv(key) + } + } + + // Test + result := isAccessibleMode() + assert.Equal(t, tt.expected, result, "isAccessibleMode() = %v, want %v", result, tt.expected) + + // Restore original environment + for key, value := range originalEnv { + if value == "" { + os.Unsetenv(key) + } else { + os.Setenv(key, value) + } + } + }) + } +} + +func TestConfirmAction(t *testing.T) { + // Note: This test can't fully test the interactive behavior without mocking + // the terminal input, but we can verify the function signature and basic setup + + t.Run("function signature", func(t *testing.T) { + // This test just verifies the function exists and has the right signature + // Actual interactive testing would require a mock terminal + _ = ConfirmAction + }) +} From 78eedf87b18c3daa2012246ea220f1dd56879e72 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 5 Jan 2026 13:58:51 +0000 Subject: [PATCH 09/18] Add check for staged files before creating commit Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/run_push.go | 27 +++++++++++++++++++++ pkg/cli/run_push_test.go | 51 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+) diff --git a/pkg/cli/run_push.go b/pkg/cli/run_push.go index da85f77a47d..76c120d3b3b 100644 --- a/pkg/cli/run_push.go +++ b/pkg/cli/run_push.go @@ -272,6 +272,33 @@ func isWorkflowSpecFormatLocal(path string) bool { func pushWorkflowFiles(workflowName string, files []string, verbose bool) error { runPushLog.Printf("Pushing %d files for workflow: %s", len(files), workflowName) + // Check if there are any staged files in git + statusCmd := exec.Command("git", "diff", "--cached", "--name-only") + statusOutput, err := statusCmd.CombinedOutput() + if err != nil { + runPushLog.Printf("Failed to check git status: %v, output: %s", err, string(statusOutput)) + return fmt.Errorf("failed to check git status: %w\nOutput: %s", err, string(statusOutput)) + } + + // If there are any staged files, give up + if len(strings.TrimSpace(string(statusOutput))) > 0 { + stagedFiles := strings.Split(strings.TrimSpace(string(statusOutput)), "\n") + runPushLog.Printf("Found %d staged files, refusing to proceed", len(stagedFiles)) + + fmt.Fprintln(os.Stderr, "") + fmt.Fprintln(os.Stderr, console.FormatErrorMessage("Cannot proceed: there are already staged files in git")) + fmt.Fprintln(os.Stderr, "") + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Staged files:")) + for _, file := range stagedFiles { + fmt.Fprintf(os.Stderr, " - %s\n", file) + } + fmt.Fprintln(os.Stderr, "") + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Please commit or unstage these files before using --push")) + fmt.Fprintln(os.Stderr, "") + + return fmt.Errorf("git has staged files - commit or unstage them before using --push") + } + if verbose { fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Staging %d files for commit", len(files)))) for _, file := range files { diff --git a/pkg/cli/run_push_test.go b/pkg/cli/run_push_test.go index 130db2ce587..44b4a725a26 100644 --- a/pkg/cli/run_push_test.go +++ b/pkg/cli/run_push_test.go @@ -2,6 +2,7 @@ package cli import ( "os" + "os/exec" "path/filepath" "strings" "testing" @@ -331,3 +332,53 @@ assert.True(t, mdStat.ModTime().After(lockStat.ModTime()), "Workflow file should // but we can verify the detection logic works // The actual compilation would happen in an integration test } + +func TestPushWorkflowFiles_WithStagedFiles(t *testing.T) { +// Create a temporary directory for testing +tmpDir := t.TempDir() + +// Initialize a git repo +cmd := exec.Command("git", "init") +cmd.Dir = tmpDir +err := cmd.Run() +require.NoError(t, err) + +// Configure git +cmd = exec.Command("git", "config", "user.email", "test@example.com") +cmd.Dir = tmpDir +err = cmd.Run() +require.NoError(t, err) + +cmd = exec.Command("git", "config", "user.name", "Test User") +cmd.Dir = tmpDir +err = cmd.Run() +require.NoError(t, err) + +// Create a test file and stage it +testFile := filepath.Join(tmpDir, "test-file.txt") +err = os.WriteFile(testFile, []byte("test content"), 0644) +require.NoError(t, err) + +cmd = exec.Command("git", "add", "test-file.txt") +cmd.Dir = tmpDir +err = cmd.Run() +require.NoError(t, err) + +// Save current directory and change to tmpDir +originalDir, err := os.Getwd() +require.NoError(t, err) +err = os.Chdir(tmpDir) +require.NoError(t, err) +defer os.Chdir(originalDir) + +// Try to push workflow files - should fail due to staged files +workflowFile := filepath.Join(tmpDir, "workflow.md") +err = os.WriteFile(workflowFile, []byte("# Test"), 0644) +require.NoError(t, err) + +err = pushWorkflowFiles("test-workflow", []string{workflowFile}, false) + +// Should return an error about staged files +assert.Error(t, err) +assert.Contains(t, err.Error(), "staged files") +} From 889d12b9c2df61e7c1a803cbd6de709fede255a8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 5 Jan 2026 14:08:12 +0000 Subject: [PATCH 10/18] Move staged files check after staging workflow files Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/run_push.go | 99 +++++++++++++++++++++++++++++---------------- 1 file changed, 64 insertions(+), 35 deletions(-) diff --git a/pkg/cli/run_push.go b/pkg/cli/run_push.go index 76c120d3b3b..94d9d246ac8 100644 --- a/pkg/cli/run_push.go +++ b/pkg/cli/run_push.go @@ -272,33 +272,6 @@ func isWorkflowSpecFormatLocal(path string) bool { func pushWorkflowFiles(workflowName string, files []string, verbose bool) error { runPushLog.Printf("Pushing %d files for workflow: %s", len(files), workflowName) - // Check if there are any staged files in git - statusCmd := exec.Command("git", "diff", "--cached", "--name-only") - statusOutput, err := statusCmd.CombinedOutput() - if err != nil { - runPushLog.Printf("Failed to check git status: %v, output: %s", err, string(statusOutput)) - return fmt.Errorf("failed to check git status: %w\nOutput: %s", err, string(statusOutput)) - } - - // If there are any staged files, give up - if len(strings.TrimSpace(string(statusOutput))) > 0 { - stagedFiles := strings.Split(strings.TrimSpace(string(statusOutput)), "\n") - runPushLog.Printf("Found %d staged files, refusing to proceed", len(stagedFiles)) - - fmt.Fprintln(os.Stderr, "") - fmt.Fprintln(os.Stderr, console.FormatErrorMessage("Cannot proceed: there are already staged files in git")) - fmt.Fprintln(os.Stderr, "") - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Staged files:")) - for _, file := range stagedFiles { - fmt.Fprintf(os.Stderr, " - %s\n", file) - } - fmt.Fprintln(os.Stderr, "") - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Please commit or unstage these files before using --push")) - fmt.Fprintln(os.Stderr, "") - - return fmt.Errorf("git has staged files - commit or unstage them before using --push") - } - if verbose { fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Staging %d files for commit", len(files)))) for _, file := range files { @@ -318,6 +291,70 @@ func pushWorkflowFiles(workflowName string, files []string, verbose bool) error fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Files staged successfully")) } + // Check if there are any staged files in git (after we've staged our files) + statusCmd := exec.Command("git", "diff", "--cached", "--name-only") + statusOutput, err := statusCmd.CombinedOutput() + if err != nil { + runPushLog.Printf("Failed to check git status: %v, output: %s", err, string(statusOutput)) + return fmt.Errorf("failed to check git status: %w\nOutput: %s", err, string(statusOutput)) + } + + // Check if there are no staged changes (nothing to commit) + if len(strings.TrimSpace(string(statusOutput))) == 0 { + if verbose { + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("No changes to commit")) + } + runPushLog.Print("No changes to commit") + return nil + } + + // Get the list of staged files + stagedFiles := strings.Split(strings.TrimSpace(string(statusOutput)), "\n") + + // Check if there are staged files beyond what we just staged + // Convert our files list to a map for quick lookup + ourFiles := make(map[string]bool) + for _, file := range files { + // Normalize the path + absPath, err := filepath.Abs(file) + if err == nil { + ourFiles[absPath] = true + } + ourFiles[file] = true + } + + // Check if there are any staged files that aren't in our list + var extraStagedFiles []string + for _, stagedFile := range stagedFiles { + // Try both absolute and relative paths + absStagedPath, err := filepath.Abs(stagedFile) + if err == nil && ourFiles[absStagedPath] { + continue + } + if ourFiles[stagedFile] { + continue + } + extraStagedFiles = append(extraStagedFiles, stagedFile) + } + + // If there are extra staged files that we didn't stage, give up + if len(extraStagedFiles) > 0 { + runPushLog.Printf("Found %d extra staged files not in our list, refusing to proceed", len(extraStagedFiles)) + + fmt.Fprintln(os.Stderr, "") + fmt.Fprintln(os.Stderr, console.FormatErrorMessage("Cannot proceed: there are already staged files in git that are not part of this workflow")) + fmt.Fprintln(os.Stderr, "") + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Extra staged files:")) + for _, file := range extraStagedFiles { + fmt.Fprintf(os.Stderr, " - %s\n", file) + } + fmt.Fprintln(os.Stderr, "") + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Please commit or unstage these files before using --push")) + fmt.Fprintln(os.Stderr, "") + + return fmt.Errorf("git has staged files not part of workflow - commit or unstage them before using --push") + } + // Create commit message commitMessage := fmt.Sprintf("Updated agentic workflow %s", workflowName) runPushLog.Printf("Creating commit with message: %s", commitMessage) @@ -351,14 +388,6 @@ func pushWorkflowFiles(workflowName string, files []string, verbose bool) error // Commit the changes cmd = exec.Command("git", "commit", "-m", commitMessage) if output, err := cmd.CombinedOutput(); err != nil { - // Check if there are no changes to commit - if strings.Contains(string(output), "nothing to commit") { - if verbose { - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("No changes to commit")) - } - runPushLog.Print("No changes to commit") - return nil - } runPushLog.Printf("Failed to commit: %v, output: %s", err, string(output)) return fmt.Errorf("failed to commit changes: %w\nOutput: %s", err, string(output)) } From 4fa877ebd56511cb22cff87a2775d68cda8c93dc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 5 Jan 2026 14:17:39 +0000 Subject: [PATCH 11/18] Add warnings for missing/outdated lock files when not using --push Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/run_command.go | 14 +++++++++++++ pkg/cli/run_push.go | 47 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/pkg/cli/run_command.go b/pkg/cli/run_command.go index ac21edaf7fa..5122e006303 100644 --- a/pkg/cli/run_command.go +++ b/pkg/cli/run_command.go @@ -225,6 +225,20 @@ func RunWorkflowOnGitHub(ctx context.Context, workflowIdOrName string, enable bo fmt.Printf("Using lock file: %s\n", lockFileName) } + // Check for missing or outdated lock files (when not using --push) + if !push && repoOverride == "" { + workflowMarkdownPath := strings.TrimSuffix(lockFilePath, ".lock.yml") + ".md" + if status, err := checkLockFileStatus(workflowMarkdownPath); err == nil { + if status.Missing { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage("Lock file is missing")) + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Run 'gh aw compile %s' to create the lock file", workflowIdOrName))) + } else if status.Outdated { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage("Lock file is outdated (workflow file is newer)")) + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Run 'gh aw compile %s' to update the lock file", workflowIdOrName))) + } + } + } + // Handle --push flag: commit and push workflow files before running if push { // Only valid for local workflows diff --git a/pkg/cli/run_push.go b/pkg/cli/run_push.go index 94d9d246ac8..aace9fe17d5 100644 --- a/pkg/cli/run_push.go +++ b/pkg/cli/run_push.go @@ -119,6 +119,53 @@ func recompileWorkflow(workflowPath string, verbose bool) error { return nil } +// checkLockFileStatus checks if a lock file is missing or outdated and returns status info +type LockFileStatus struct { + Missing bool + Outdated bool + LockPath string +} + +// checkLockFileStatus checks the status of a workflow's lock file +func checkLockFileStatus(workflowPath string) (*LockFileStatus, error) { + runPushLog.Printf("Checking lock file status for: %s", workflowPath) + + // Get absolute path for the workflow + absWorkflowPath, err := filepath.Abs(workflowPath) + if err != nil { + return nil, fmt.Errorf("failed to get absolute path for workflow: %w", err) + } + + lockFilePath := strings.TrimSuffix(absWorkflowPath, ".md") + ".lock.yml" + status := &LockFileStatus{ + LockPath: lockFilePath, + } + + // Check if lock file exists + lockStat, err := os.Stat(lockFilePath) + if err != nil { + if os.IsNotExist(err) { + status.Missing = true + runPushLog.Printf("Lock file missing: %s", lockFilePath) + return status, nil + } + return nil, fmt.Errorf("failed to stat lock file: %w", err) + } + + // Lock file exists - check if it's outdated + mdStat, err := os.Stat(absWorkflowPath) + if err != nil { + return nil, fmt.Errorf("failed to stat workflow file: %w", err) + } + + if mdStat.ModTime().After(lockStat.ModTime()) { + status.Outdated = true + runPushLog.Printf("Lock file outdated (md: %v, lock: %v)", mdStat.ModTime(), lockStat.ModTime()) + } + + return status, nil +} + // collectImports recursively collects all imported files (transitive closure) func collectImports(workflowPath string, files map[string]bool, visited map[string]bool, verbose bool) error { // Avoid processing the same file multiple times From e7fadd80cec9d0571b6dfd8ac8133637d15bf3d1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 5 Jan 2026 14:30:50 +0000 Subject: [PATCH 12/18] Add extensive logging and fix slice allocation/sorting in run_push.go Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/run_push.go | 118 +++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 111 insertions(+), 7 deletions(-) diff --git a/pkg/cli/run_push.go b/pkg/cli/run_push.go index aace9fe17d5..0930fe3f119 100644 --- a/pkg/cli/run_push.go +++ b/pkg/cli/run_push.go @@ -6,6 +6,7 @@ import ( "os" "os/exec" "path/filepath" + "sort" "strings" "github.com/githubnext/gh-aw/pkg/console" @@ -26,8 +27,10 @@ func collectWorkflowFiles(workflowPath string, verbose bool) ([]string, error) { // Get absolute path for the workflow absWorkflowPath, err := filepath.Abs(workflowPath) if err != nil { + runPushLog.Printf("Failed to get absolute path for %s: %v", workflowPath, err) return nil, fmt.Errorf("failed to get absolute path for workflow: %w", err) } + runPushLog.Printf("Resolved absolute workflow path: %s", absWorkflowPath) // Add the workflow .md file files[absWorkflowPath] = true @@ -35,17 +38,22 @@ func collectWorkflowFiles(workflowPath string, verbose bool) ([]string, error) { // Check if lock file needs recompilation lockFilePath := strings.TrimSuffix(absWorkflowPath, ".md") + ".lock.yml" + runPushLog.Printf("Checking lock file: %s", lockFilePath) needsRecompile := false if lockStat, err := os.Stat(lockFilePath); err == nil { + runPushLog.Printf("Lock file exists: %s", lockFilePath) // Lock file exists - check if it's outdated if mdStat, err := os.Stat(absWorkflowPath); err == nil { + runPushLog.Printf("Comparing modification times - md: %v, lock: %v", mdStat.ModTime(), lockStat.ModTime()) if mdStat.ModTime().After(lockStat.ModTime()) { needsRecompile = true runPushLog.Printf("Lock file is outdated (md: %v, lock: %v)", mdStat.ModTime(), lockStat.ModTime()) if verbose { fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Detected outdated lock file, recompiling workflow...")) } + } else { + runPushLog.Printf("Lock file is up-to-date") } } } else if os.IsNotExist(err) { @@ -55,16 +63,23 @@ func collectWorkflowFiles(workflowPath string, verbose bool) ([]string, error) { if verbose { fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Lock file not found, compiling workflow...")) } + } else { + runPushLog.Printf("Error checking lock file: %v", err) } // Recompile if needed if needsRecompile { + runPushLog.Printf("Recompilation needed for %s", absWorkflowPath) if err := recompileWorkflow(absWorkflowPath, verbose); err != nil { + runPushLog.Printf("Failed to recompile workflow: %v", err) return nil, fmt.Errorf("failed to recompile workflow: %w", err) } if verbose { fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Workflow compiled successfully")) } + runPushLog.Printf("Recompilation completed successfully") + } else { + runPushLog.Printf("Recompilation not needed") } // Add the corresponding .lock.yml file @@ -72,19 +87,27 @@ func collectWorkflowFiles(workflowPath string, verbose bool) ([]string, error) { files[lockFilePath] = true runPushLog.Printf("Added lock file: %s", lockFilePath) } else if verbose { + runPushLog.Printf("Lock file not found after compilation: %s", lockFilePath) fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Lock file not found after compilation: %s", lockFilePath))) } // Collect transitive closure of imported files + runPushLog.Printf("Starting import collection for %s", absWorkflowPath) if err := collectImports(absWorkflowPath, files, visited, verbose); err != nil { + runPushLog.Printf("Failed to collect imports: %v", err) return nil, fmt.Errorf("failed to collect imports: %w", err) } + runPushLog.Printf("Import collection completed") // Convert map to slice - result := make([]string, 0, len(files)) + result := make([]string, 0) for file := range files { result = append(result, file) } + + // Sort files for stable output + sort.Strings(result) + runPushLog.Printf("Sorted %d files for stable output", len(result)) runPushLog.Printf("Collected %d files total", len(result)) return result, nil @@ -109,9 +132,13 @@ func recompileWorkflow(workflowPath string, verbose bool) error { Strict: false, } + runPushLog.Printf("Compilation config: Validate=%v, NoEmit=%v", config.Validate, config.NoEmit) + // Use background context for compilation ctx := context.Background() + runPushLog.Printf("Starting compilation with CompileWorkflows") if _, err := CompileWorkflows(ctx, config); err != nil { + runPushLog.Printf("Compilation failed: %v", err) return fmt.Errorf("compilation failed: %w", err) } @@ -133,10 +160,13 @@ func checkLockFileStatus(workflowPath string) (*LockFileStatus, error) { // Get absolute path for the workflow absWorkflowPath, err := filepath.Abs(workflowPath) if err != nil { + runPushLog.Printf("Failed to get absolute path for %s: %v", workflowPath, err) return nil, fmt.Errorf("failed to get absolute path for workflow: %w", err) } + runPushLog.Printf("Resolved absolute path: %s", absWorkflowPath) lockFilePath := strings.TrimSuffix(absWorkflowPath, ".md") + ".lock.yml" + runPushLog.Printf("Expected lock file path: %s", lockFilePath) status := &LockFileStatus{ LockPath: lockFilePath, } @@ -149,18 +179,24 @@ func checkLockFileStatus(workflowPath string) (*LockFileStatus, error) { runPushLog.Printf("Lock file missing: %s", lockFilePath) return status, nil } + runPushLog.Printf("Error stating lock file: %v", err) return nil, fmt.Errorf("failed to stat lock file: %w", err) } + runPushLog.Printf("Lock file exists: %s (modtime: %v)", lockFilePath, lockStat.ModTime()) // Lock file exists - check if it's outdated mdStat, err := os.Stat(absWorkflowPath) if err != nil { + runPushLog.Printf("Error stating workflow file: %v", err) return nil, fmt.Errorf("failed to stat workflow file: %w", err) } + runPushLog.Printf("Workflow file modtime: %v", mdStat.ModTime()) if mdStat.ModTime().After(lockStat.ModTime()) { status.Outdated = true runPushLog.Printf("Lock file outdated (md: %v, lock: %v)", mdStat.ModTime(), lockStat.ModTime()) + } else { + runPushLog.Printf("Lock file is up-to-date") } return status, nil @@ -170,6 +206,7 @@ func checkLockFileStatus(workflowPath string) (*LockFileStatus, error) { func collectImports(workflowPath string, files map[string]bool, visited map[string]bool, verbose bool) error { // Avoid processing the same file multiple times if visited[workflowPath] { + runPushLog.Printf("Skipping already visited file: %s", workflowPath) return nil } visited[workflowPath] = true @@ -179,16 +216,19 @@ func collectImports(workflowPath string, files map[string]bool, visited map[stri // Read the workflow file content, err := os.ReadFile(workflowPath) if err != nil { + runPushLog.Printf("Failed to read workflow file %s: %v", workflowPath, err) return fmt.Errorf("failed to read workflow file %s: %w", workflowPath, err) } + runPushLog.Printf("Read %d bytes from %s", len(content), workflowPath) // Extract frontmatter to get imports field result, err := parser.ExtractFrontmatterFromContent(string(content)) if err != nil { // No frontmatter is okay - might be a simple file - runPushLog.Printf("No frontmatter in %s, skipping imports extraction", workflowPath) + runPushLog.Printf("No frontmatter in %s, skipping imports extraction: %v", workflowPath, err) return nil } + runPushLog.Printf("Extracted frontmatter from %s", workflowPath) // Get imports from frontmatter importsField, exists := result.Frontmatter["imports"] @@ -196,79 +236,107 @@ func collectImports(workflowPath string, files map[string]bool, visited map[stri runPushLog.Printf("No imports field in %s", workflowPath) return nil } + runPushLog.Printf("Found imports field in %s", workflowPath) // Parse imports field - can be array of strings or objects with path workflowDir := filepath.Dir(workflowPath) + runPushLog.Printf("Workflow directory: %s", workflowDir) var imports []string switch v := importsField.(type) { case []any: - for _, item := range v { + runPushLog.Printf("Parsing imports as []any with %d items", len(v)) + for i, item := range v { switch importItem := item.(type) { case string: // Simple string import + runPushLog.Printf("Import %d: string format: %s", i, importItem) imports = append(imports, importItem) case map[string]any: // Object import with path field if pathValue, hasPath := importItem["path"]; hasPath { if pathStr, ok := pathValue.(string); ok { + runPushLog.Printf("Import %d: object format with path: %s", i, pathStr) imports = append(imports, pathStr) + } else { + runPushLog.Printf("Import %d: object has path but not string type", i) } + } else { + runPushLog.Printf("Import %d: object missing path field", i) } + default: + runPushLog.Printf("Import %d: unknown type: %T", i, importItem) } } case []string: + runPushLog.Printf("Parsing imports as []string with %d items", len(v)) imports = v + default: + runPushLog.Printf("Imports field has unexpected type: %T", v) } runPushLog.Printf("Found %d imports in %s", len(imports), workflowPath) // Process each import - for _, importPath := range imports { + for i, importPath := range imports { + runPushLog.Printf("Processing import %d/%d: %s", i+1, len(imports), importPath) + // Resolve the import path resolvedPath := resolveImportPathLocal(importPath, workflowDir) if resolvedPath == "" { + runPushLog.Printf("Could not resolve import path: %s", importPath) if verbose { fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Could not resolve import: %s", importPath))) } continue } + runPushLog.Printf("Resolved import path: %s -> %s", importPath, resolvedPath) // Get absolute path var absImportPath string if filepath.IsAbs(resolvedPath) { absImportPath = resolvedPath + runPushLog.Printf("Import path is absolute: %s", absImportPath) } else { absImportPath = filepath.Join(workflowDir, resolvedPath) + runPushLog.Printf("Joined relative path: %s + %s = %s", workflowDir, resolvedPath, absImportPath) } // Check if file exists if _, err := os.Stat(absImportPath); err != nil { + runPushLog.Printf("Import file not found: %s (error: %v)", absImportPath, err) if verbose { fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Import file not found: %s", absImportPath))) } continue } + runPushLog.Printf("Import file exists: %s", absImportPath) // Add the import file files[absImportPath] = true runPushLog.Printf("Added import file: %s", absImportPath) // Recursively collect imports from this file + runPushLog.Printf("Recursively collecting imports from: %s", absImportPath) if err := collectImports(absImportPath, files, visited, verbose); err != nil { + runPushLog.Printf("Failed to recursively collect imports from %s: %v", absImportPath, err) return err } } + runPushLog.Printf("Finished processing imports for: %s", workflowPath) return nil } // resolveImportPathLocal is a local version of resolveImportPath for push functionality // This is needed to avoid circular dependencies with imports.go func resolveImportPathLocal(importPath, baseDir string) string { + runPushLog.Printf("Resolving import path: %s (baseDir: %s)", importPath, baseDir) + // Handle section references (file.md#Section) - strip the section part if strings.Contains(importPath, "#") { parts := strings.SplitN(importPath, "#", 2) + runPushLog.Printf("Stripping section reference: %s -> %s", importPath, parts[0]) importPath = parts[0] } @@ -280,23 +348,32 @@ func resolveImportPathLocal(importPath, baseDir string) string { // If the import path is absolute (starts with /), use it relative to repo root if strings.HasPrefix(importPath, "/") { + runPushLog.Printf("Import path is absolute (starts with /): %s", importPath) // Find git root gitRoot, err := findGitRoot() if err != nil { + runPushLog.Printf("Failed to find git root: %v", err) return "" } - return filepath.Join(gitRoot, strings.TrimPrefix(importPath, "/")) + resolved := filepath.Join(gitRoot, strings.TrimPrefix(importPath, "/")) + runPushLog.Printf("Resolved absolute import: %s (git root: %s)", resolved, gitRoot) + return resolved } // Otherwise, resolve relative to the workflow file's directory - return filepath.Join(baseDir, importPath) + resolved := filepath.Join(baseDir, importPath) + runPushLog.Printf("Resolved relative import: %s", resolved) + return resolved } // isWorkflowSpecFormatLocal is a local version of isWorkflowSpecFormat for push functionality // This is duplicated from imports.go to avoid circular dependencies func isWorkflowSpecFormatLocal(path string) bool { + runPushLog.Printf("Checking if workflowspec format: %s", path) + // Check if it contains @ (ref separator) or looks like owner/repo/path if strings.Contains(path, "@") { + runPushLog.Printf("Path contains @ - workflowspec format: %s", path) return true } @@ -304,20 +381,24 @@ func isWorkflowSpecFormatLocal(path string) bool { cleanPath := path if idx := strings.Index(path, "#"); idx != -1 { cleanPath = path[:idx] + runPushLog.Printf("Removed section reference: %s -> %s", path, cleanPath) } // Check if it has at least 3 parts and doesn't start with . or / parts := strings.Split(cleanPath, "/") if len(parts) >= 3 && !strings.HasPrefix(cleanPath, ".") && !strings.HasPrefix(cleanPath, "/") { + runPushLog.Printf("Path has %d parts and matches owner/repo/path format - workflowspec format: %s", len(parts), path) return true } + runPushLog.Printf("Path is not workflowspec format: %s", path) return false } // pushWorkflowFiles commits and pushes the workflow files to the repository func pushWorkflowFiles(workflowName string, files []string, verbose bool) error { runPushLog.Printf("Pushing %d files for workflow: %s", len(files), workflowName) + runPushLog.Printf("Files to push: %v", files) if verbose { fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Staging %d files for commit", len(files)))) @@ -328,26 +409,31 @@ func pushWorkflowFiles(workflowName string, files []string, verbose bool) error // Stage all files gitArgs := append([]string{"add"}, files...) + runPushLog.Printf("Executing git command: git %v", gitArgs) cmd := exec.Command("git", gitArgs...) if output, err := cmd.CombinedOutput(); err != nil { runPushLog.Printf("Failed to stage files: %v, output: %s", err, string(output)) return fmt.Errorf("failed to stage files: %w\nOutput: %s", err, string(output)) } + runPushLog.Printf("Successfully staged %d files", len(files)) if verbose { fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Files staged successfully")) } // Check if there are any staged files in git (after we've staged our files) + runPushLog.Printf("Checking staged files with git diff --cached --name-only") statusCmd := exec.Command("git", "diff", "--cached", "--name-only") statusOutput, err := statusCmd.CombinedOutput() if err != nil { runPushLog.Printf("Failed to check git status: %v, output: %s", err, string(statusOutput)) return fmt.Errorf("failed to check git status: %w\nOutput: %s", err, string(statusOutput)) } + runPushLog.Printf("Git status output: %s", string(statusOutput)) // Check if there are no staged changes (nothing to commit) if len(strings.TrimSpace(string(statusOutput))) == 0 { + runPushLog.Printf("No staged changes detected") if verbose { fmt.Fprintln(os.Stderr, console.FormatInfoMessage("No changes to commit")) } @@ -357,36 +443,47 @@ func pushWorkflowFiles(workflowName string, files []string, verbose bool) error // Get the list of staged files stagedFiles := strings.Split(strings.TrimSpace(string(statusOutput)), "\n") + runPushLog.Printf("Found %d staged files: %v", len(stagedFiles), stagedFiles) // Check if there are staged files beyond what we just staged // Convert our files list to a map for quick lookup + runPushLog.Printf("Building map of our files for comparison") ourFiles := make(map[string]bool) for _, file := range files { // Normalize the path absPath, err := filepath.Abs(file) if err == nil { ourFiles[absPath] = true + runPushLog.Printf("Added to our files map: %s (absolute: %s)", file, absPath) + } else { + runPushLog.Printf("Failed to get absolute path for %s: %v", file, err) } ourFiles[file] = true + runPushLog.Printf("Added to our files map: %s", file) } // Check if there are any staged files that aren't in our list + runPushLog.Printf("Checking for extra staged files not in our list") var extraStagedFiles []string for _, stagedFile := range stagedFiles { + runPushLog.Printf("Checking staged file: %s", stagedFile) // Try both absolute and relative paths absStagedPath, err := filepath.Abs(stagedFile) if err == nil && ourFiles[absStagedPath] { + runPushLog.Printf("Staged file %s matches our file %s (absolute)", stagedFile, absStagedPath) continue } if ourFiles[stagedFile] { + runPushLog.Printf("Staged file %s matches our file (relative)", stagedFile) continue } + runPushLog.Printf("Extra staged file detected: %s", stagedFile) extraStagedFiles = append(extraStagedFiles, stagedFile) } // If there are extra staged files that we didn't stage, give up if len(extraStagedFiles) > 0 { - runPushLog.Printf("Found %d extra staged files not in our list, refusing to proceed", len(extraStagedFiles)) + runPushLog.Printf("Found %d extra staged files not in our list, refusing to proceed: %v", len(extraStagedFiles), extraStagedFiles) fmt.Fprintln(os.Stderr, "") fmt.Fprintln(os.Stderr, console.FormatErrorMessage("Cannot proceed: there are already staged files in git that are not part of this workflow")) @@ -401,6 +498,7 @@ func pushWorkflowFiles(workflowName string, files []string, verbose bool) error return fmt.Errorf("git has staged files not part of workflow - commit or unstage them before using --push") } + runPushLog.Printf("No extra staged files detected - all staged files are part of our workflow") // Create commit message commitMessage := fmt.Sprintf("Updated agentic workflow %s", workflowName) @@ -417,6 +515,7 @@ func pushWorkflowFiles(workflowName string, files []string, verbose bool) error fmt.Fprintln(os.Stderr, "") // Ask for confirmation using console helper + runPushLog.Printf("Requesting user confirmation for commit and push") confirmed, err := console.ConfirmAction( "Do you want to commit and push these changes?", "Yes, commit and push", @@ -431,13 +530,16 @@ func pushWorkflowFiles(workflowName string, files []string, verbose bool) error runPushLog.Print("Push cancelled by user") return fmt.Errorf("push cancelled by user") } + runPushLog.Printf("User confirmed - proceeding with commit and push") // Commit the changes + runPushLog.Printf("Executing git commit with message: %s", commitMessage) cmd = exec.Command("git", "commit", "-m", commitMessage) if output, err := cmd.CombinedOutput(); err != nil { runPushLog.Printf("Failed to commit: %v, output: %s", err, string(output)) return fmt.Errorf("failed to commit changes: %w\nOutput: %s", err, string(output)) } + runPushLog.Printf("Commit successful") if verbose { fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Changes committed successfully")) @@ -445,11 +547,13 @@ func pushWorkflowFiles(workflowName string, files []string, verbose bool) error // Push the changes runPushLog.Print("Pushing changes to remote") + runPushLog.Printf("Executing git push") cmd = exec.Command("git", "push") if output, err := cmd.CombinedOutput(); err != nil { runPushLog.Printf("Failed to push: %v, output: %s", err, string(output)) return fmt.Errorf("failed to push changes: %w\nOutput: %s", err, string(output)) } + runPushLog.Printf("Push to remote successful") if verbose { fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Changes pushed to remote")) From 45038431310be55c4e544d61420348832d5f10b5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 5 Jan 2026 14:49:44 +0000 Subject: [PATCH 13/18] Update warning messages to suggest using --push instead of compile Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/run_command.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cli/run_command.go b/pkg/cli/run_command.go index 5122e006303..c20fce89e10 100644 --- a/pkg/cli/run_command.go +++ b/pkg/cli/run_command.go @@ -231,10 +231,10 @@ func RunWorkflowOnGitHub(ctx context.Context, workflowIdOrName string, enable bo if status, err := checkLockFileStatus(workflowMarkdownPath); err == nil { if status.Missing { fmt.Fprintln(os.Stderr, console.FormatWarningMessage("Lock file is missing")) - fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Run 'gh aw compile %s' to create the lock file", workflowIdOrName))) + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Run 'gh aw run %s --push' to automatically compile and push the lock file", workflowIdOrName))) } else if status.Outdated { fmt.Fprintln(os.Stderr, console.FormatWarningMessage("Lock file is outdated (workflow file is newer)")) - fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Run 'gh aw compile %s' to update the lock file", workflowIdOrName))) + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Run 'gh aw run %s --push' to automatically compile and push the lock file", workflowIdOrName))) } } } From a466a5c496eeabebf5df99aa826d8052637c59e3 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 5 Jan 2026 14:58:56 +0000 Subject: [PATCH 14/18] Refactor isAccessibleMode to own file and remove duplicates Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/interactive.go | 13 ++--- pkg/cli/interactive_test.go | 4 +- pkg/console/accessibility.go | 19 +++++++ pkg/console/accessibility_test.go | 83 +++++++++++++++++++++++++++++++ pkg/console/confirm.go | 11 +--- pkg/console/confirm_test.go | 76 ---------------------------- 6 files changed, 108 insertions(+), 98 deletions(-) create mode 100644 pkg/console/accessibility.go create mode 100644 pkg/console/accessibility_test.go diff --git a/pkg/cli/interactive.go b/pkg/cli/interactive.go index f1bdd91fe69..cb2794c1fcd 100644 --- a/pkg/cli/interactive.go +++ b/pkg/cli/interactive.go @@ -30,13 +30,6 @@ var commonWorkflowNames = []string{ "documentation-check", } -// isAccessibleMode detects if accessibility mode should be enabled based on environment variables -func isAccessibleMode() bool { - return os.Getenv("ACCESSIBLE") != "" || - os.Getenv("TERM") == "dumb" || - os.Getenv("NO_COLOR") != "" -} - // InteractiveWorkflowBuilder collects user input to build an agentic workflow type InteractiveWorkflowBuilder struct { WorkflowName string @@ -102,7 +95,7 @@ func (b *InteractiveWorkflowBuilder) promptForWorkflowName() error { Value(&b.WorkflowName). Validate(ValidateWorkflowName), ), - ).WithAccessible(isAccessibleMode()) + ).WithAccessible(console.IsAccessibleMode()) return form.Run() } @@ -225,7 +218,7 @@ func (b *InteractiveWorkflowBuilder) promptForConfiguration() error { ). Title("Instructions"). Description("Describe what you want this workflow to accomplish"), - ).WithAccessible(isAccessibleMode()) + ).WithAccessible(console.IsAccessibleMode()) if err := form.Run(); err != nil { return err @@ -268,7 +261,7 @@ func (b *InteractiveWorkflowBuilder) generateWorkflow(force bool) error { Negative("No, cancel"). Value(&overwrite), ), - ).WithAccessible(isAccessibleMode()) + ).WithAccessible(console.IsAccessibleMode()) if err := confirmForm.Run(); err != nil { return fmt.Errorf("confirmation failed: %w", err) diff --git a/pkg/cli/interactive_test.go b/pkg/cli/interactive_test.go index 6b5a513da7c..1d66f9c8587 100644 --- a/pkg/cli/interactive_test.go +++ b/pkg/cli/interactive_test.go @@ -225,7 +225,7 @@ func TestIsAccessibleMode(t *testing.T) { os.Unsetenv("NO_COLOR") } - result := isAccessibleMode() + result := console.IsAccessibleMode() // Restore original values if origAccessible != "" { @@ -245,7 +245,7 @@ func TestIsAccessibleMode(t *testing.T) { } if result != tt.expected { - t.Errorf("isAccessibleMode() with ACCESSIBLE=%q TERM=%q NO_COLOR=%q = %v, want %v", + t.Errorf("console.IsAccessibleMode() with ACCESSIBLE=%q TERM=%q NO_COLOR=%q = %v, want %v", tt.accessible, tt.term, tt.noColor, result, tt.expected) } }) diff --git a/pkg/console/accessibility.go b/pkg/console/accessibility.go new file mode 100644 index 00000000000..d33cb83eac6 --- /dev/null +++ b/pkg/console/accessibility.go @@ -0,0 +1,19 @@ +package console + +import "os" + +// IsAccessibleMode detects if accessibility mode should be enabled based on environment variables. +// Accessibility mode is enabled when: +// - ACCESSIBLE environment variable is set to any value +// - TERM environment variable is set to "dumb" +// - NO_COLOR environment variable is set to any value +// +// This function should be used by UI components to determine whether to: +// - Disable animations and spinners +// - Simplify interactive elements +// - Use plain text instead of fancy formatting +func IsAccessibleMode() bool { + return os.Getenv("ACCESSIBLE") != "" || + os.Getenv("TERM") == "dumb" || + os.Getenv("NO_COLOR") != "" +} diff --git a/pkg/console/accessibility_test.go b/pkg/console/accessibility_test.go new file mode 100644 index 00000000000..59a818f9f0a --- /dev/null +++ b/pkg/console/accessibility_test.go @@ -0,0 +1,83 @@ +package console + +import ( + "os" + "testing" +) + +func TestIsAccessibleMode(t *testing.T) { + tests := []struct { + name string + envVars map[string]string + expected bool + }{ + { + name: "ACCESSIBLE set", + envVars: map[string]string{"ACCESSIBLE": "1"}, + expected: true, + }, + { + name: "TERM=dumb", + envVars: map[string]string{"TERM": "dumb"}, + expected: true, + }, + { + name: "NO_COLOR set", + envVars: map[string]string{"NO_COLOR": "1"}, + expected: true, + }, + { + name: "no accessibility indicators", + envVars: map[string]string{}, + expected: false, + }, + { + name: "TERM not dumb", + envVars: map[string]string{"TERM": "xterm-256color"}, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Save original environment + origAccessible := os.Getenv("ACCESSIBLE") + origTerm := os.Getenv("TERM") + origNoColor := os.Getenv("NO_COLOR") + + // Clean up after test + defer func() { + if origAccessible != "" { + os.Setenv("ACCESSIBLE", origAccessible) + } else { + os.Unsetenv("ACCESSIBLE") + } + if origTerm != "" { + os.Setenv("TERM", origTerm) + } else { + os.Unsetenv("TERM") + } + if origNoColor != "" { + os.Setenv("NO_COLOR", origNoColor) + } else { + os.Unsetenv("NO_COLOR") + } + }() + + // Clear all relevant env vars first + for _, key := range []string{"ACCESSIBLE", "TERM", "NO_COLOR"} { + os.Unsetenv(key) + } + + // Set test env vars + for key, value := range tt.envVars { + os.Setenv(key, value) + } + + result := IsAccessibleMode() + if result != tt.expected { + t.Errorf("IsAccessibleMode() = %v, want %v", result, tt.expected) + } + }) + } +} diff --git a/pkg/console/confirm.go b/pkg/console/confirm.go index 7e9b86f948e..440752c8ce2 100644 --- a/pkg/console/confirm.go +++ b/pkg/console/confirm.go @@ -1,18 +1,9 @@ package console import ( - "os" - "github.com/charmbracelet/huh" ) -// isAccessibleMode detects if accessibility mode should be enabled based on environment variables -func isAccessibleMode() bool { - return os.Getenv("ACCESSIBLE") != "" || - os.Getenv("TERM") == "dumb" || - os.Getenv("NO_COLOR") != "" -} - // ConfirmAction shows an interactive confirmation dialog using Bubble Tea (huh) // Returns true if the user confirms, false if they cancel or an error occurs func ConfirmAction(title, affirmative, negative string) (bool, error) { @@ -26,7 +17,7 @@ func ConfirmAction(title, affirmative, negative string) (bool, error) { Negative(negative). Value(&confirmed), ), - ).WithAccessible(isAccessibleMode()) + ).WithAccessible(IsAccessibleMode()) if err := confirmForm.Run(); err != nil { return false, err diff --git a/pkg/console/confirm_test.go b/pkg/console/confirm_test.go index 439ed1efbef..748730f630b 100644 --- a/pkg/console/confirm_test.go +++ b/pkg/console/confirm_test.go @@ -1,85 +1,9 @@ package console import ( - "os" "testing" - - "github.com/stretchr/testify/assert" ) -func TestIsAccessibleMode(t *testing.T) { - tests := []struct { - name string - envVars map[string]string - expected bool - }{ - { - name: "no environment variables", - envVars: map[string]string{}, - expected: false, - }, - { - name: "ACCESSIBLE set", - envVars: map[string]string{"ACCESSIBLE": "1"}, - expected: true, - }, - { - name: "TERM=dumb", - envVars: map[string]string{"TERM": "dumb"}, - expected: true, - }, - { - name: "NO_COLOR set", - envVars: map[string]string{"NO_COLOR": "1"}, - expected: true, - }, - { - name: "regular TERM", - envVars: map[string]string{"TERM": "xterm-256color"}, - expected: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - // Save original environment - originalEnv := make(map[string]string) - for key := range tt.envVars { - originalEnv[key] = os.Getenv(key) - } - - // Set test environment - for key, value := range tt.envVars { - if value == "" { - os.Unsetenv(key) - } else { - os.Setenv(key, value) - } - } - - // Clear other relevant env vars - for _, key := range []string{"ACCESSIBLE", "TERM", "NO_COLOR"} { - if _, exists := tt.envVars[key]; !exists { - os.Unsetenv(key) - } - } - - // Test - result := isAccessibleMode() - assert.Equal(t, tt.expected, result, "isAccessibleMode() = %v, want %v", result, tt.expected) - - // Restore original environment - for key, value := range originalEnv { - if value == "" { - os.Unsetenv(key) - } else { - os.Setenv(key, value) - } - } - }) - } -} - func TestConfirmAction(t *testing.T) { // Note: This test can't fully test the interactive behavior without mocking // the terminal input, but we can verify the function signature and basic setup From 5895b64032ab3c8d8429a317fe932c41c93ce64b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 5 Jan 2026 15:17:26 +0000 Subject: [PATCH 15/18] Add branch verification check for --push with --ref flag Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/interactive_test.go | 2 ++ pkg/cli/run_command.go | 15 +++++++++++++++ 2 files changed, 17 insertions(+) diff --git a/pkg/cli/interactive_test.go b/pkg/cli/interactive_test.go index 1d66f9c8587..d35ca48baef 100644 --- a/pkg/cli/interactive_test.go +++ b/pkg/cli/interactive_test.go @@ -4,6 +4,8 @@ import ( "os" "strings" "testing" + + "github.com/githubnext/gh-aw/pkg/console" ) func TestValidateWorkflowName_Integration(t *testing.T) { diff --git a/pkg/cli/run_command.go b/pkg/cli/run_command.go index c20fce89e10..85345437856 100644 --- a/pkg/cli/run_command.go +++ b/pkg/cli/run_command.go @@ -246,6 +246,21 @@ func RunWorkflowOnGitHub(ctx context.Context, workflowIdOrName string, enable bo return fmt.Errorf("--push flag is only supported for local workflows, not remote repositories") } + // Check that current branch matches --ref value if specified + // Switching branches is not supported + if refOverride != "" { + currentBranch, err := getCurrentBranch() + if err != nil { + return fmt.Errorf("failed to determine current branch: %w", err) + } + if currentBranch != refOverride { + return fmt.Errorf("--push requires the current branch (%s) to match the --ref value (%s). Switching branches is not supported. Please checkout the target branch first", currentBranch, refOverride) + } + if verbose { + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Verified current branch matches --ref: %s", currentBranch))) + } + } + if verbose { fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Collecting workflow files for push...")) } From b79562525fe730c01710ce35971629b5315ac9bb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 5 Jan 2026 15:33:03 +0000 Subject: [PATCH 16/18] Move branch validation to after changes detection Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/run_command.go | 19 ++----------------- pkg/cli/run_push.go | 24 +++++++++++++++++++++++- pkg/cli/run_push_test.go | 2 +- 3 files changed, 26 insertions(+), 19 deletions(-) diff --git a/pkg/cli/run_command.go b/pkg/cli/run_command.go index 85345437856..66f2d2d39be 100644 --- a/pkg/cli/run_command.go +++ b/pkg/cli/run_command.go @@ -246,21 +246,6 @@ func RunWorkflowOnGitHub(ctx context.Context, workflowIdOrName string, enable bo return fmt.Errorf("--push flag is only supported for local workflows, not remote repositories") } - // Check that current branch matches --ref value if specified - // Switching branches is not supported - if refOverride != "" { - currentBranch, err := getCurrentBranch() - if err != nil { - return fmt.Errorf("failed to determine current branch: %w", err) - } - if currentBranch != refOverride { - return fmt.Errorf("--push requires the current branch (%s) to match the --ref value (%s). Switching branches is not supported. Please checkout the target branch first", currentBranch, refOverride) - } - if verbose { - fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Verified current branch matches --ref: %s", currentBranch))) - } - } - if verbose { fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Collecting workflow files for push...")) } @@ -272,8 +257,8 @@ func RunWorkflowOnGitHub(ctx context.Context, workflowIdOrName string, enable bo return fmt.Errorf("failed to collect workflow files: %w", err) } - // Commit and push the files - if err := pushWorkflowFiles(workflowIdOrName, files, verbose); err != nil { + // Commit and push the files (includes branch verification if --ref is specified) + if err := pushWorkflowFiles(workflowIdOrName, files, refOverride, verbose); err != nil { return fmt.Errorf("failed to push workflow files: %w", err) } diff --git a/pkg/cli/run_push.go b/pkg/cli/run_push.go index 0930fe3f119..a5cc422c35e 100644 --- a/pkg/cli/run_push.go +++ b/pkg/cli/run_push.go @@ -396,7 +396,7 @@ func isWorkflowSpecFormatLocal(path string) bool { } // pushWorkflowFiles commits and pushes the workflow files to the repository -func pushWorkflowFiles(workflowName string, files []string, verbose bool) error { +func pushWorkflowFiles(workflowName string, files []string, refOverride string, verbose bool) error { runPushLog.Printf("Pushing %d files for workflow: %s", len(files), workflowName) runPushLog.Printf("Files to push: %v", files) @@ -440,6 +440,28 @@ func pushWorkflowFiles(workflowName string, files []string, verbose bool) error runPushLog.Print("No changes to commit") return nil } + + // Now that we know there are changes to commit, check that current branch matches --ref value if specified + // This happens after we've determined there are actual changes, so we don't fail unnecessarily + if refOverride != "" { + runPushLog.Printf("Checking if current branch matches --ref value: %s", refOverride) + currentBranch, err := getCurrentBranch() + if err != nil { + runPushLog.Printf("Failed to determine current branch: %v", err) + return fmt.Errorf("failed to determine current branch: %w", err) + } + runPushLog.Printf("Current branch: %s", currentBranch) + + if currentBranch != refOverride { + runPushLog.Printf("Current branch (%s) does not match --ref value (%s)", currentBranch, refOverride) + return fmt.Errorf("--push requires the current branch (%s) to match the --ref value (%s). Switching branches is not supported. Please checkout the target branch first", currentBranch, refOverride) + } + + runPushLog.Printf("Current branch matches --ref value: %s", currentBranch) + if verbose { + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Verified current branch matches --ref: %s", currentBranch))) + } + } // Get the list of staged files stagedFiles := strings.Split(strings.TrimSpace(string(statusOutput)), "\n") diff --git a/pkg/cli/run_push_test.go b/pkg/cli/run_push_test.go index 44b4a725a26..85b030de7e5 100644 --- a/pkg/cli/run_push_test.go +++ b/pkg/cli/run_push_test.go @@ -376,7 +376,7 @@ workflowFile := filepath.Join(tmpDir, "workflow.md") err = os.WriteFile(workflowFile, []byte("# Test"), 0644) require.NoError(t, err) -err = pushWorkflowFiles("test-workflow", []string{workflowFile}, false) +err = pushWorkflowFiles("test-workflow", []string{workflowFile}, "", false) // Should return an error about staged files assert.Error(t, err) From 622d74037a243b149b609082d5a5fe0043bbb820 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 5 Jan 2026 16:08:33 +0000 Subject: [PATCH 17/18] Run gofmt to fix formatting issues Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/run_push.go | 46 ++--- pkg/cli/run_push_test.go | 154 +++++++-------- pkg/console/accessibility_test.go | 2 +- pkg/console/confirm.go | 2 +- pkg/console/confirm_test.go | 2 +- pkg/workflow/action_pins_logging_test.go | 176 +++++++++--------- pkg/workflow/compiler_safe_outputs_config.go | 6 +- .../compiler_safe_outputs_config_test.go | 12 +- pkg/workflow/compiler_safe_outputs_core.go | 26 +-- pkg/workflow/compiler_safe_outputs_env.go | 2 +- .../compiler_safe_outputs_env_test.go | 14 +- .../compiler_safe_outputs_job_test.go | 30 +-- .../compiler_safe_outputs_steps_test.go | 26 +-- 13 files changed, 248 insertions(+), 250 deletions(-) diff --git a/pkg/cli/run_push.go b/pkg/cli/run_push.go index a5cc422c35e..f4a5ebb0540 100644 --- a/pkg/cli/run_push.go +++ b/pkg/cli/run_push.go @@ -40,7 +40,7 @@ func collectWorkflowFiles(workflowPath string, verbose bool) ([]string, error) { lockFilePath := strings.TrimSuffix(absWorkflowPath, ".md") + ".lock.yml" runPushLog.Printf("Checking lock file: %s", lockFilePath) needsRecompile := false - + if lockStat, err := os.Stat(lockFilePath); err == nil { runPushLog.Printf("Lock file exists: %s", lockFilePath) // Lock file exists - check if it's outdated @@ -104,7 +104,7 @@ func collectWorkflowFiles(workflowPath string, verbose bool) ([]string, error) { for file := range files { result = append(result, file) } - + // Sort files for stable output sort.Strings(result) runPushLog.Printf("Sorted %d files for stable output", len(result)) @@ -116,7 +116,7 @@ func collectWorkflowFiles(workflowPath string, verbose bool) ([]string, error) { // recompileWorkflow compiles a workflow using CompileWorkflows func recompileWorkflow(workflowPath string, verbose bool) error { runPushLog.Printf("Recompiling workflow: %s", workflowPath) - + config := CompileConfig{ MarkdownFiles: []string{workflowPath}, Verbose: verbose, @@ -131,9 +131,9 @@ func recompileWorkflow(workflowPath string, verbose bool) error { TrialLogicalRepoSlug: "", Strict: false, } - + runPushLog.Printf("Compilation config: Validate=%v, NoEmit=%v", config.Validate, config.NoEmit) - + // Use background context for compilation ctx := context.Background() runPushLog.Printf("Starting compilation with CompileWorkflows") @@ -141,7 +141,7 @@ func recompileWorkflow(workflowPath string, verbose bool) error { runPushLog.Printf("Compilation failed: %v", err) return fmt.Errorf("compilation failed: %w", err) } - + runPushLog.Printf("Successfully recompiled workflow: %s", workflowPath) return nil } @@ -156,7 +156,7 @@ type LockFileStatus struct { // checkLockFileStatus checks the status of a workflow's lock file func checkLockFileStatus(workflowPath string) (*LockFileStatus, error) { runPushLog.Printf("Checking lock file status for: %s", workflowPath) - + // Get absolute path for the workflow absWorkflowPath, err := filepath.Abs(workflowPath) if err != nil { @@ -164,13 +164,13 @@ func checkLockFileStatus(workflowPath string) (*LockFileStatus, error) { return nil, fmt.Errorf("failed to get absolute path for workflow: %w", err) } runPushLog.Printf("Resolved absolute path: %s", absWorkflowPath) - + lockFilePath := strings.TrimSuffix(absWorkflowPath, ".md") + ".lock.yml" runPushLog.Printf("Expected lock file path: %s", lockFilePath) status := &LockFileStatus{ LockPath: lockFilePath, } - + // Check if lock file exists lockStat, err := os.Stat(lockFilePath) if err != nil { @@ -183,7 +183,7 @@ func checkLockFileStatus(workflowPath string) (*LockFileStatus, error) { return nil, fmt.Errorf("failed to stat lock file: %w", err) } runPushLog.Printf("Lock file exists: %s (modtime: %v)", lockFilePath, lockStat.ModTime()) - + // Lock file exists - check if it's outdated mdStat, err := os.Stat(absWorkflowPath) if err != nil { @@ -191,14 +191,14 @@ func checkLockFileStatus(workflowPath string) (*LockFileStatus, error) { return nil, fmt.Errorf("failed to stat workflow file: %w", err) } runPushLog.Printf("Workflow file modtime: %v", mdStat.ModTime()) - + if mdStat.ModTime().After(lockStat.ModTime()) { status.Outdated = true runPushLog.Printf("Lock file outdated (md: %v, lock: %v)", mdStat.ModTime(), lockStat.ModTime()) } else { runPushLog.Printf("Lock file is up-to-date") } - + return status, nil } @@ -280,7 +280,7 @@ func collectImports(workflowPath string, files map[string]bool, visited map[stri // Process each import for i, importPath := range imports { runPushLog.Printf("Processing import %d/%d: %s", i+1, len(imports), importPath) - + // Resolve the import path resolvedPath := resolveImportPathLocal(importPath, workflowDir) if resolvedPath == "" { @@ -332,7 +332,7 @@ func collectImports(workflowPath string, files map[string]bool, visited map[stri // This is needed to avoid circular dependencies with imports.go func resolveImportPathLocal(importPath, baseDir string) string { runPushLog.Printf("Resolving import path: %s (baseDir: %s)", importPath, baseDir) - + // Handle section references (file.md#Section) - strip the section part if strings.Contains(importPath, "#") { parts := strings.SplitN(importPath, "#", 2) @@ -370,7 +370,7 @@ func resolveImportPathLocal(importPath, baseDir string) string { // This is duplicated from imports.go to avoid circular dependencies func isWorkflowSpecFormatLocal(path string) bool { runPushLog.Printf("Checking if workflowspec format: %s", path) - + // Check if it contains @ (ref separator) or looks like owner/repo/path if strings.Contains(path, "@") { runPushLog.Printf("Path contains @ - workflowspec format: %s", path) @@ -440,7 +440,7 @@ func pushWorkflowFiles(workflowName string, files []string, refOverride string, runPushLog.Print("No changes to commit") return nil } - + // Now that we know there are changes to commit, check that current branch matches --ref value if specified // This happens after we've determined there are actual changes, so we don't fail unnecessarily if refOverride != "" { @@ -451,12 +451,12 @@ func pushWorkflowFiles(workflowName string, files []string, refOverride string, return fmt.Errorf("failed to determine current branch: %w", err) } runPushLog.Printf("Current branch: %s", currentBranch) - + if currentBranch != refOverride { runPushLog.Printf("Current branch (%s) does not match --ref value (%s)", currentBranch, refOverride) return fmt.Errorf("--push requires the current branch (%s) to match the --ref value (%s). Switching branches is not supported. Please checkout the target branch first", currentBranch, refOverride) } - + runPushLog.Printf("Current branch matches --ref value: %s", currentBranch) if verbose { fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Verified current branch matches --ref: %s", currentBranch))) @@ -466,7 +466,7 @@ func pushWorkflowFiles(workflowName string, files []string, refOverride string, // Get the list of staged files stagedFiles := strings.Split(strings.TrimSpace(string(statusOutput)), "\n") runPushLog.Printf("Found %d staged files: %v", len(stagedFiles), stagedFiles) - + // Check if there are staged files beyond what we just staged // Convert our files list to a map for quick lookup runPushLog.Printf("Building map of our files for comparison") @@ -483,7 +483,7 @@ func pushWorkflowFiles(workflowName string, files []string, refOverride string, ourFiles[file] = true runPushLog.Printf("Added to our files map: %s", file) } - + // Check if there are any staged files that aren't in our list runPushLog.Printf("Checking for extra staged files not in our list") var extraStagedFiles []string @@ -502,11 +502,11 @@ func pushWorkflowFiles(workflowName string, files []string, refOverride string, runPushLog.Printf("Extra staged file detected: %s", stagedFile) extraStagedFiles = append(extraStagedFiles, stagedFile) } - + // If there are extra staged files that we didn't stage, give up if len(extraStagedFiles) > 0 { runPushLog.Printf("Found %d extra staged files not in our list, refusing to proceed: %v", len(extraStagedFiles), extraStagedFiles) - + fmt.Fprintln(os.Stderr, "") fmt.Fprintln(os.Stderr, console.FormatErrorMessage("Cannot proceed: there are already staged files in git that are not part of this workflow")) fmt.Fprintln(os.Stderr, "") @@ -517,7 +517,7 @@ func pushWorkflowFiles(workflowName string, files []string, refOverride string, fmt.Fprintln(os.Stderr, "") fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Please commit or unstage these files before using --push")) fmt.Fprintln(os.Stderr, "") - + return fmt.Errorf("git has staged files not part of workflow - commit or unstage them before using --push") } runPushLog.Printf("No extra staged files detected - all staged files are part of our workflow") diff --git a/pkg/cli/run_push_test.go b/pkg/cli/run_push_test.go index 85b030de7e5..a2ae92bf19a 100644 --- a/pkg/cli/run_push_test.go +++ b/pkg/cli/run_push_test.go @@ -188,7 +188,7 @@ This is a test workflow without a lock file. fileSet[file] = true } assert.True(t, fileSet[workflowPath], "Should include workflow .md file") - + lockFilePath := strings.TrimSuffix(workflowPath, ".md") + ".lock.yml" assert.True(t, fileSet[lockFilePath], "Should include auto-generated lock .yml file") } @@ -292,93 +292,93 @@ func TestResolveImportPathLocal(t *testing.T) { } func TestCollectWorkflowFiles_WithOutdatedLockFile(t *testing.T) { -// Create a temporary directory for testing -tmpDir := t.TempDir() + // Create a temporary directory for testing + tmpDir := t.TempDir() -// Create a workflow file -workflowPath := filepath.Join(tmpDir, "test-workflow.md") -workflowContent := `--- + // Create a workflow file + workflowPath := filepath.Join(tmpDir, "test-workflow.md") + workflowContent := `--- name: Test Workflow on: workflow_dispatch --- # Test Workflow This is a test workflow. ` -err := os.WriteFile(workflowPath, []byte(workflowContent), 0644) -require.NoError(t, err) + err := os.WriteFile(workflowPath, []byte(workflowContent), 0644) + require.NoError(t, err) -// Create an old lock file (simulate outdated) -lockFilePath := filepath.Join(tmpDir, "test-workflow.lock.yml") -lockContent := `name: Test Workflow + // Create an old lock file (simulate outdated) + lockFilePath := filepath.Join(tmpDir, "test-workflow.lock.yml") + lockContent := `name: Test Workflow on: workflow_dispatch ` -err = os.WriteFile(lockFilePath, []byte(lockContent), 0644) -require.NoError(t, err) - -// Make the workflow file newer by sleeping and touching it -time.Sleep(100 * time.Millisecond) -currentTime := time.Now() -err = os.Chtimes(workflowPath, currentTime, currentTime) -require.NoError(t, err) - -// Verify the lock file is older -mdStat, err := os.Stat(workflowPath) -require.NoError(t, err) -lockStat, err := os.Stat(lockFilePath) -require.NoError(t, err) -assert.True(t, mdStat.ModTime().After(lockStat.ModTime()), "Workflow file should be newer than lock file") - -// Note: We can't actually test recompilation here without a full compilation setup, -// but we can verify the detection logic works -// The actual compilation would happen in an integration test + err = os.WriteFile(lockFilePath, []byte(lockContent), 0644) + require.NoError(t, err) + + // Make the workflow file newer by sleeping and touching it + time.Sleep(100 * time.Millisecond) + currentTime := time.Now() + err = os.Chtimes(workflowPath, currentTime, currentTime) + require.NoError(t, err) + + // Verify the lock file is older + mdStat, err := os.Stat(workflowPath) + require.NoError(t, err) + lockStat, err := os.Stat(lockFilePath) + require.NoError(t, err) + assert.True(t, mdStat.ModTime().After(lockStat.ModTime()), "Workflow file should be newer than lock file") + + // Note: We can't actually test recompilation here without a full compilation setup, + // but we can verify the detection logic works + // The actual compilation would happen in an integration test } func TestPushWorkflowFiles_WithStagedFiles(t *testing.T) { -// Create a temporary directory for testing -tmpDir := t.TempDir() - -// Initialize a git repo -cmd := exec.Command("git", "init") -cmd.Dir = tmpDir -err := cmd.Run() -require.NoError(t, err) - -// Configure git -cmd = exec.Command("git", "config", "user.email", "test@example.com") -cmd.Dir = tmpDir -err = cmd.Run() -require.NoError(t, err) - -cmd = exec.Command("git", "config", "user.name", "Test User") -cmd.Dir = tmpDir -err = cmd.Run() -require.NoError(t, err) - -// Create a test file and stage it -testFile := filepath.Join(tmpDir, "test-file.txt") -err = os.WriteFile(testFile, []byte("test content"), 0644) -require.NoError(t, err) - -cmd = exec.Command("git", "add", "test-file.txt") -cmd.Dir = tmpDir -err = cmd.Run() -require.NoError(t, err) - -// Save current directory and change to tmpDir -originalDir, err := os.Getwd() -require.NoError(t, err) -err = os.Chdir(tmpDir) -require.NoError(t, err) -defer os.Chdir(originalDir) - -// Try to push workflow files - should fail due to staged files -workflowFile := filepath.Join(tmpDir, "workflow.md") -err = os.WriteFile(workflowFile, []byte("# Test"), 0644) -require.NoError(t, err) - -err = pushWorkflowFiles("test-workflow", []string{workflowFile}, "", false) - -// Should return an error about staged files -assert.Error(t, err) -assert.Contains(t, err.Error(), "staged files") + // Create a temporary directory for testing + tmpDir := t.TempDir() + + // Initialize a git repo + cmd := exec.Command("git", "init") + cmd.Dir = tmpDir + err := cmd.Run() + require.NoError(t, err) + + // Configure git + cmd = exec.Command("git", "config", "user.email", "test@example.com") + cmd.Dir = tmpDir + err = cmd.Run() + require.NoError(t, err) + + cmd = exec.Command("git", "config", "user.name", "Test User") + cmd.Dir = tmpDir + err = cmd.Run() + require.NoError(t, err) + + // Create a test file and stage it + testFile := filepath.Join(tmpDir, "test-file.txt") + err = os.WriteFile(testFile, []byte("test content"), 0644) + require.NoError(t, err) + + cmd = exec.Command("git", "add", "test-file.txt") + cmd.Dir = tmpDir + err = cmd.Run() + require.NoError(t, err) + + // Save current directory and change to tmpDir + originalDir, err := os.Getwd() + require.NoError(t, err) + err = os.Chdir(tmpDir) + require.NoError(t, err) + defer os.Chdir(originalDir) + + // Try to push workflow files - should fail due to staged files + workflowFile := filepath.Join(tmpDir, "workflow.md") + err = os.WriteFile(workflowFile, []byte("# Test"), 0644) + require.NoError(t, err) + + err = pushWorkflowFiles("test-workflow", []string{workflowFile}, "", false) + + // Should return an error about staged files + assert.Error(t, err) + assert.Contains(t, err.Error(), "staged files") } diff --git a/pkg/console/accessibility_test.go b/pkg/console/accessibility_test.go index 59a818f9f0a..914e7154bba 100644 --- a/pkg/console/accessibility_test.go +++ b/pkg/console/accessibility_test.go @@ -44,7 +44,7 @@ func TestIsAccessibleMode(t *testing.T) { origAccessible := os.Getenv("ACCESSIBLE") origTerm := os.Getenv("TERM") origNoColor := os.Getenv("NO_COLOR") - + // Clean up after test defer func() { if origAccessible != "" { diff --git a/pkg/console/confirm.go b/pkg/console/confirm.go index 440752c8ce2..26f8c3e9539 100644 --- a/pkg/console/confirm.go +++ b/pkg/console/confirm.go @@ -8,7 +8,7 @@ import ( // Returns true if the user confirms, false if they cancel or an error occurs func ConfirmAction(title, affirmative, negative string) (bool, error) { var confirmed bool - + confirmForm := huh.NewForm( huh.NewGroup( huh.NewConfirm(). diff --git a/pkg/console/confirm_test.go b/pkg/console/confirm_test.go index 748730f630b..86d8086ccdc 100644 --- a/pkg/console/confirm_test.go +++ b/pkg/console/confirm_test.go @@ -7,7 +7,7 @@ import ( func TestConfirmAction(t *testing.T) { // Note: This test can't fully test the interactive behavior without mocking // the terminal input, but we can verify the function signature and basic setup - + t.Run("function signature", func(t *testing.T) { // This test just verifies the function exists and has the right signature // Actual interactive testing would require a mock terminal diff --git a/pkg/workflow/action_pins_logging_test.go b/pkg/workflow/action_pins_logging_test.go index 9e9f3549c67..f395799c622 100644 --- a/pkg/workflow/action_pins_logging_test.go +++ b/pkg/workflow/action_pins_logging_test.go @@ -174,106 +174,106 @@ func TestActionPinResolutionWithStrictMode(t *testing.T) { // TestActionCacheDuplicateSHAWarning verifies that we log warnings when multiple // version references resolve to the same SHA, which can cause version comment flipping func TestActionCacheDuplicateSHAWarning(t *testing.T) { -// Create a test cache with one entry -cache := &ActionCache{ -Entries: map[string]ActionCacheEntry{ -"actions/github-script@v8": { -Repo: "actions/github-script", -Version: "v8", -SHA: "ed597411d8f924073f98dfc5c65a23a2325f34cd", -}, -}, -path: "/tmp/test-cache.json", -} + // Create a test cache with one entry + cache := &ActionCache{ + Entries: map[string]ActionCacheEntry{ + "actions/github-script@v8": { + Repo: "actions/github-script", + Version: "v8", + SHA: "ed597411d8f924073f98dfc5c65a23a2325f34cd", + }, + }, + path: "/tmp/test-cache.json", + } -// Add a second entry with the same SHA but different version -cache.Set("actions/github-script", "v8.0.0", "ed597411d8f924073f98dfc5c65a23a2325f34cd") + // Add a second entry with the same SHA but different version + cache.Set("actions/github-script", "v8.0.0", "ed597411d8f924073f98dfc5c65a23a2325f34cd") -// Verify both entries are in the cache -if len(cache.Entries) != 2 { -t.Errorf("Expected 2 cache entries, got %d", len(cache.Entries)) -} + // Verify both entries are in the cache + if len(cache.Entries) != 2 { + t.Errorf("Expected 2 cache entries, got %d", len(cache.Entries)) + } -// Verify both have the same SHA (this is what causes the issue) -v8Entry := cache.Entries["actions/github-script@v8"] -v800Entry := cache.Entries["actions/github-script@v8.0.0"] -if v8Entry.SHA != v800Entry.SHA { -t.Error("Expected both entries to have the same SHA") -} + // Verify both have the same SHA (this is what causes the issue) + v8Entry := cache.Entries["actions/github-script@v8"] + v800Entry := cache.Entries["actions/github-script@v8.0.0"] + if v8Entry.SHA != v800Entry.SHA { + t.Error("Expected both entries to have the same SHA") + } -t.Logf("Cache has duplicate SHA entries with different versions:") -t.Logf(" v8: %s", v8Entry.SHA[:8]) -t.Logf(" v8.0.0: %s", v800Entry.SHA[:8]) -t.Logf("This configuration causes version comment flipping in lock files") + t.Logf("Cache has duplicate SHA entries with different versions:") + t.Logf(" v8: %s", v8Entry.SHA[:8]) + t.Logf(" v8.0.0: %s", v800Entry.SHA[:8]) + t.Logf("This configuration causes version comment flipping in lock files") } // TestDeduplicationRemovesLessPreciseVersions verifies that deduplication // keeps the most precise version and logs detailed information func TestDeduplicationRemovesLessPreciseVersions(t *testing.T) { -tests := []struct { -name string -entries map[string]ActionCacheEntry -expectedKeep string -expectedRemoveCount int -}{ -{ -name: "v8.0.0 is kept over v8", -entries: map[string]ActionCacheEntry{ -"actions/github-script@v8": { -Repo: "actions/github-script", -Version: "v8", -SHA: "ed597411d8f924073f98dfc5c65a23a2325f34cd", -}, -"actions/github-script@v8.0.0": { -Repo: "actions/github-script", -Version: "v8.0.0", -SHA: "ed597411d8f924073f98dfc5c65a23a2325f34cd", -}, -}, -expectedKeep: "actions/github-script@v8.0.0", -expectedRemoveCount: 1, -}, -{ -name: "v6.1.0 is kept over v6", -entries: map[string]ActionCacheEntry{ -"actions/setup-node@v6": { -Repo: "actions/setup-node", -Version: "v6", -SHA: "395ad3262231945c25e8478fd5baf05154b1d79f", -}, -"actions/setup-node@v6.1.0": { -Repo: "actions/setup-node", -Version: "v6.1.0", -SHA: "395ad3262231945c25e8478fd5baf05154b1d79f", -}, -}, -expectedKeep: "actions/setup-node@v6.1.0", -expectedRemoveCount: 1, -}, -} + tests := []struct { + name string + entries map[string]ActionCacheEntry + expectedKeep string + expectedRemoveCount int + }{ + { + name: "v8.0.0 is kept over v8", + entries: map[string]ActionCacheEntry{ + "actions/github-script@v8": { + Repo: "actions/github-script", + Version: "v8", + SHA: "ed597411d8f924073f98dfc5c65a23a2325f34cd", + }, + "actions/github-script@v8.0.0": { + Repo: "actions/github-script", + Version: "v8.0.0", + SHA: "ed597411d8f924073f98dfc5c65a23a2325f34cd", + }, + }, + expectedKeep: "actions/github-script@v8.0.0", + expectedRemoveCount: 1, + }, + { + name: "v6.1.0 is kept over v6", + entries: map[string]ActionCacheEntry{ + "actions/setup-node@v6": { + Repo: "actions/setup-node", + Version: "v6", + SHA: "395ad3262231945c25e8478fd5baf05154b1d79f", + }, + "actions/setup-node@v6.1.0": { + Repo: "actions/setup-node", + Version: "v6.1.0", + SHA: "395ad3262231945c25e8478fd5baf05154b1d79f", + }, + }, + expectedKeep: "actions/setup-node@v6.1.0", + expectedRemoveCount: 1, + }, + } -for _, tt := range tests { -t.Run(tt.name, func(t *testing.T) { -cache := &ActionCache{ -Entries: tt.entries, -path: "/tmp/test-cache.json", -} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cache := &ActionCache{ + Entries: tt.entries, + path: "/tmp/test-cache.json", + } -initialCount := len(cache.Entries) -cache.deduplicateEntries() + initialCount := len(cache.Entries) + cache.deduplicateEntries() -if _, exists := cache.Entries[tt.expectedKeep]; !exists { -t.Errorf("Expected entry %s to be kept, but it was removed", tt.expectedKeep) -} + if _, exists := cache.Entries[tt.expectedKeep]; !exists { + t.Errorf("Expected entry %s to be kept, but it was removed", tt.expectedKeep) + } -removed := initialCount - len(cache.Entries) -if removed != tt.expectedRemoveCount { -t.Errorf("Expected %d entries to be removed, but %d were removed", -tt.expectedRemoveCount, removed) -} + removed := initialCount - len(cache.Entries) + if removed != tt.expectedRemoveCount { + t.Errorf("Expected %d entries to be removed, but %d were removed", + tt.expectedRemoveCount, removed) + } -t.Logf("Deduplication kept %s, removed %d less precise entries", -tt.expectedKeep, removed) -}) -} + t.Logf("Deduplication kept %s, removed %d less precise entries", + tt.expectedKeep, removed) + }) + } } diff --git a/pkg/workflow/compiler_safe_outputs_config.go b/pkg/workflow/compiler_safe_outputs_config.go index d1167e9e819..39932c192b0 100644 --- a/pkg/workflow/compiler_safe_outputs_config.go +++ b/pkg/workflow/compiler_safe_outputs_config.go @@ -1,10 +1,10 @@ package workflow import ( -"encoding/json" -"fmt" + "encoding/json" + "fmt" -"github.com/githubnext/gh-aw/pkg/logger" + "github.com/githubnext/gh-aw/pkg/logger" ) var consolidatedSafeOutputsConfigLog = logger.New("workflow:compiler_safe_outputs_config") diff --git a/pkg/workflow/compiler_safe_outputs_config_test.go b/pkg/workflow/compiler_safe_outputs_config_test.go index 68229838fd4..523d6ed4284 100644 --- a/pkg/workflow/compiler_safe_outputs_config_test.go +++ b/pkg/workflow/compiler_safe_outputs_config_test.go @@ -278,7 +278,6 @@ func TestHandlerConfigMaxValues(t *testing.T) { var steps []string compiler.addHandlerManagerConfigEnvVar(&steps, workflowData) - // Extract and validate JSON for _, step := range steps { if strings.Contains(step, "GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG") { @@ -319,7 +318,6 @@ func TestHandlerConfigAllowedLabels(t *testing.T) { var steps []string compiler.addHandlerManagerConfigEnvVar(&steps, workflowData) - // Extract and validate JSON for _, step := range steps { if strings.Contains(step, "GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG") { @@ -438,8 +436,8 @@ func TestHandlerConfigBooleanFields(t *testing.T) { // TestHandlerConfigUpdateFields tests update field configurations func TestHandlerConfigUpdateFields(t *testing.T) { tests := []struct { - name string - config *UpdateIssuesConfig + name string + config *UpdateIssuesConfig expectedKeys []string }{ { @@ -568,9 +566,9 @@ func TestHandlerConfigTargetRepo(t *testing.T) { // TestHandlerConfigPatchSize tests max patch size configuration func TestHandlerConfigPatchSize(t *testing.T) { tests := []struct { - name string - maxPatchSize int - expectedSize int + name string + maxPatchSize int + expectedSize int }{ { name: "default patch size", diff --git a/pkg/workflow/compiler_safe_outputs_core.go b/pkg/workflow/compiler_safe_outputs_core.go index 9b9686e6639..4181a7ce374 100644 --- a/pkg/workflow/compiler_safe_outputs_core.go +++ b/pkg/workflow/compiler_safe_outputs_core.go @@ -1,7 +1,7 @@ package workflow import ( -"github.com/githubnext/gh-aw/pkg/logger" + "github.com/githubnext/gh-aw/pkg/logger" ) var consolidatedSafeOutputsLog = logger.New("workflow:compiler_safe_outputs_consolidated") @@ -9,18 +9,18 @@ var consolidatedSafeOutputsLog = logger.New("workflow:compiler_safe_outputs_cons // SafeOutputStepConfig holds configuration for building a single safe output step // within the consolidated safe-outputs job type SafeOutputStepConfig struct { -StepName string // Human-readable step name (e.g., "Create Issue") -StepID string // Step ID for referencing outputs (e.g., "create_issue") -Script string // JavaScript script to execute (for inline mode) -ScriptName string // Name of the script in the registry (for file mode) -CustomEnvVars []string // Environment variables specific to this step -Condition ConditionNode // Step-level condition (if clause) -Token string // GitHub token for this step -UseCopilotToken bool // Whether to use Copilot token preference chain -UseAgentToken bool // Whether to use agent token preference chain -PreSteps []string // Optional steps to run before the script step -PostSteps []string // Optional steps to run after the script step -Outputs map[string]string // Outputs from this step + StepName string // Human-readable step name (e.g., "Create Issue") + StepID string // Step ID for referencing outputs (e.g., "create_issue") + Script string // JavaScript script to execute (for inline mode) + ScriptName string // Name of the script in the registry (for file mode) + CustomEnvVars []string // Environment variables specific to this step + Condition ConditionNode // Step-level condition (if clause) + Token string // GitHub token for this step + UseCopilotToken bool // Whether to use Copilot token preference chain + UseAgentToken bool // Whether to use agent token preference chain + PreSteps []string // Optional steps to run before the script step + PostSteps []string // Optional steps to run after the script step + Outputs map[string]string // Outputs from this step } // Note: The implementation functions have been moved to focused module files: diff --git a/pkg/workflow/compiler_safe_outputs_env.go b/pkg/workflow/compiler_safe_outputs_env.go index 99dcc41fe69..36ff94d4fd9 100644 --- a/pkg/workflow/compiler_safe_outputs_env.go +++ b/pkg/workflow/compiler_safe_outputs_env.go @@ -1,7 +1,7 @@ package workflow import ( -"github.com/githubnext/gh-aw/pkg/logger" + "github.com/githubnext/gh-aw/pkg/logger" ) var consolidatedSafeOutputsEnvLog = logger.New("workflow:compiler_safe_outputs_env") diff --git a/pkg/workflow/compiler_safe_outputs_env_test.go b/pkg/workflow/compiler_safe_outputs_env_test.go index f91f5c021f1..9e4256125b4 100644 --- a/pkg/workflow/compiler_safe_outputs_env_test.go +++ b/pkg/workflow/compiler_safe_outputs_env_test.go @@ -11,10 +11,10 @@ import ( // TestAddAllSafeOutputConfigEnvVars tests environment variable generation for all safe output types func TestAddAllSafeOutputConfigEnvVars(t *testing.T) { tests := []struct { - name string - safeOutputs *SafeOutputsConfig - trialMode bool - checkContains []string + name string + safeOutputs *SafeOutputsConfig + trialMode bool + checkContains []string checkNotContains []string }{ { @@ -70,7 +70,7 @@ func TestAddAllSafeOutputConfigEnvVars(t *testing.T) { { name: "update issues with staged flag", safeOutputs: &SafeOutputsConfig{ - Staged: true, + Staged: true, UpdateIssues: &UpdateIssuesConfig{}, }, checkContains: []string{ @@ -80,7 +80,7 @@ func TestAddAllSafeOutputConfigEnvVars(t *testing.T) { { name: "update discussions with staged flag", safeOutputs: &SafeOutputsConfig{ - Staged: true, + Staged: true, UpdateDiscussions: &UpdateDiscussionsConfig{}, }, checkContains: []string{ @@ -306,7 +306,7 @@ func TestEnvVarsWithMultipleSafeOutputTypes(t *testing.T) { AddLabels: &AddLabelsConfig{ Allowed: []string{"bug", "enhancement"}, }, - UpdateIssues: &UpdateIssuesConfig{}, + UpdateIssues: &UpdateIssuesConfig{}, UpdateDiscussions: &UpdateDiscussionsConfig{}, }, } diff --git a/pkg/workflow/compiler_safe_outputs_job_test.go b/pkg/workflow/compiler_safe_outputs_job_test.go index ebbef988627..d9c3804cefe 100644 --- a/pkg/workflow/compiler_safe_outputs_job_test.go +++ b/pkg/workflow/compiler_safe_outputs_job_test.go @@ -12,14 +12,14 @@ import ( // TestBuildConsolidatedSafeOutputsJob tests the main job builder function func TestBuildConsolidatedSafeOutputsJob(t *testing.T) { tests := []struct { - name string - safeOutputs *SafeOutputsConfig - threatDetection bool - expectedJobName string - expectedSteps int - expectNil bool - checkPermissions bool - expectedPerms []string + name string + safeOutputs *SafeOutputsConfig + threatDetection bool + expectedJobName string + expectedSteps int + expectNil bool + checkPermissions bool + expectedPerms []string }{ { name: "no safe outputs configured", @@ -162,12 +162,12 @@ func TestBuildConsolidatedSafeOutputsJob(t *testing.T) { // TestBuildJobLevelSafeOutputEnvVars tests job-level environment variable generation func TestBuildJobLevelSafeOutputEnvVars(t *testing.T) { tests := []struct { - name string - workflowData *WorkflowData - workflowID string - trialMode bool - trialRepo string - expectedVars map[string]string + name string + workflowData *WorkflowData + workflowID string + trialMode bool + trialRepo string + expectedVars map[string]string checkContains bool }{ { @@ -343,7 +343,7 @@ func TestJobWithGitHubApp(t *testing.T) { Name: "Test Workflow", SafeOutputs: &SafeOutputsConfig{ App: &GitHubAppConfig{ - AppID: "12345", + AppID: "12345", PrivateKey: "test-key", }, CreateIssues: &CreateIssuesConfig{ diff --git a/pkg/workflow/compiler_safe_outputs_steps_test.go b/pkg/workflow/compiler_safe_outputs_steps_test.go index 0ed5e3bf72e..7c7420a3f70 100644 --- a/pkg/workflow/compiler_safe_outputs_steps_test.go +++ b/pkg/workflow/compiler_safe_outputs_steps_test.go @@ -11,18 +11,18 @@ import ( // TestBuildConsolidatedSafeOutputStep tests individual step building func TestBuildConsolidatedSafeOutputStep(t *testing.T) { tests := []struct { - name string - config SafeOutputStepConfig - checkContains []string + name string + config SafeOutputStepConfig + checkContains []string checkNotContains []string }{ { name: "basic step with inline script", config: SafeOutputStepConfig{ - StepName: "Test Step", - StepID: "test_step", - Script: "console.log('test');", - Token: "${{ github.token }}", + StepName: "Test Step", + StepID: "test_step", + Script: "console.log('test');", + Token: "${{ github.token }}", }, checkContains: []string{ "name: Test Step", @@ -55,11 +55,11 @@ func TestBuildConsolidatedSafeOutputStep(t *testing.T) { { name: "step with condition", config: SafeOutputStepConfig{ - StepName: "Conditional Step", - StepID: "conditional", - Script: "console.log('test');", - Token: "${{ github.token }}", - Condition: BuildEquals(BuildStringLiteral("test"), BuildStringLiteral("test")), + StepName: "Conditional Step", + StepID: "conditional", + Script: "console.log('test');", + Token: "${{ github.token }}", + Condition: BuildEquals(BuildStringLiteral("test"), BuildStringLiteral("test")), }, checkContains: []string{ "if: 'test' == 'test'", @@ -186,7 +186,7 @@ func TestBuildSharedPRCheckoutSteps(t *testing.T) { name: "with GitHub App token", safeOutputs: &SafeOutputsConfig{ App: &GitHubAppConfig{ - AppID: "12345", + AppID: "12345", PrivateKey: "test-key", }, CreatePullRequests: &CreatePullRequestsConfig{}, From dbdf37b23ff06202bed8185614066dc9901db02f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 5 Jan 2026 16:23:22 +0000 Subject: [PATCH 18/18] Fix all linting issues: testifylint, unconvert, and unused vars Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/run_push_test.go | 2 +- pkg/workflow/compiler_safe_outputs_config.go | 4 ---- pkg/workflow/compiler_safe_outputs_config_test.go | 4 ++-- pkg/workflow/compiler_safe_outputs_env.go | 6 ------ pkg/workflow/compiler_safe_outputs_job_test.go | 4 ++-- 5 files changed, 5 insertions(+), 15 deletions(-) diff --git a/pkg/cli/run_push_test.go b/pkg/cli/run_push_test.go index a2ae92bf19a..57692a1c5f4 100644 --- a/pkg/cli/run_push_test.go +++ b/pkg/cli/run_push_test.go @@ -379,6 +379,6 @@ func TestPushWorkflowFiles_WithStagedFiles(t *testing.T) { err = pushWorkflowFiles("test-workflow", []string{workflowFile}, "", false) // Should return an error about staged files - assert.Error(t, err) + require.Error(t, err) assert.Contains(t, err.Error(), "staged files") } diff --git a/pkg/workflow/compiler_safe_outputs_config.go b/pkg/workflow/compiler_safe_outputs_config.go index 39932c192b0..d4aafb77c4d 100644 --- a/pkg/workflow/compiler_safe_outputs_config.go +++ b/pkg/workflow/compiler_safe_outputs_config.go @@ -3,12 +3,8 @@ package workflow import ( "encoding/json" "fmt" - - "github.com/githubnext/gh-aw/pkg/logger" ) -var consolidatedSafeOutputsConfigLog = logger.New("workflow:compiler_safe_outputs_config") - func (c *Compiler) addHandlerManagerConfigEnvVar(steps *[]string, data *WorkflowData) { if data.SafeOutputs == nil { return diff --git a/pkg/workflow/compiler_safe_outputs_config_test.go b/pkg/workflow/compiler_safe_outputs_config_test.go index 523d6ed4284..f312df285a4 100644 --- a/pkg/workflow/compiler_safe_outputs_config_test.go +++ b/pkg/workflow/compiler_safe_outputs_config_test.go @@ -296,7 +296,7 @@ func TestHandlerConfigMaxValues(t *testing.T) { maxVal, ok := issueConfig["max"] require.True(t, ok) - assert.Equal(t, float64(10), maxVal) + assert.InDelta(t, float64(10), maxVal, 0.0001) } } } @@ -617,7 +617,7 @@ func TestHandlerConfigPatchSize(t *testing.T) { maxSize, ok := prConfig["max_patch_size"] require.True(t, ok) - assert.Equal(t, float64(tt.expectedSize), maxSize) + assert.InDelta(t, float64(tt.expectedSize), maxSize, 0.0001) } } } diff --git a/pkg/workflow/compiler_safe_outputs_env.go b/pkg/workflow/compiler_safe_outputs_env.go index 36ff94d4fd9..d26d5695c58 100644 --- a/pkg/workflow/compiler_safe_outputs_env.go +++ b/pkg/workflow/compiler_safe_outputs_env.go @@ -1,11 +1,5 @@ package workflow -import ( - "github.com/githubnext/gh-aw/pkg/logger" -) - -var consolidatedSafeOutputsEnvLog = logger.New("workflow:compiler_safe_outputs_env") - func (c *Compiler) addAllSafeOutputConfigEnvVars(steps *[]string, data *WorkflowData) { if data.SafeOutputs == nil { return diff --git a/pkg/workflow/compiler_safe_outputs_job_test.go b/pkg/workflow/compiler_safe_outputs_job_test.go index d9c3804cefe..e81cf2ddb25 100644 --- a/pkg/workflow/compiler_safe_outputs_job_test.go +++ b/pkg/workflow/compiler_safe_outputs_job_test.go @@ -121,7 +121,7 @@ func TestBuildConsolidatedSafeOutputsJob(t *testing.T) { SafeOutputs: tt.safeOutputs, } - job, stepNames, err := compiler.buildConsolidatedSafeOutputsJob(workflowData, string(string(constants.AgentJobName)), "test-workflow.md") + job, stepNames, err := compiler.buildConsolidatedSafeOutputsJob(workflowData, string(constants.AgentJobName), "test-workflow.md") if tt.expectNil { assert.Nil(t, job) @@ -137,7 +137,7 @@ func TestBuildConsolidatedSafeOutputsJob(t *testing.T) { assert.NotEmpty(t, job.Env) // Check job dependencies - assert.Contains(t, job.Needs, string(string(constants.AgentJobName))) + assert.Contains(t, job.Needs, string(constants.AgentJobName)) if tt.threatDetection { assert.Contains(t, job.Needs, string(constants.DetectionJobName)) }