From 158a18e5e2a68aaf294c4e744f992c83db8a820b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 3 Jul 2026 10:26:26 +0000 Subject: [PATCH 1/4] Initial plan From 99830b5f9b7011be5bbd13a20e364e12e0e2c780 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 3 Jul 2026 10:40:57 +0000 Subject: [PATCH 2/4] fix: guided-error codemods now return exit code 2 and correct summary message Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/codemod_engine_env_secrets.go | 1 + pkg/cli/fix_codemods.go | 17 ++++ pkg/cli/fix_command.go | 25 ++++- pkg/cli/fix_command_test.go | 135 ++++++++++++++++++++++++++ 4 files changed, 175 insertions(+), 3 deletions(-) diff --git a/pkg/cli/codemod_engine_env_secrets.go b/pkg/cli/codemod_engine_env_secrets.go index be164f683e7..e18463de0e9 100644 --- a/pkg/cli/codemod_engine_env_secrets.go +++ b/pkg/cli/codemod_engine_env_secrets.go @@ -263,6 +263,7 @@ func getTopLevelEnvSecretsGuidedErrorCodemod() Codemod { Name: "Detect secrets in top-level env section (manual fix required)", Description: "Detects secrets in the top-level env: block that will be leaked to the agent container in strict mode, and emits a guided error pointing users to engine-specific secret configuration.", IntroducedIn: "1.5.0", + Guided: true, Apply: func(content string, frontmatter map[string]any) (string, bool, error) { envValue, hasEnv := frontmatter["env"] if !hasEnv { diff --git a/pkg/cli/fix_codemods.go b/pkg/cli/fix_codemods.go index 7225d34c9e9..15914c14d06 100644 --- a/pkg/cli/fix_codemods.go +++ b/pkg/cli/fix_codemods.go @@ -16,9 +16,26 @@ type Codemod struct { Name string // Human-readable name Description string // Description of what the codemod does IntroducedIn string // Version where this codemod was introduced + Guided bool // If true, errors from Apply are guided/manual-fix errors (not auto-correctable) Apply func(content string, frontmatter map[string]any) (string, bool, error) } +// GuidedError is returned when a codemod with Guided: true emits an error. +// Unlike regular processing errors, a guided error signals that the file was +// read successfully but requires a human to manually address an issue that +// cannot be auto-corrected by any codemod. +type GuidedError struct { + Cause error +} + +func (e *GuidedError) Error() string { + return e.Cause.Error() +} + +func (e *GuidedError) Unwrap() error { + return e.Cause +} + // CodemodResult represents the result of applying a codemod type CodemodResult struct { Applied bool // Whether the codemod was applied diff --git a/pkg/cli/fix_command.go b/pkg/cli/fix_command.go index 7e83abb438c..cf10bdedc13 100644 --- a/pkg/cli/fix_command.go +++ b/pkg/cli/fix_command.go @@ -1,6 +1,7 @@ package cli import ( + "errors" "fmt" "os" "path/filepath" @@ -160,6 +161,7 @@ func runFixCommand(workflowIDs []string, write bool, verbose bool, workflowDir s // Process each file var totalFixed int var totalFiles int + var totalGuidedErrors int var workflowsNeedingFixes []workflowFixInfo for _, file := range files { @@ -168,6 +170,10 @@ func runFixCommand(workflowIDs []string, write bool, verbose bool, workflowDir s fixed, appliedFixes, err := processWorkflowFileWithInfo(file, codemods, write, verbose) if err != nil { fmt.Fprintf(os.Stderr, "%s\n", console.FormatErrorMessage(fmt.Sprintf("Error processing %s: %v", filepath.Base(file), err))) + var guidedErr *GuidedError + if errors.As(err, &guidedErr) { + totalGuidedErrors++ + } continue } @@ -239,7 +245,7 @@ func runFixCommand(workflowIDs []string, write bool, verbose bool, workflowDir s if write { if totalFixed > 0 { fmt.Fprintf(os.Stderr, "%s\n", console.FormatSuccessMessage(fmt.Sprintf("✓ Fixed %d of %d workflow files", totalFixed, totalFiles))) - } else { + } else if totalGuidedErrors == 0 { fmt.Fprintf(os.Stderr, "%s\n", console.FormatInfoMessage("✓ No fixes needed")) } } else { @@ -257,11 +263,20 @@ func runFixCommand(workflowIDs []string, write bool, verbose bool, workflowDir s for _, wf := range workflowsNeedingFixes { fmt.Fprintf(os.Stderr, " gh aw fix %s --write\n", strings.TrimSuffix(wf.File, ".md")) } - } else { + } else if totalGuidedErrors == 0 { fmt.Fprintf(os.Stderr, "%s\n", console.FormatInfoMessage("✓ No fixes needed")) } } + if totalGuidedErrors > 0 { + noun := "file needs" + if totalGuidedErrors > 1 { + noun = "files need" + } + fmt.Fprintf(os.Stderr, "%s\n", console.FormatErrorMessage(fmt.Sprintf("%d %s a manual fix", totalGuidedErrors, noun))) + return &ExitCodeError{Code: 2} + } + return nil } @@ -302,7 +317,11 @@ func processWorkflowFileWithInfo(filePath string, codemods []Codemod, write bool newContent, applied, err := codemod.Apply(currentContent, currentResult.Frontmatter) if err != nil { fixLog.Printf("Codemod %s failed: %v", codemod.ID, err) - return false, nil, fmt.Errorf("codemod %s failed: %w", codemod.ID, err) + wrappedErr := fmt.Errorf("codemod %s failed: %w", codemod.ID, err) + if codemod.Guided { + return false, nil, &GuidedError{Cause: wrappedErr} + } + return false, nil, wrappedErr } if applied { diff --git a/pkg/cli/fix_command_test.go b/pkg/cli/fix_command_test.go index 934f136d0b1..6a1095b69e9 100644 --- a/pkg/cli/fix_command_test.go +++ b/pkg/cli/fix_command_test.go @@ -1177,3 +1177,138 @@ tools: t.Errorf("Expected scaffolded Serena workflow to pass languages through to upstream import, got:\n%s", scaffolded) } } + +func TestRunFix_GuidedErrorReturnsExitCode2(t *testing.T) { + tmpDir := t.TempDir() + workflowFile := filepath.Join(tmpDir, "byok-workflow.md") + + // Workflow with a secret in top-level env (triggers the guided-error codemod) + content := `--- +on: workflow_dispatch +env: + AZURE_CLIENT_ID: ${{ secrets.AZURE_CLIENT_ID }} +--- + +# BYOK Workflow +` + require.NoError(t, os.WriteFile(workflowFile, []byte(content), 0644)) + + err := RunFix(FixConfig{ + Write: false, + WorkflowDir: tmpDir, + }) + + require.Error(t, err, "RunFix should return an error when a guided error is triggered") + var exitErr *ExitCodeError + require.ErrorAs(t, err, &exitErr, "error should be an ExitCodeError") + assert.Equal(t, 2, exitErr.Code, "exit code should be 2 for guided errors") +} + +func TestRunFix_GuidedErrorDoesNotPrintNoFixesNeeded(t *testing.T) { + tmpDir := t.TempDir() + workflowFile := filepath.Join(tmpDir, "byok-workflow.md") + + // Workflow with a secret in top-level env (triggers the guided-error codemod) + content := `--- +on: workflow_dispatch +env: + AZURE_CLIENT_ID: ${{ secrets.AZURE_CLIENT_ID }} +--- + +# BYOK Workflow +` + require.NoError(t, os.WriteFile(workflowFile, []byte(content), 0644)) + + // Capture stderr + origStderr := os.Stderr + r, w, err := os.Pipe() + require.NoError(t, err) + os.Stderr = w + t.Cleanup(func() { os.Stderr = origStderr }) + + _ = RunFix(FixConfig{ + Write: false, + WorkflowDir: tmpDir, + }) + + require.NoError(t, w.Close()) + output, err := io.ReadAll(r) + require.NoError(t, err) + outputStr := string(output) + + assert.NotContains(t, outputStr, "No fixes needed", "should not print 'No fixes needed' when a guided error is present") + assert.Contains(t, outputStr, "manual fix", "should mention manual fix in output") +} + +func TestRunFix_GuidedErrorWithMultipleFiles(t *testing.T) { + tmpDir := t.TempDir() + + // File 1: has guided error + content1 := `--- +on: workflow_dispatch +env: + TOKEN: ${{ secrets.MY_TOKEN }} +--- + +# Workflow 1 +` + // File 2: has guided error + content2 := `--- +on: workflow_dispatch +env: + API_KEY: ${{ secrets.API_KEY }} +--- + +# Workflow 2 +` + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "workflow-one.md"), []byte(content1), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "workflow-two.md"), []byte(content2), 0644)) + + // Capture stderr + origStderr := os.Stderr + r, w, err := os.Pipe() + require.NoError(t, err) + os.Stderr = w + t.Cleanup(func() { os.Stderr = origStderr }) + + runErr := RunFix(FixConfig{ + Write: false, + WorkflowDir: tmpDir, + }) + + require.NoError(t, w.Close()) + output, err := io.ReadAll(r) + require.NoError(t, err) + outputStr := string(output) + + require.Error(t, runErr) + var exitErr *ExitCodeError + require.ErrorAs(t, runErr, &exitErr) + assert.Equal(t, 2, exitErr.Code) + + assert.Contains(t, outputStr, "2 files need a manual fix", "should report plural count") +} + +func TestProcessWorkflowFileWithInfo_GuidedErrorWrapped(t *testing.T) { + tmpDir := t.TempDir() + workflowFile := filepath.Join(tmpDir, "test.md") + + content := `--- +on: workflow_dispatch +env: + TOKEN: ${{ secrets.MY_TOKEN }} +--- + +# Test +` + require.NoError(t, os.WriteFile(workflowFile, []byte(content), 0644)) + + guidedCodemod := getCodemodByID("top-level-env-secrets-guided-error") + require.NotNil(t, guidedCodemod) + + _, _, err := processWorkflowFileWithInfo(workflowFile, []Codemod{*guidedCodemod}, false, false) + + require.Error(t, err) + var guidedErr *GuidedError + assert.ErrorAs(t, err, &guidedErr, "error should be wrapped as GuidedError") +} From 4a04da8f72b16bbed571c9a82c84ae8026efd9ff Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 3 Jul 2026 10:43:52 +0000 Subject: [PATCH 3/4] review: rename noun to pluralSuffix, add singular form assertion in test Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/fix_command.go | 6 +++--- pkg/cli/fix_command_test.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/cli/fix_command.go b/pkg/cli/fix_command.go index cf10bdedc13..309463c27df 100644 --- a/pkg/cli/fix_command.go +++ b/pkg/cli/fix_command.go @@ -269,11 +269,11 @@ func runFixCommand(workflowIDs []string, write bool, verbose bool, workflowDir s } if totalGuidedErrors > 0 { - noun := "file needs" + pluralSuffix := "file needs" if totalGuidedErrors > 1 { - noun = "files need" + pluralSuffix = "files need" } - fmt.Fprintf(os.Stderr, "%s\n", console.FormatErrorMessage(fmt.Sprintf("%d %s a manual fix", totalGuidedErrors, noun))) + fmt.Fprintf(os.Stderr, "%s\n", console.FormatErrorMessage(fmt.Sprintf("%d %s a manual fix", totalGuidedErrors, pluralSuffix))) return &ExitCodeError{Code: 2} } diff --git a/pkg/cli/fix_command_test.go b/pkg/cli/fix_command_test.go index 6a1095b69e9..dd23e8194d3 100644 --- a/pkg/cli/fix_command_test.go +++ b/pkg/cli/fix_command_test.go @@ -1237,7 +1237,7 @@ env: outputStr := string(output) assert.NotContains(t, outputStr, "No fixes needed", "should not print 'No fixes needed' when a guided error is present") - assert.Contains(t, outputStr, "manual fix", "should mention manual fix in output") + assert.Contains(t, outputStr, "1 file needs a manual fix", "should print singular form for a single guided error") } func TestRunFix_GuidedErrorWithMultipleFiles(t *testing.T) { From a1d37096d031c037ed7757af91304a3562aca907 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 3 Jul 2026 14:41:15 +0000 Subject: [PATCH 4/4] fix: count guided-error files in totalFiles; suppress false success and return exit 1 on hard processing errors Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- pkg/cli/fix_command.go | 12 ++++- pkg/cli/fix_command_test.go | 90 +++++++++++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+), 2 deletions(-) diff --git a/pkg/cli/fix_command.go b/pkg/cli/fix_command.go index 309463c27df..e6108ae6fca 100644 --- a/pkg/cli/fix_command.go +++ b/pkg/cli/fix_command.go @@ -162,6 +162,7 @@ func runFixCommand(workflowIDs []string, write bool, verbose bool, workflowDir s var totalFixed int var totalFiles int var totalGuidedErrors int + var totalProcessingErrors int var workflowsNeedingFixes []workflowFixInfo for _, file := range files { @@ -173,6 +174,9 @@ func runFixCommand(workflowIDs []string, write bool, verbose bool, workflowDir s var guidedErr *GuidedError if errors.As(err, &guidedErr) { totalGuidedErrors++ + totalFiles++ + } else { + totalProcessingErrors++ } continue } @@ -245,7 +249,7 @@ func runFixCommand(workflowIDs []string, write bool, verbose bool, workflowDir s if write { if totalFixed > 0 { fmt.Fprintf(os.Stderr, "%s\n", console.FormatSuccessMessage(fmt.Sprintf("✓ Fixed %d of %d workflow files", totalFixed, totalFiles))) - } else if totalGuidedErrors == 0 { + } else if totalGuidedErrors == 0 && totalProcessingErrors == 0 { fmt.Fprintf(os.Stderr, "%s\n", console.FormatInfoMessage("✓ No fixes needed")) } } else { @@ -263,7 +267,7 @@ func runFixCommand(workflowIDs []string, write bool, verbose bool, workflowDir s for _, wf := range workflowsNeedingFixes { fmt.Fprintf(os.Stderr, " gh aw fix %s --write\n", strings.TrimSuffix(wf.File, ".md")) } - } else if totalGuidedErrors == 0 { + } else if totalGuidedErrors == 0 && totalProcessingErrors == 0 { fmt.Fprintf(os.Stderr, "%s\n", console.FormatInfoMessage("✓ No fixes needed")) } } @@ -277,6 +281,10 @@ func runFixCommand(workflowIDs []string, write bool, verbose bool, workflowDir s return &ExitCodeError{Code: 2} } + if totalProcessingErrors > 0 { + return &ExitCodeError{Code: 1} + } + return nil } diff --git a/pkg/cli/fix_command_test.go b/pkg/cli/fix_command_test.go index dd23e8194d3..b134118ab8f 100644 --- a/pkg/cli/fix_command_test.go +++ b/pkg/cli/fix_command_test.go @@ -1312,3 +1312,93 @@ env: var guidedErr *GuidedError assert.ErrorAs(t, err, &guidedErr, "error should be wrapped as GuidedError") } + +func TestRunFix_HardProcessingErrorReturnsExitCode1(t *testing.T) { + tmpDir := t.TempDir() + + // Create a directory where a file is expected — reading it will fail + subdir := filepath.Join(tmpDir, "not-a-file.md") + require.NoError(t, os.Mkdir(subdir, 0755)) + + err := RunFix(FixConfig{ + Write: false, + WorkflowDir: tmpDir, + }) + + require.Error(t, err, "RunFix should return an error when a hard processing error occurs") + var exitErr *ExitCodeError + require.ErrorAs(t, err, &exitErr, "error should be an ExitCodeError") + assert.Equal(t, 1, exitErr.Code, "exit code should be 1 for hard processing errors") +} + +func TestRunFix_HardProcessingErrorDoesNotPrintNoFixesNeeded(t *testing.T) { + tmpDir := t.TempDir() + + // Create a directory where a file is expected — reading it will fail + subdir := filepath.Join(tmpDir, "not-a-file.md") + require.NoError(t, os.Mkdir(subdir, 0755)) + + origStderr := os.Stderr + r, w, err := os.Pipe() + require.NoError(t, err) + os.Stderr = w + t.Cleanup(func() { os.Stderr = origStderr }) + + _ = RunFix(FixConfig{ + Write: false, + WorkflowDir: tmpDir, + }) + + require.NoError(t, w.Close()) + output, readErr := io.ReadAll(r) + require.NoError(t, readErr) + + assert.NotContains(t, string(output), "No fixes needed", "should not print 'No fixes needed' when a hard processing error occurred") +} + +func TestRunFix_GuidedErrorCountsInTotalFiles(t *testing.T) { + tmpDir := t.TempDir() + + // File 1: clean (no fixes needed) + clean := `--- +on: workflow_dispatch +--- + +# Clean Workflow +` + // File 2: guided error + guided := `--- +on: workflow_dispatch +env: + TOKEN: ${{ secrets.MY_TOKEN }} +--- + +# BYOK Workflow +` + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "a-clean.md"), []byte(clean), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "b-guided.md"), []byte(guided), 0644)) + + origStderr := os.Stderr + r, w, err := os.Pipe() + require.NoError(t, err) + os.Stderr = w + t.Cleanup(func() { os.Stderr = origStderr }) + + runErr := RunFix(FixConfig{ + Write: false, + WorkflowDir: tmpDir, + }) + + require.NoError(t, w.Close()) + output, readErr := io.ReadAll(r) + require.NoError(t, readErr) + + // Should still report guided error count and exit 2 + require.Error(t, runErr) + var exitErr *ExitCodeError + require.ErrorAs(t, runErr, &exitErr) + assert.Equal(t, 2, exitErr.Code) + + // "No fixes needed" must be absent + assert.NotContains(t, string(output), "No fixes needed") +}