From e73f069414ca03013bf44c3ea1988259a91948b3 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Feb 2026 02:43:49 +0000 Subject: [PATCH 1/9] Initial plan From a2b97a084aa8d41728821a118f2c693f785ee691 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Feb 2026 02:51:04 +0000 Subject: [PATCH 2/9] Add fallback-as-issue field to create-pull-request config Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/create_pull_request.cjs | 25 +++++++++++++++++++- pkg/workflow/create_pull_request.go | 30 +++++++++++++++++++++--- pkg/workflow/permissions_factory.go | 9 +++++++ 3 files changed, 60 insertions(+), 4 deletions(-) diff --git a/actions/setup/js/create_pull_request.cjs b/actions/setup/js/create_pull_request.cjs index b3e2e47d3df..ec9ab650951 100644 --- a/actions/setup/js/create_pull_request.cjs +++ b/actions/setup/js/create_pull_request.cjs @@ -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; @@ -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; @@ -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: "permission_denied", + }; + } + core.info("Falling back to creating an issue instead"); // Create issue as fallback with enhanced body content diff --git a/pkg/workflow/create_pull_request.go b/pkg/workflow/create_pull_request.go index 24206a4930e..c47eb623913 100644 --- a/pkg/workflow/create_pull_request.go +++ b/pkg/workflow/create_pull_request.go @@ -25,6 +25,7 @@ type CreatePullRequestsConfig struct { AutoMerge bool `yaml:"auto-merge,omitempty"` // Enable auto-merge for the pull request when all required checks pass BaseBranch string `yaml:"base-branch,omitempty"` // Base branch for the pull request (defaults to github.ref_name if not specified) Footer *bool `yaml:"footer,omitempty"` // Controls whether AI-generated footer is added. When false, visible footer is omitted but XML markers are kept. + FallbackAsIssue *bool `yaml:"fallback-as-issue,omitempty"` // When true (default), creates an issue if PR creation fails. When false, no fallback occurs and issues: write permission is not requested. } // buildCreateOutputPullRequestJob creates the create_pull_request job @@ -38,8 +39,12 @@ func (c *Compiler) buildCreateOutputPullRequestJob(data *WorkflowData, mainJobNa if data.SafeOutputs.CreatePullRequests.Draft != nil { draftValue = *data.SafeOutputs.CreatePullRequests.Draft } - createPRLog.Printf("Building create-pull-request job: workflow=%s, main_job=%s, draft=%v, reviewers=%d", - data.Name, mainJobName, draftValue, len(data.SafeOutputs.CreatePullRequests.Reviewers)) + fallbackAsIssue := true // Default + if data.SafeOutputs.CreatePullRequests.FallbackAsIssue != nil { + fallbackAsIssue = *data.SafeOutputs.CreatePullRequests.FallbackAsIssue + } + createPRLog.Printf("Building create-pull-request job: workflow=%s, main_job=%s, draft=%v, reviewers=%d, fallback_as_issue=%v", + data.Name, mainJobName, draftValue, len(data.SafeOutputs.CreatePullRequests.Reviewers), fallbackAsIssue) } // Build pre-steps for patch download, checkout, and git config @@ -92,6 +97,13 @@ func (c *Compiler) buildCreateOutputPullRequestJob(data *WorkflowData, mainJobNa // Pass the auto-merge configuration customEnvVars = append(customEnvVars, fmt.Sprintf(" GH_AW_PR_AUTO_MERGE: %q\n", fmt.Sprintf("%t", data.SafeOutputs.CreatePullRequests.AutoMerge))) + // Pass the fallback-as-issue configuration - default to true for backwards compatibility + fallbackAsIssue := true // Default value + if data.SafeOutputs.CreatePullRequests.FallbackAsIssue != nil { + fallbackAsIssue = *data.SafeOutputs.CreatePullRequests.FallbackAsIssue + } + customEnvVars = append(customEnvVars, fmt.Sprintf(" GH_AW_PR_FALLBACK_AS_ISSUE: %q\n", fmt.Sprintf("%t", fallbackAsIssue))) + // Pass the maximum patch size configuration maxPatchSize := 1024 // Default value if data.SafeOutputs != nil && data.SafeOutputs.MaximumPatchSize > 0 { @@ -151,6 +163,18 @@ func (c *Compiler) buildCreateOutputPullRequestJob(data *WorkflowData, mainJobNa "error_message": "${{ steps.create_pull_request.outputs.error_message }}", } + // Choose permissions based on fallback-as-issue setting + var permissions *Permissions + if fallbackAsIssue { + // Default: include issues: write for fallback behavior + permissions = NewPermissionsContentsWriteIssuesWritePRWrite() + createPRLog.Print("Using permissions with issues:write (fallback-as-issue enabled)") + } else { + // Fallback disabled: only need contents: write and pull-requests: write + permissions = NewPermissionsContentsWritePRWrite() + createPRLog.Print("Using permissions without issues:write (fallback-as-issue disabled)") + } + // Use the shared builder function to create the job return c.buildSafeOutputJob(data, SafeOutputJobConfig{ JobName: "create_pull_request", @@ -159,7 +183,7 @@ func (c *Compiler) buildCreateOutputPullRequestJob(data *WorkflowData, mainJobNa MainJobName: mainJobName, CustomEnvVars: customEnvVars, Script: "", // Legacy - handler manager uses require() to load handler from /tmp/gh-aw/actions - Permissions: NewPermissionsContentsWriteIssuesWritePRWrite(), + Permissions: permissions, Outputs: outputs, PreSteps: preSteps, PostSteps: postSteps, diff --git a/pkg/workflow/permissions_factory.go b/pkg/workflow/permissions_factory.go index 3d7892b1da6..b12667ab146 100644 --- a/pkg/workflow/permissions_factory.go +++ b/pkg/workflow/permissions_factory.go @@ -124,6 +124,15 @@ func NewPermissionsContentsWrite() *Permissions { }) } +// NewPermissionsContentsWritePRWrite creates permissions with contents: write, pull-requests: write +// Used when create-pull-request has fallback-as-issue: false (no issue creation fallback) +func NewPermissionsContentsWritePRWrite() *Permissions { + return NewPermissionsFromMap(map[PermissionScope]PermissionLevel{ + PermissionContents: PermissionWrite, + PermissionPullRequests: PermissionWrite, + }) +} + // NewPermissionsContentsWriteIssuesWritePRWrite creates permissions with contents: write, issues: write, pull-requests: write func NewPermissionsContentsWriteIssuesWritePRWrite() *Permissions { return NewPermissionsFromMap(map[PermissionScope]PermissionLevel{ From 8ee1db695cb6339c05a585338812606fff00e7de Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Feb 2026 03:00:52 +0000 Subject: [PATCH 3/9] Add tests and fix implementation for fallback-as-issue field Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/parser/schemas/main_workflow_schema.json | 5 + pkg/workflow/compile_outputs_pr_test.go | 195 +++++++++++++++++++ pkg/workflow/compiler_safe_outputs_config.go | 3 +- pkg/workflow/compiler_safe_outputs_job.go | 11 +- 4 files changed, 212 insertions(+), 2 deletions(-) diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index bb674fe47f9..abf60d6a3bc 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -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, diff --git a/pkg/workflow/compile_outputs_pr_test.go b/pkg/workflow/compile_outputs_pr_test.go index 87e414a9fee..5eacf5b180e 100644 --- a/pkg/workflow/compile_outputs_pr_test.go +++ b/pkg/workflow/compile_outputs_pr_test.go @@ -606,3 +606,198 @@ 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) + +// Verify permissions include issues: write (default behavior) +if !strings.Contains(lockContentStr, "contents: write") { +t.Error("Expected contents: write permission in create_pull_request job") +} + +if !strings.Contains(lockContentStr, "pull-requests: write") { +t.Error("Expected pull-requests: write permission in create_pull_request job") +} + +// 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") +} +} + diff --git a/pkg/workflow/compiler_safe_outputs_config.go b/pkg/workflow/compiler_safe_outputs_config.go index 0786e33a39e..bb73f63482f 100644 --- a/pkg/workflow/compiler_safe_outputs_config.go +++ b/pkg/workflow/compiler_safe_outputs_config.go @@ -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) diff --git a/pkg/workflow/compiler_safe_outputs_job.go b/pkg/workflow/compiler_safe_outputs_job.go index 4e387d6a566..439efbc3b6e 100644 --- a/pkg/workflow/compiler_safe_outputs_job.go +++ b/pkg/workflow/compiler_safe_outputs_job.go @@ -199,7 +199,16 @@ 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 + fallbackAsIssue := true // Default + if data.SafeOutputs.CreatePullRequests.FallbackAsIssue != nil { + fallbackAsIssue = *data.SafeOutputs.CreatePullRequests.FallbackAsIssue + } + if fallbackAsIssue { + permissions.Merge(NewPermissionsContentsWriteIssuesWritePRWrite()) + } else { + permissions.Merge(NewPermissionsContentsWritePRWrite()) + } } if data.SafeOutputs.PushToPullRequestBranch != nil { permissions.Merge(NewPermissionsContentsWriteIssuesWritePRWrite()) From 1be51ccabaa0f642114fdcabf57cee3b27e2cf01 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Feb 2026 03:07:20 +0000 Subject: [PATCH 4/9] Address code review feedback: extract helper and improve test assertions Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../docs/reference/frontmatter-full.md | 7 + pkg/workflow/compile_outputs_pr_test.go | 218 ++++++++++-------- pkg/workflow/compiler_safe_outputs_job.go | 6 +- pkg/workflow/create_pull_request.go | 40 ++-- 4 files changed, 147 insertions(+), 124 deletions(-) diff --git a/docs/src/content/docs/reference/frontmatter-full.md b/docs/src/content/docs/reference/frontmatter-full.md index 04b941f182f..8874bae7ee5 100644 --- a/docs/src/content/docs/reference/frontmatter-full.md +++ b/docs/src/content/docs/reference/frontmatter-full.md @@ -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 diff --git a/pkg/workflow/compile_outputs_pr_test.go b/pkg/workflow/compile_outputs_pr_test.go index 5eacf5b180e..6dcda131ac5 100644 --- a/pkg/workflow/compile_outputs_pr_test.go +++ b/pkg/workflow/compile_outputs_pr_test.go @@ -608,11 +608,11 @@ This test verifies that auto-merge configuration is properly handled. } func TestOutputPullRequestFallbackAsIssueFalse(t *testing.T) { -// Create temporary directory for test files -tmpDir := testutil.TempDir(t, "output-pr-fallback-false-test") + // 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 := `--- + // Test case with create-pull-request configuration with fallback-as-issue: false + testContent := `--- on: push permissions: contents: read @@ -632,50 +632,50 @@ safe-outputs: 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) -} + 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() + 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) -} + // 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") -} + // 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 == 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 == 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") -} + 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) -} + // 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) -} + // 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) + // 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:") @@ -704,22 +704,22 @@ lockContentStr := string(lockContent) 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") -} + // 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") -} + 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") + // 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 := `--- + // Test case with create-pull-request configuration without fallback-as-issue (should default to true) + testContent := `--- on: push permissions: contents: read @@ -738,66 +738,84 @@ safe-outputs: 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) -} + 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() + compiler := NewCompiler() -// Parse the workflow data -workflowData, err := compiler.ParseWorkflowFile(testFile) -if err != nil { -t.Fatalf("Unexpected error parsing workflow: %v", err) -} + // 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") -} + // 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") -} + 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) -} + // 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) -} + // 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) -} + // 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) + // Convert to string for easier testing + lockContentStr := string(lockContent) -// Verify permissions include issues: write (default behavior) -if !strings.Contains(lockContentStr, "contents: write") { -t.Error("Expected contents: write permission in create_pull_request job") -} + // 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") + } -if !strings.Contains(lockContentStr, "pull-requests: write") { -t.Error("Expected pull-requests: write permission in create_pull_request job") -} + // 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) + } -// 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") -} + safeOutputsJobSection := lockContentStr[safeOutputsJobStart:endIdx] -// 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") -} -} + // Verify permissions in safe_outputs job include issues: write (default behavior) + // Verify permissions 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") + } +} diff --git a/pkg/workflow/compiler_safe_outputs_job.go b/pkg/workflow/compiler_safe_outputs_job.go index 439efbc3b6e..c347d5a8fb0 100644 --- a/pkg/workflow/compiler_safe_outputs_job.go +++ b/pkg/workflow/compiler_safe_outputs_job.go @@ -200,11 +200,7 @@ func (c *Compiler) buildConsolidatedSafeOutputsJob(data *WorkflowData, mainJobNa } if data.SafeOutputs.CreatePullRequests != nil { // Check fallback-as-issue setting to determine permissions - fallbackAsIssue := true // Default - if data.SafeOutputs.CreatePullRequests.FallbackAsIssue != nil { - fallbackAsIssue = *data.SafeOutputs.CreatePullRequests.FallbackAsIssue - } - if fallbackAsIssue { + if getFallbackAsIssue(data.SafeOutputs.CreatePullRequests) { permissions.Merge(NewPermissionsContentsWriteIssuesWritePRWrite()) } else { permissions.Merge(NewPermissionsContentsWritePRWrite()) diff --git a/pkg/workflow/create_pull_request.go b/pkg/workflow/create_pull_request.go index c47eb623913..7213d5aca76 100644 --- a/pkg/workflow/create_pull_request.go +++ b/pkg/workflow/create_pull_request.go @@ -9,22 +9,30 @@ import ( var createPRLog = logger.New("workflow:create_pull_request") +// getFallbackAsIssue returns the effective fallback-as-issue setting (defaults to true) +func getFallbackAsIssue(config *CreatePullRequestsConfig) bool { + if config == nil || config.FallbackAsIssue == nil { + return true // Default + } + return *config.FallbackAsIssue +} + // CreatePullRequestsConfig holds configuration for creating GitHub pull requests from agent output type CreatePullRequestsConfig struct { BaseSafeOutputConfig `yaml:",inline"` TitlePrefix string `yaml:"title-prefix,omitempty"` Labels []string `yaml:"labels,omitempty"` - AllowedLabels []string `yaml:"allowed-labels,omitempty"` // Optional list of allowed labels. If omitted, any labels are allowed (including creating new ones). - Reviewers []string `yaml:"reviewers,omitempty"` // List of users/bots to assign as reviewers to the pull request - Draft *bool `yaml:"draft,omitempty"` // Pointer to distinguish between unset (nil) and explicitly false - IfNoChanges string `yaml:"if-no-changes,omitempty"` // Behavior when no changes to push: "warn" (default), "error", or "ignore" - AllowEmpty bool `yaml:"allow-empty,omitempty"` // Allow creating PR without patch file or with empty patch (useful for preparing feature branches) - TargetRepoSlug string `yaml:"target-repo,omitempty"` // Target repository in format "owner/repo" for cross-repository pull requests - AllowedRepos []string `yaml:"allowed-repos,omitempty"` // List of additional repositories that pull requests can be created in (additionally to the target-repo) - Expires int `yaml:"expires,omitempty"` // Hours until the pull request expires and should be automatically closed (only for same-repo PRs) - AutoMerge bool `yaml:"auto-merge,omitempty"` // Enable auto-merge for the pull request when all required checks pass - BaseBranch string `yaml:"base-branch,omitempty"` // Base branch for the pull request (defaults to github.ref_name if not specified) - Footer *bool `yaml:"footer,omitempty"` // Controls whether AI-generated footer is added. When false, visible footer is omitted but XML markers are kept. + AllowedLabels []string `yaml:"allowed-labels,omitempty"` // Optional list of allowed labels. If omitted, any labels are allowed (including creating new ones). + Reviewers []string `yaml:"reviewers,omitempty"` // List of users/bots to assign as reviewers to the pull request + Draft *bool `yaml:"draft,omitempty"` // Pointer to distinguish between unset (nil) and explicitly false + IfNoChanges string `yaml:"if-no-changes,omitempty"` // Behavior when no changes to push: "warn" (default), "error", or "ignore" + AllowEmpty bool `yaml:"allow-empty,omitempty"` // Allow creating PR without patch file or with empty patch (useful for preparing feature branches) + TargetRepoSlug string `yaml:"target-repo,omitempty"` // Target repository in format "owner/repo" for cross-repository pull requests + AllowedRepos []string `yaml:"allowed-repos,omitempty"` // List of additional repositories that pull requests can be created in (additionally to the target-repo) + Expires int `yaml:"expires,omitempty"` // Hours until the pull request expires and should be automatically closed (only for same-repo PRs) + AutoMerge bool `yaml:"auto-merge,omitempty"` // Enable auto-merge for the pull request when all required checks pass + BaseBranch string `yaml:"base-branch,omitempty"` // Base branch for the pull request (defaults to github.ref_name if not specified) + Footer *bool `yaml:"footer,omitempty"` // Controls whether AI-generated footer is added. When false, visible footer is omitted but XML markers are kept. FallbackAsIssue *bool `yaml:"fallback-as-issue,omitempty"` // When true (default), creates an issue if PR creation fails. When false, no fallback occurs and issues: write permission is not requested. } @@ -39,10 +47,7 @@ func (c *Compiler) buildCreateOutputPullRequestJob(data *WorkflowData, mainJobNa if data.SafeOutputs.CreatePullRequests.Draft != nil { draftValue = *data.SafeOutputs.CreatePullRequests.Draft } - fallbackAsIssue := true // Default - if data.SafeOutputs.CreatePullRequests.FallbackAsIssue != nil { - fallbackAsIssue = *data.SafeOutputs.CreatePullRequests.FallbackAsIssue - } + fallbackAsIssue := getFallbackAsIssue(data.SafeOutputs.CreatePullRequests) createPRLog.Printf("Building create-pull-request job: workflow=%s, main_job=%s, draft=%v, reviewers=%d, fallback_as_issue=%v", data.Name, mainJobName, draftValue, len(data.SafeOutputs.CreatePullRequests.Reviewers), fallbackAsIssue) } @@ -98,10 +103,7 @@ func (c *Compiler) buildCreateOutputPullRequestJob(data *WorkflowData, mainJobNa customEnvVars = append(customEnvVars, fmt.Sprintf(" GH_AW_PR_AUTO_MERGE: %q\n", fmt.Sprintf("%t", data.SafeOutputs.CreatePullRequests.AutoMerge))) // Pass the fallback-as-issue configuration - default to true for backwards compatibility - fallbackAsIssue := true // Default value - if data.SafeOutputs.CreatePullRequests.FallbackAsIssue != nil { - fallbackAsIssue = *data.SafeOutputs.CreatePullRequests.FallbackAsIssue - } + fallbackAsIssue := getFallbackAsIssue(data.SafeOutputs.CreatePullRequests) customEnvVars = append(customEnvVars, fmt.Sprintf(" GH_AW_PR_FALLBACK_AS_ISSUE: %q\n", fmt.Sprintf("%t", fallbackAsIssue))) // Pass the maximum patch size configuration From 4b4cb73a9e543d33bad6e49304aaf4c1a351112b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Feb 2026 03:08:31 +0000 Subject: [PATCH 5/9] Remove duplicate comment in test Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/compile_outputs_pr_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/workflow/compile_outputs_pr_test.go b/pkg/workflow/compile_outputs_pr_test.go index 6dcda131ac5..d79ef284db9 100644 --- a/pkg/workflow/compile_outputs_pr_test.go +++ b/pkg/workflow/compile_outputs_pr_test.go @@ -795,7 +795,6 @@ This workflow tests the create-pull-request with default fallback-as-issue behav safeOutputsJobSection := lockContentStr[safeOutputsJobStart:endIdx] // Verify permissions in safe_outputs job include issues: write (default behavior) - // Verify permissions include issues: write (default behavior) if !strings.Contains(safeOutputsJobSection, "contents: write") { t.Error("Expected contents: write permission in safe_outputs job") } From 298620fc3eecd83cbb7592ab87192ff0ea0053c1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Feb 2026 03:18:08 +0000 Subject: [PATCH 6/9] Set fallback-as-issue: false in unbloat-docs workflow Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .github/workflows/unbloat-docs.lock.yml | 4 ++-- .github/workflows/unbloat-docs.md | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/unbloat-docs.lock.yml b/.github/workflows/unbloat-docs.lock.yml index 6a1bc157bd8..16eeab5ebf4 100644 --- a/.github/workflows/unbloat-docs.lock.yml +++ b/.github/workflows/unbloat-docs.lock.yml @@ -27,7 +27,7 @@ # - shared/mood.md # - shared/reporting.md # -# frontmatter-hash: 7706ca8d2adbcff49f2f7845d093e11087a1080e56faa29aa77269d50216cc1e +# frontmatter-hash: 20e450b369a92da1e54c415aaa26f96042c30a9b632c0a3f7c0eef60775022ae name: "Documentation Unbloat" "on": @@ -1438,7 +1438,7 @@ jobs: uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 env: GH_AW_AGENT_OUTPUT: ${{ env.GH_AW_AGENT_OUTPUT }} - GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG: "{\"add_comment\":{\"max\":1},\"create_pull_request\":{\"auto_merge\":true,\"base_branch\":\"${{ github.ref_name }}\",\"draft\":true,\"expires\":48,\"labels\":[\"documentation\",\"automation\"],\"max\":1,\"max_patch_size\":1024,\"title_prefix\":\"[docs] \"},\"missing_data\":{},\"missing_tool\":{}}" + GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG: "{\"add_comment\":{\"max\":1},\"create_pull_request\":{\"auto_merge\":true,\"base_branch\":\"${{ github.ref_name }}\",\"draft\":true,\"expires\":48,\"fallback_as_issue\":false,\"labels\":[\"documentation\",\"automation\"],\"max\":1,\"max_patch_size\":1024,\"title_prefix\":\"[docs] \"},\"missing_data\":{},\"missing_tool\":{}}" with: github-token: ${{ secrets.GH_AW_GITHUB_TOKEN || secrets.GITHUB_TOKEN }} script: | diff --git a/.github/workflows/unbloat-docs.md b/.github/workflows/unbloat-docs.md index afc3a6527ce..c5ebdd31253 100644 --- a/.github/workflows/unbloat-docs.md +++ b/.github/workflows/unbloat-docs.md @@ -78,6 +78,7 @@ safe-outputs: reviewers: [copilot] draft: true auto-merge: true + fallback-as-issue: false add-comment: max: 1 upload-asset: From 2f963c45d397fa663f930f50561c28464163767b Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Feb 2026 20:09:20 -0800 Subject: [PATCH 7/9] [WIP] Add linting to improve code quality (#15287) --- pkg/workflow/compile_outputs_pr_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/workflow/compile_outputs_pr_test.go b/pkg/workflow/compile_outputs_pr_test.go index d79ef284db9..68b750dd153 100644 --- a/pkg/workflow/compile_outputs_pr_test.go +++ b/pkg/workflow/compile_outputs_pr_test.go @@ -806,7 +806,6 @@ This workflow tests the create-pull-request with default fallback-as-issue behav 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") From dff8a8364802ff336bc1b495c4621172be9e453c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Feb 2026 04:15:41 +0000 Subject: [PATCH 8/9] Fix error_type and add JavaScript tests for fallback-as-issue Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/create_pull_request.cjs | 2 +- actions/setup/js/create_pull_request.test.cjs | 48 +++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 actions/setup/js/create_pull_request.test.cjs diff --git a/actions/setup/js/create_pull_request.cjs b/actions/setup/js/create_pull_request.cjs index ec9ab650951..80fb0dc1d32 100644 --- a/actions/setup/js/create_pull_request.cjs +++ b/actions/setup/js/create_pull_request.cjs @@ -771,7 +771,7 @@ ${patchPreview}`; return { success: false, error: errorMessage, - error_type: "permission_denied", + error_type: "pr_creation_failed", }; } diff --git a/actions/setup/js/create_pull_request.test.cjs b/actions/setup/js/create_pull_request.test.cjs new file mode 100644 index 00000000000..e2e88e781d8 --- /dev/null +++ b/actions/setup/js/create_pull_request.test.cjs @@ -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" + }); + }); +}); From 47c72959136208dd0712d0d95a73d02a74c254cc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Feb 2026 04:18:31 +0000 Subject: [PATCH 9/9] Add `fallback-as-issue` field to disable PR creation fallback Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/create_pull_request.test.cjs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/actions/setup/js/create_pull_request.test.cjs b/actions/setup/js/create_pull_request.test.cjs index e2e88e781d8..09afaa089cb 100644 --- a/actions/setup/js/create_pull_request.test.cjs +++ b/actions/setup/js/create_pull_request.test.cjs @@ -6,21 +6,21 @@ describe("create_pull_request - fallback-as-issue configuration", () => { 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); }); });