Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions pkg/cli/codemod_engine_env_secrets.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
17 changes: 17 additions & 0 deletions pkg/cli/fix_codemods.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
33 changes: 30 additions & 3 deletions pkg/cli/fix_command.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package cli

import (
"errors"
"fmt"
"os"
"path/filepath"
Expand Down Expand Up @@ -160,6 +161,8 @@ func runFixCommand(workflowIDs []string, write bool, verbose bool, workflowDir s
// Process each file
var totalFixed int
var totalFiles int
var totalGuidedErrors int
var totalProcessingErrors int
var workflowsNeedingFixes []workflowFixInfo

for _, file := range files {
Expand All @@ -168,6 +171,13 @@ 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++
totalFiles++
} else {
totalProcessingErrors++
}
continue
}

Expand Down Expand Up @@ -239,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 {
} else if totalGuidedErrors == 0 && totalProcessingErrors == 0 {
fmt.Fprintf(os.Stderr, "%s\n", console.FormatInfoMessage("✓ No fixes needed"))
}
Comment on lines 250 to 254
} else {
Expand All @@ -257,11 +267,24 @@ 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 && totalProcessingErrors == 0 {
fmt.Fprintf(os.Stderr, "%s\n", console.FormatInfoMessage("✓ No fixes needed"))
}
}

if totalGuidedErrors > 0 {
pluralSuffix := "file needs"
if totalGuidedErrors > 1 {
pluralSuffix = "files need"
}
fmt.Fprintf(os.Stderr, "%s\n", console.FormatErrorMessage(fmt.Sprintf("%d %s a manual fix", totalGuidedErrors, pluralSuffix)))
return &ExitCodeError{Code: 2}
}

if totalProcessingErrors > 0 {
return &ExitCodeError{Code: 1}
}

return nil
}

Expand Down Expand Up @@ -302,7 +325,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 {
Expand Down
225 changes: 225 additions & 0 deletions pkg/cli/fix_command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1177,3 +1177,228 @@ 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, "1 file needs a manual fix", "should print singular form for a single guided error")
}

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")
}

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")
}
Loading