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
4 changes: 2 additions & 2 deletions .github/workflows/unbloat-docs.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions .github/workflows/unbloat-docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ safe-outputs:
reviewers: [copilot]
draft: true
auto-merge: true
fallback-as-issue: false
add-comment:
max: 1
upload-asset:
Expand Down
25 changes: 24 additions & 1 deletion actions/setup/js/create_pull_request.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ async function main(config = {}) {
const maxSizeKb = config.max_patch_size ? parseInt(String(config.max_patch_size), 10) : 1024;
const { defaultTargetRepo, allowedRepos } = resolveTargetRepoConfig(config);
const includeFooter = config.footer !== false; // Default to true (include footer)
const fallbackAsIssue = config.fallback_as_issue !== false; // Default to true (fallback enabled)

// Environment validation - fail early if required variables are missing
const workflowId = process.env.GH_AW_WORKFLOW_ID;
Expand Down Expand Up @@ -514,8 +515,20 @@ async function main(config = {}) {
await exec.exec(`git push origin ${branchName}`);
core.info("Changes pushed to branch");
} catch (pushError) {
// Push failed - create fallback issue instead of PR
// Push failed - create fallback issue instead of PR (if fallback is enabled)
core.error(`Git push failed: ${pushError instanceof Error ? pushError.message : String(pushError)}`);

if (!fallbackAsIssue) {
// Fallback is disabled - return error without creating issue
core.error("fallback-as-issue is disabled - not creating fallback issue");
const error = `Failed to push changes: ${pushError instanceof Error ? pushError.message : String(pushError)}`;
return {
success: false,
error,
error_type: "push_failed",
};
}

core.warning("Git push operation failed - creating fallback issue instead of pull request");

const runId = context.runId;
Expand Down Expand Up @@ -752,6 +765,16 @@ ${patchPreview}`;
};
}

if (!fallbackAsIssue) {
// Fallback is disabled - return error without creating issue
core.error("fallback-as-issue is disabled - not creating fallback issue");
return {
success: false,
error: errorMessage,
error_type: "pr_creation_failed",
};
}

core.info("Falling back to creating an issue instead");

// Create issue as fallback with enhanced body content
Expand Down
48 changes: 48 additions & 0 deletions actions/setup/js/create_pull_request.test.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// @ts-check
import { describe, it, expect } from "vitest";

describe("create_pull_request - fallback-as-issue configuration", () => {
describe("configuration parsing", () => {
it("should default fallback_as_issue to true when not specified", () => {
const config = {};
const fallbackAsIssue = config.fallback_as_issue !== false;

expect(fallbackAsIssue).toBe(true);
});

it("should respect fallback_as_issue when set to false", () => {
const config = { fallback_as_issue: false };
const fallbackAsIssue = config.fallback_as_issue !== false;

expect(fallbackAsIssue).toBe(false);
});

it("should respect fallback_as_issue when explicitly set to true", () => {
const config = { fallback_as_issue: true };
const fallbackAsIssue = config.fallback_as_issue !== false;

expect(fallbackAsIssue).toBe(true);
});
});

describe("error type documentation", () => {
it("should document expected error types", () => {
// This test documents the expected error types for different failure scenarios
const errorTypes = {
push_failed: "Used when git push operation fails and fallback-as-issue is false",
pr_creation_failed: "Used when PR creation fails (except permission errors) and fallback-as-issue is false",
permission_denied: "Used when GitHub Actions lacks permission to create/approve PRs (handled before fallback logic)",
};

// Verify the error types are documented
expect(errorTypes.push_failed).toBeDefined();
expect(errorTypes.pr_creation_failed).toBeDefined();
expect(errorTypes.permission_denied).toBeDefined();

// These error types should be returned in the corresponding code paths:
// - push failure with fallback disabled: error_type: "push_failed"
// - PR creation failure with fallback disabled: error_type: "pr_creation_failed"
// - Permission error (always): error_type: "permission_denied"
});
});
});
7 changes: 7 additions & 0 deletions docs/src/content/docs/reference/frontmatter-full.md
Original file line number Diff line number Diff line change
Expand Up @@ -2631,6 +2631,13 @@ safe-outputs:
# (optional)
footer: true

# Controls the fallback behavior when pull request creation fails. When true
# (default), an issue is created as a fallback with the patch content. When false,
# no issue is created and the workflow fails with an error. Setting to false also
# removes the issues:write permission requirement.
# (optional)
fallback-as-issue: true

# Option 2: Enable pull request creation with default configuration
create-pull-request: null

Expand Down
5 changes: 5 additions & 0 deletions pkg/parser/schemas/main_workflow_schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -4894,6 +4894,11 @@
"type": "boolean",
"description": "Controls whether AI-generated footer is added to the pull request. When false, the visible footer content is omitted but XML markers (workflow-id, tracker-id, metadata) are still included for searchability. Defaults to true.",
"default": true
},
"fallback-as-issue": {
"type": "boolean",
"description": "Controls the fallback behavior when pull request creation fails. When true (default), an issue is created as a fallback with the patch content. When false, no issue is created and the workflow fails with an error. Setting to false also removes the issues:write permission requirement.",
"default": true
}
},
"additionalProperties": false,
Expand Down
211 changes: 211 additions & 0 deletions pkg/workflow/compile_outputs_pr_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -606,3 +606,214 @@ This test verifies that auto-merge configuration is properly handled.
t.Error("Expected auto_merge:true in handler config JSON")
}
}

func TestOutputPullRequestFallbackAsIssueFalse(t *testing.T) {
// Create temporary directory for test files
tmpDir := testutil.TempDir(t, "output-pr-fallback-false-test")

// Test case with create-pull-request configuration with fallback-as-issue: false
testContent := `---
on: push
permissions:
contents: read
pull-requests: write
engine: claude
features:
dangerous-permissions-write: true
strict: false
safe-outputs:
create-pull-request:
title-prefix: "[test] "
fallback-as-issue: false
---

# Test Output Pull Request Fallback False

This workflow tests the create-pull-request with fallback-as-issue disabled.
`

testFile := filepath.Join(tmpDir, "test-output-pr-fallback-false.md")
if err := os.WriteFile(testFile, []byte(testContent), 0644); err != nil {
t.Fatal(err)
}

compiler := NewCompiler()

// Parse the workflow data
workflowData, err := compiler.ParseWorkflowFile(testFile)
if err != nil {
t.Fatalf("Unexpected error parsing workflow with fallback-as-issue: false: %v", err)
}

// Verify that fallback-as-issue is parsed correctly
if workflowData.SafeOutputs == nil {
t.Fatal("Expected output configuration to be parsed")
}

if workflowData.SafeOutputs.CreatePullRequests == nil {
t.Fatal("Expected pull-request configuration to be parsed")
}

if workflowData.SafeOutputs.CreatePullRequests.FallbackAsIssue == nil {
t.Fatal("Expected fallback-as-issue to be set")
}

if *workflowData.SafeOutputs.CreatePullRequests.FallbackAsIssue != false {
t.Error("Expected fallback-as-issue to be false")
}

// Compile the workflow
if err := compiler.CompileWorkflow(testFile); err != nil {
t.Fatalf("Unexpected error compiling workflow with fallback-as-issue: false: %v", err)
}

// Read the generated lock file
lockFile := stringutil.MarkdownToLockFile(testFile)
lockContent, err := os.ReadFile(lockFile)
if err != nil {
t.Fatalf("Failed to read generated lock file: %v", err)
}

// Convert to string for easier testing
lockContentStr := string(lockContent)

// Find the safe_outputs job section in the lock file
safeOutputsJobStart := strings.Index(lockContentStr, "safe_outputs:")
if safeOutputsJobStart == -1 {
t.Fatal("Could not find safe_outputs job in lock file")
}

// Find the next job after safe_outputs (to limit our search scope)
// Extract a large section after safe_outputs job (next 2000 chars should include all job details)
endIdx := safeOutputsJobStart + 2000
if endIdx > len(lockContentStr) {
endIdx = len(lockContentStr)
}
safeOutputsJobSection := lockContentStr[safeOutputsJobStart:endIdx]

// Verify permissions in safe_outputs job
if !strings.Contains(safeOutputsJobSection, "contents: write") {
t.Error("Expected contents: write permission in safe_outputs job")
}

if !strings.Contains(safeOutputsJobSection, "pull-requests: write") {
t.Error("Expected pull-requests: write permission in safe_outputs job")
}

if strings.Contains(safeOutputsJobSection, "issues: write") {
t.Error("Did not expect issues: write permission in safe_outputs job when fallback-as-issue: false")
}

// Verify handler config includes fallback_as_issue: false
if !strings.Contains(lockContentStr, "GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG") {
t.Error("Expected GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG environment variable")
}

if !strings.Contains(lockContentStr, `fallback_as_issue\":false`) {
t.Error("Expected fallback_as_issue:false in handler config JSON")
}
}

func TestOutputPullRequestFallbackAsIssueDefault(t *testing.T) {
// Create temporary directory for test files
tmpDir := testutil.TempDir(t, "output-pr-fallback-default-test")

// Test case with create-pull-request configuration without fallback-as-issue (should default to true)
testContent := `---
on: push
permissions:
contents: read
pull-requests: write
engine: claude
features:
dangerous-permissions-write: true
strict: false
safe-outputs:
create-pull-request:
title-prefix: "[test] "
---

# Test Output Pull Request Fallback Default

This workflow tests the create-pull-request with default fallback-as-issue behavior.
`

testFile := filepath.Join(tmpDir, "test-output-pr-fallback-default.md")
if err := os.WriteFile(testFile, []byte(testContent), 0644); err != nil {
t.Fatal(err)
}

compiler := NewCompiler()

// Parse the workflow data
workflowData, err := compiler.ParseWorkflowFile(testFile)
if err != nil {
t.Fatalf("Unexpected error parsing workflow: %v", err)
}

// Verify that fallback-as-issue defaults to true (nil means default)
if workflowData.SafeOutputs == nil {
t.Fatal("Expected output configuration to be parsed")
}

if workflowData.SafeOutputs.CreatePullRequests == nil {
t.Fatal("Expected pull-request configuration to be parsed")
}

// When not specified, FallbackAsIssue should be nil (which means default to true)
if workflowData.SafeOutputs.CreatePullRequests.FallbackAsIssue != nil {
t.Logf("FallbackAsIssue is set to %v, expected nil for default", *workflowData.SafeOutputs.CreatePullRequests.FallbackAsIssue)
}

// Compile the workflow
if err := compiler.CompileWorkflow(testFile); err != nil {
t.Fatalf("Unexpected error compiling workflow: %v", err)
}

// Read the generated lock file
lockFile := stringutil.MarkdownToLockFile(testFile)
lockContent, err := os.ReadFile(lockFile)
if err != nil {
t.Fatalf("Failed to read generated lock file: %v", err)
}

// Convert to string for easier testing
lockContentStr := string(lockContent)

// Find the safe_outputs job section in the lock file
safeOutputsJobStart := strings.Index(lockContentStr, "safe_outputs:")
if safeOutputsJobStart == -1 {
t.Fatal("Could not find safe_outputs job in lock file")
}

// Extract a large section after safe_outputs job (next 2000 chars should include all job details)
endIdx := safeOutputsJobStart + 2000
if endIdx > len(lockContentStr) {
endIdx = len(lockContentStr)
}

safeOutputsJobSection := lockContentStr[safeOutputsJobStart:endIdx]

// Verify permissions in safe_outputs job include issues: write (default behavior)
if !strings.Contains(safeOutputsJobSection, "contents: write") {
t.Error("Expected contents: write permission in safe_outputs job")
}

if !strings.Contains(safeOutputsJobSection, "pull-requests: write") {
t.Error("Expected pull-requests: write permission in safe_outputs job")
}
if !strings.Contains(safeOutputsJobSection, "issues: write") {
t.Error("Expected issues: write permission in safe_outputs job when fallback-as-issue defaults to true")
}

// Verify handler config defaults fallback_as_issue to true (or omitted means default true)
if !strings.Contains(lockContentStr, "GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG") {
t.Error("Expected GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG environment variable")
}

// When not explicitly set, fallback_as_issue is omitted from JSON (defaults to true in handler)
// So we just verify it is NOT explicitly set to false
if strings.Contains(lockContentStr, `fallback_as_issue\":false`) {
t.Error("Did not expect fallback_as_issue:false in handler config JSON when using default")
}
}
3 changes: 2 additions & 1 deletion pkg/workflow/compiler_safe_outputs_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,8 @@ var handlerRegistry = map[string]handlerBuilder{
AddIfNotEmpty("target-repo", c.TargetRepoSlug).
AddStringSlice("allowed_repos", c.AllowedRepos).
AddDefault("max_patch_size", maxPatchSize).
AddBoolPtr("footer", getEffectiveFooter(c.Footer, cfg.Footer))
AddBoolPtr("footer", getEffectiveFooter(c.Footer, cfg.Footer)).
AddBoolPtr("fallback_as_issue", c.FallbackAsIssue)
// Add base_branch - use custom value if specified, otherwise use github.ref_name
if c.BaseBranch != "" {
builder.AddDefault("base_branch", c.BaseBranch)
Expand Down
7 changes: 6 additions & 1 deletion pkg/workflow/compiler_safe_outputs_job.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,12 @@ func (c *Compiler) buildConsolidatedSafeOutputsJob(data *WorkflowData, mainJobNa
permissions.Merge(NewPermissionsContentsReadPRWrite())
}
if data.SafeOutputs.CreatePullRequests != nil {
permissions.Merge(NewPermissionsContentsWriteIssuesWritePRWrite())
// Check fallback-as-issue setting to determine permissions
if getFallbackAsIssue(data.SafeOutputs.CreatePullRequests) {
permissions.Merge(NewPermissionsContentsWriteIssuesWritePRWrite())
} else {
permissions.Merge(NewPermissionsContentsWritePRWrite())
}
}
if data.SafeOutputs.PushToPullRequestBranch != nil {
permissions.Merge(NewPermissionsContentsWriteIssuesWritePRWrite())
Expand Down
Loading
Loading