From 67f2d5edd2c29eb0030196d3fc567dea6a743ce1 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Wed, 4 Feb 2026 22:39:43 +0000 Subject: [PATCH 1/3] fix trial command --- pkg/cli/repo_error_messages_test.go | 2 +- pkg/cli/trial_command.go | 16 ++++++++- pkg/cli/trial_repository.go | 54 +++++++++++++++++++++++------ 3 files changed, 59 insertions(+), 13 deletions(-) diff --git a/pkg/cli/repo_error_messages_test.go b/pkg/cli/repo_error_messages_test.go index d2ae082b3d..8f18905907 100644 --- a/pkg/cli/repo_error_messages_test.go +++ b/pkg/cli/repo_error_messages_test.go @@ -55,7 +55,7 @@ func TestRepoSlugErrorMessages(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { // Test ensureTrialRepository function - err := ensureTrialRepository(tt.repoSlug, "", false, false) + err := ensureTrialRepository(tt.repoSlug, "", false, false, false) if err == nil { t.Errorf("expected error for repo slug '%s', got nil", tt.repoSlug) diff --git a/pkg/cli/trial_command.go b/pkg/cli/trial_command.go index b975df41ae..1145a2c626 100644 --- a/pkg/cli/trial_command.go +++ b/pkg/cli/trial_command.go @@ -53,6 +53,7 @@ type TrialOptions struct { DeleteHostRepo bool ForceDelete bool Quiet bool + DryRun bool TimeoutMinutes int TriggerContext string RepeatCount int @@ -121,6 +122,7 @@ Trial results are saved both locally (in trials/ directory) and in the host repo deleteHostRepo, _ := cmd.Flags().GetBool("delete-host-repo-after") forceDeleteHostRepo, _ := cmd.Flags().GetBool("force-delete-host-repo-before") yes, _ := cmd.Flags().GetBool("yes") + dryRun, _ := cmd.Flags().GetBool("dry-run") timeout, _ := cmd.Flags().GetInt("timeout") triggerContext, _ := cmd.Flags().GetString("trigger-context") repeatCount, _ := cmd.Flags().GetInt("repeat") @@ -147,6 +149,7 @@ Trial results are saved both locally (in trials/ directory) and in the host repo DeleteHostRepo: deleteHostRepo, ForceDelete: forceDeleteHostRepo, Quiet: yes, + DryRun: dryRun, TimeoutMinutes: timeout, TriggerContext: triggerContext, RepeatCount: repeatCount, @@ -180,6 +183,7 @@ Trial results are saved both locally (in trials/ directory) and in the host repo cmd.Flags().Bool("delete-host-repo-after", false, "Delete the host repository after completion (default: keep)") cmd.Flags().Bool("force-delete-host-repo-before", false, "Force delete the host repository before creation, if it exists before creating it") cmd.Flags().BoolP("yes", "y", false, "Skip confirmation prompts") + cmd.Flags().Bool("dry-run", false, "Show what would be done without making any changes") cmd.Flags().Int("timeout", 30, "Execution timeout in minutes (default: 30)") cmd.Flags().String("trigger-context", "", "Trigger context URL (e.g., GitHub issue URL) for issue-triggered workflows") cmd.Flags().Int("repeat", 0, "Number of times to repeat running workflows (0 = run once)") @@ -207,6 +211,10 @@ func RunWorkflowTrials(ctx context.Context, workflowSpecs []string, opts TrialOp parsedSpecs = append(parsedSpecs, parsedSpec) } + if opts.DryRun { + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("[DRY RUN] Showing what would be done without making changes")) + } + if len(parsedSpecs) == 1 { fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Starting trial of workflow '%s' from '%s'", parsedSpecs[0].WorkflowName, parsedSpecs[0].RepoSlug))) } else { @@ -303,10 +311,16 @@ func RunWorkflowTrials(ctx context.Context, workflowSpecs []string, opts TrialOp // Step 2: Create or reuse host repository trialLog.Printf("Ensuring trial repository exists: %s", hostRepoSlug) - if err := ensureTrialRepository(hostRepoSlug, cloneRepoSlug, opts.ForceDelete, opts.Verbose); err != nil { + if err := ensureTrialRepository(hostRepoSlug, cloneRepoSlug, opts.ForceDelete, opts.DryRun, opts.Verbose); err != nil { return fmt.Errorf("failed to ensure host repository: %w", err) } + // In dry-run mode, stop here after showing what would be done + if opts.DryRun { + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("[DRY RUN] Stopping here. No actual changes were made.")) + return nil + } + // Step 2.5: Create secret tracker var secretTracker *TrialSecretTracker if opts.PushSecrets { diff --git a/pkg/cli/trial_repository.go b/pkg/cli/trial_repository.go index 50bb32feaf..a9345d4e6f 100644 --- a/pkg/cli/trial_repository.go +++ b/pkg/cli/trial_repository.go @@ -21,8 +21,9 @@ var trialRepoLog = logger.New("cli:trial_repository") // ensureTrialRepository creates a host repository if it doesn't exist, or reuses existing one // For clone-repo mode, reusing an existing host repository is not allowed // If forceDeleteHostRepo is true, deletes the repository if it exists before creating it -func ensureTrialRepository(repoSlug string, cloneRepoSlug string, forceDeleteHostRepo bool, verbose bool) error { - trialRepoLog.Printf("Ensuring trial repository: %s (cloneRepo=%s, forceDelete=%v)", repoSlug, cloneRepoSlug, forceDeleteHostRepo) +// If dryRun is true, only shows what would be done without making changes +func ensureTrialRepository(repoSlug string, cloneRepoSlug string, forceDeleteHostRepo bool, dryRun bool, verbose bool) error { + trialRepoLog.Printf("Ensuring trial repository: %s (cloneRepo=%s, forceDelete=%v, dryRun=%v)", repoSlug, cloneRepoSlug, forceDeleteHostRepo, dryRun) parts := strings.Split(repoSlug, "/") if len(parts) != 2 || parts[0] == "" || parts[1] == "" { @@ -31,7 +32,18 @@ func ensureTrialRepository(repoSlug string, cloneRepoSlug string, forceDeleteHos // Check if repository already exists cmd := workflow.ExecGH("repo", "view", repoSlug) - if err := cmd.Run(); err == nil { + output, err := cmd.CombinedOutput() + repoExists := err == nil + + if dryRun && verbose { + if repoExists { + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("[DRY RUN] Repository %s exists", repoSlug))) + } else { + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("[DRY RUN] Repository %s does not exist (output: %s)", repoSlug, string(output)))) + } + } + + if repoExists { trialRepoLog.Printf("Repository %s already exists", repoSlug) // Repository exists - determine what to do if forceDeleteHostRepo { @@ -40,11 +52,15 @@ func ensureTrialRepository(repoSlug string, cloneRepoSlug string, forceDeleteHos fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Force deleting existing host repository: %s", repoSlug))) } - if deleteOutput, deleteErr := workflow.RunGHCombined("Deleting repository...", "repo", "delete", repoSlug, "--yes"); deleteErr != nil { - return fmt.Errorf("failed to force delete existing host repository %s: %w (output: %s)", repoSlug, deleteErr, string(deleteOutput)) - } + if dryRun { + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("[DRY RUN] Would delete repository: %s", repoSlug))) + } else { + if deleteOutput, deleteErr := workflow.RunGHCombined("Deleting repository...", "repo", "delete", repoSlug, "--yes"); deleteErr != nil { + return fmt.Errorf("failed to force delete existing host repository %s: %w (output: %s)", repoSlug, deleteErr, string(deleteOutput)) + } - fmt.Fprintln(os.Stderr, console.FormatSuccessMessage(fmt.Sprintf("Force deleted existing host repository: %s", repoSlug))) + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage(fmt.Sprintf("Force deleted existing host repository: %s", repoSlug))) + } // Continue to create the repository below } else { @@ -57,18 +73,34 @@ func ensureTrialRepository(repoSlug string, cloneRepoSlug string, forceDeleteHos fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Reusing existing host repository: %s", repoSlug))) } } - fmt.Fprintln(os.Stderr, console.FormatSuccessMessage(fmt.Sprintf("Using existing host repository: https://github.com/%s", repoSlug))) + prefix := "" + if dryRun { + prefix = "[DRY RUN] " + } + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage(fmt.Sprintf("%sUsing existing host repository: https://github.com/%s", prefix, repoSlug))) return nil } } // Repository doesn't exist, create it - if verbose { - fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Creating private host repository: %s", repoSlug))) + if verbose || dryRun { + prefix := "" + if dryRun { + prefix = "[DRY RUN] " + } + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("%sCreating private host repository: %s", prefix, repoSlug))) + } + + if dryRun { + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("[DRY RUN] Would create repository with description: 'GitHub Agentic Workflows host repository'")) + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("[DRY RUN] Would enable GitHub Actions permissions at: https://github.com/%s/settings/actions", repoSlug))) + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("[DRY RUN] Would enable discussions")) + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage(fmt.Sprintf("[DRY RUN] Would create host repository: https://github.com/%s", repoSlug))) + return nil } // Use gh CLI to create private repo with initial README using full OWNER/REPO format - output, err := workflow.RunGHCombined("Creating repository...", "repo", "create", repoSlug, "--private", "--add-readme", "--description", "GitHub Agentic Workflows host repository") + output, err = workflow.RunGHCombined("Creating repository...", "repo", "create", repoSlug, "--private", "--add-readme", "--description", "GitHub Agentic Workflows host repository") if err != nil { // Check if the error is because the repository already exists From 041ef59e4c3221d16c33c95f154b2499a62d879a Mon Sep 17 00:00:00 2001 From: Don Syme Date: Wed, 4 Feb 2026 23:04:10 +0000 Subject: [PATCH 2/3] fix gh aw trial --- pkg/cli/trial_command.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/cli/trial_command.go b/pkg/cli/trial_command.go index 1145a2c626..9af4af6567 100644 --- a/pkg/cli/trial_command.go +++ b/pkg/cli/trial_command.go @@ -95,6 +95,7 @@ Repeat and cleanup examples: ` + string(constants.CLIExtensionPrefix) + ` trial githubnext/agentics/my-workflow --repeat 3 # Run 3 times total ` + string(constants.CLIExtensionPrefix) + ` trial githubnext/agentics/my-workflow --delete-host-repo-after # Delete repo after completion ` + string(constants.CLIExtensionPrefix) + ` trial githubnext/agentics/my-workflow --quiet --host-repo my-trial # Custom host repo + ` + string(constants.CLIExtensionPrefix) + ` trial githubnext/agentics/my-workflow --dry-run # Show what would be done without changes Auto-merge examples: ` + string(constants.CLIExtensionPrefix) + ` trial githubnext/agentics/my-workflow --auto-merge-prs # Auto-merge any PRs created during trial From f47ecc903f89ee27bcc59f2076cb8a03c3b3fe49 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Wed, 4 Feb 2026 23:07:06 +0000 Subject: [PATCH 3/3] fix gh aw trial --- pkg/cli/trial_dry_run_test.go | 382 ++++++++++++++++++++++++++++++++++ 1 file changed, 382 insertions(+) create mode 100644 pkg/cli/trial_dry_run_test.go diff --git a/pkg/cli/trial_dry_run_test.go b/pkg/cli/trial_dry_run_test.go new file mode 100644 index 0000000000..22d407b93e --- /dev/null +++ b/pkg/cli/trial_dry_run_test.go @@ -0,0 +1,382 @@ +//go:build !integration + +package cli + +import ( + "bytes" + "fmt" + "os" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestTrialCommandDryRunFlag verifies that the --dry-run flag is correctly parsed +func TestTrialCommandDryRunFlag(t *testing.T) { + tests := []struct { + name string + args []string + expectError bool + description string + }{ + { + name: "dry-run flag present", + args: []string{"workflow-spec", "--dry-run"}, + expectError: false, + description: "Should accept --dry-run flag", + }, + { + name: "dry-run with other flags", + args: []string{"workflow-spec", "--dry-run", "--verbose", "-y"}, + expectError: false, + description: "Should work with other flags", + }, + { + name: "no dry-run flag", + args: []string{"workflow-spec"}, + expectError: false, + description: "Should work without dry-run flag", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cmd := NewTrialCommand(func(engine string) error { + return nil + }) + + cmd.SetArgs(tt.args) + + // We expect the command to fail because it will try to actually run + // but we're just checking flag parsing here + _ = cmd.Execute() + + // Verify the flag exists and can be retrieved + dryRunFlag := cmd.Flags().Lookup("dry-run") + require.NotNil(t, dryRunFlag, "dry-run flag should be defined") + assert.Equal(t, "bool", dryRunFlag.Value.Type(), "dry-run should be a boolean flag") + }) + } +} + +// TestTrialOptionsDryRun verifies that TrialOptions correctly stores the dry-run flag +func TestTrialOptionsDryRun(t *testing.T) { + tests := []struct { + name string + dryRun bool + expectedDryRun bool + }{ + { + name: "dry-run enabled", + dryRun: true, + expectedDryRun: true, + }, + { + name: "dry-run disabled", + dryRun: false, + expectedDryRun: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + opts := TrialOptions{ + DryRun: tt.dryRun, + } + + assert.Equal(t, tt.expectedDryRun, opts.DryRun, "DryRun field should match input") + }) + } +} + +// TestEnsureTrialRepositoryDryRun verifies behavior in dry-run mode +func TestEnsureTrialRepositoryDryRun(t *testing.T) { + // This test verifies that ensureTrialRepository handles dry-run mode correctly + // In dry-run mode, it should not make actual API calls but should validate inputs + + tests := []struct { + name string + repoSlug string + cloneRepoSlug string + forceDelete bool + dryRun bool + verbose bool + expectError bool + errorContains string + description string + }{ + { + name: "dry-run with invalid repo slug format", + repoSlug: "invalid-format", + cloneRepoSlug: "", + forceDelete: false, + dryRun: true, + verbose: false, + expectError: true, + errorContains: "invalid repository slug format", + description: "Should validate repo slug format even in dry-run mode", + }, + { + name: "dry-run with valid repo slug", + repoSlug: "owner/repo", + cloneRepoSlug: "", + forceDelete: false, + dryRun: true, + verbose: false, + expectError: false, + description: "Should accept valid repo slug in dry-run mode", + }, + { + name: "dry-run with force delete", + repoSlug: "owner/repo", + cloneRepoSlug: "", + forceDelete: true, + dryRun: true, + verbose: true, + expectError: false, + description: "Should handle force delete flag in dry-run mode", + }, + { + name: "dry-run with clone repo", + repoSlug: "owner/host-repo", + cloneRepoSlug: "owner/source-repo", + forceDelete: false, + dryRun: true, + verbose: true, + expectError: false, + description: "Should handle clone repo in dry-run mode", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Capture stderr to verify dry-run messages + oldStderr := os.Stderr + r, w, _ := os.Pipe() + os.Stderr = w + + err := ensureTrialRepository(tt.repoSlug, tt.cloneRepoSlug, tt.forceDelete, tt.dryRun, tt.verbose) + + // Restore stderr + w.Close() + os.Stderr = oldStderr + + // Read captured output + var buf bytes.Buffer + buf.ReadFrom(r) + output := buf.String() + + if tt.expectError { + require.Error(t, err, "Expected error for %s", tt.description) + if tt.errorContains != "" { + assert.Contains(t, err.Error(), tt.errorContains, "Error should contain expected text") + } + } else { + assert.NoError(t, err, "Should not error for %s", tt.description) + + // In dry-run mode with verbose, output should contain dry-run indicators + if tt.dryRun && tt.verbose { + assert.Contains(t, output, "[DRY RUN]", "Verbose dry-run should show [DRY RUN] prefix") + } + } + }) + } +} + +// TestDryRunMessageFormatting verifies that dry-run messages are formatted consistently +func TestDryRunMessageFormatting(t *testing.T) { + tests := []struct { + name string + repoSlug string + dryRun bool + verbose bool + expectedPrefixes []string + description string + }{ + { + name: "dry-run enabled with verbose", + repoSlug: "owner/test-repo", + dryRun: true, + verbose: true, + expectedPrefixes: []string{ + "[DRY RUN]", + }, + description: "Should show [DRY RUN] prefix when verbose and dry-run enabled", + }, + { + name: "dry-run enabled without verbose", + repoSlug: "owner/test-repo", + dryRun: true, + verbose: false, + expectedPrefixes: []string{ + "[DRY RUN]", + }, + description: "Should show [DRY RUN] prefix even without verbose", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Capture stderr to check message formatting + oldStderr := os.Stderr + r, w, _ := os.Pipe() + os.Stderr = w + + _ = ensureTrialRepository(tt.repoSlug, "", false, tt.dryRun, tt.verbose) + + // Restore stderr + w.Close() + os.Stderr = oldStderr + + // Read captured output + var buf bytes.Buffer + buf.ReadFrom(r) + output := buf.String() + + // Check for expected prefixes + for _, prefix := range tt.expectedPrefixes { + assert.Contains(t, output, prefix, "Output should contain prefix: %s", prefix) + } + }) + } +} + +// TestDryRunNoActualAPICallsForCreate verifies that dry-run doesn't create repositories +func TestDryRunNoActualAPICallsForCreate(t *testing.T) { + // This test documents that in dry-run mode, we should not make actual GitHub API calls + // This is a behavioral test - actual integration would require mocking gh CLI + + repoSlug := "test-owner/test-repo-" + fmt.Sprintf("%d", os.Getpid()) + dryRun := true + verbose := true + + // Capture stderr + oldStderr := os.Stderr + r, w, _ := os.Pipe() + os.Stderr = w + + err := ensureTrialRepository(repoSlug, "", false, dryRun, verbose) + + // Restore stderr + w.Close() + os.Stderr = oldStderr + + // Read captured output + var buf bytes.Buffer + buf.ReadFrom(r) + output := buf.String() + + // Should not error + assert.NoError(t, err, "Dry-run should not error") + + // Should indicate that it would create the repository + if strings.Contains(output, "exists") { + // If repo exists message shown, that's fine + t.Logf("Repository appears to exist: %s", repoSlug) + } else { + // Should show would-create messages + assert.Contains(t, output, "[DRY RUN]", "Should show dry-run prefix") + assert.Contains(t, output, "Would create", "Should indicate it would create repo") + } + + // The key assertion: the repository should NOT actually be created + // We can't fully verify this without integration tests, but the presence of + // "Would create" messages indicates the actual create call was skipped + t.Log("Dry-run mode should skip actual repository creation") +} + +// TestDryRunForceDeleteBehavior verifies dry-run behavior with force delete flag +func TestDryRunForceDeleteBehavior(t *testing.T) { + tests := []struct { + name string + repoSlug string + forceDelete bool + dryRun bool + verbose bool + description string + }{ + { + name: "dry-run with force delete", + repoSlug: "owner/test-repo", + forceDelete: true, + dryRun: true, + verbose: true, + description: "Should show would-delete message in dry-run mode", + }, + { + name: "dry-run without force delete", + repoSlug: "owner/test-repo", + forceDelete: false, + dryRun: true, + verbose: true, + description: "Should show would-reuse message in dry-run mode", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Capture stderr + oldStderr := os.Stderr + r, w, _ := os.Pipe() + os.Stderr = w + + err := ensureTrialRepository(tt.repoSlug, "", tt.forceDelete, tt.dryRun, tt.verbose) + + // Restore stderr + w.Close() + os.Stderr = oldStderr + + // Read captured output + var buf bytes.Buffer + buf.ReadFrom(r) + output := buf.String() + + assert.NoError(t, err, "Should not error in dry-run mode") + assert.Contains(t, output, "[DRY RUN]", "Should show dry-run prefix") + }) + } +} + +// TestDryRunValidationStillOccurs verifies that input validation happens in dry-run mode +func TestDryRunValidationStillOccurs(t *testing.T) { + tests := []struct { + name string + repoSlug string + expectError bool + errorContains string + }{ + { + name: "empty repo slug", + repoSlug: "/", + expectError: true, + errorContains: "invalid repository slug format", + }, + { + name: "no slash in repo slug", + repoSlug: "justname", + expectError: true, + errorContains: "invalid repository slug format", + }, + { + name: "valid repo slug", + repoSlug: "owner/repo", + expectError: false, + errorContains: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ensureTrialRepository(tt.repoSlug, "", false, true, false) + + if tt.expectError { + require.Error(t, err, "Expected validation error in dry-run mode") + assert.Contains(t, err.Error(), tt.errorContains, "Error should contain expected text") + } else { + assert.NoError(t, err, "Valid input should not error in dry-run mode") + } + }) + } +}