From e691ec8ea09fc69a799110d2cd5321272763df8e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 7 Feb 2026 05:20:42 +0000 Subject: [PATCH 1/2] Initial plan From 96abff72f964b0aca06f3f5a4ebd0b29c06ec941 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 7 Feb 2026 05:30:43 +0000 Subject: [PATCH 2/2] Fix workflow expression errors by making checkout_pr_success conditional - Add ShouldGeneratePRCheckoutStep() helper to check if checkout-pr step will be generated - Make checkout_pr_success output conditional based on contents read permission - Update notify_comment.go to conditionally add GH_AW_CHECKOUT_PR_SUCCESS env var - Add comprehensive unit tests for ShouldGeneratePRCheckoutStep() - Recompile all workflows - fixes issue-triage-agent and weekly-issue-summary Fixes # Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .github/workflows/issue-triage-agent.lock.yml | 2 - .../workflows/weekly-issue-summary.lock.yml | 2 - pkg/workflow/compiler_activation_jobs.go | 11 ++- pkg/workflow/notify_comment.go | 5 +- pkg/workflow/pr.go | 10 ++- pkg/workflow/pr_test.go | 76 +++++++++++++++++++ 6 files changed, 96 insertions(+), 10 deletions(-) create mode 100644 pkg/workflow/pr_test.go diff --git a/.github/workflows/issue-triage-agent.lock.yml b/.github/workflows/issue-triage-agent.lock.yml index 9fc90010c33..86749e2d16a 100644 --- a/.github/workflows/issue-triage-agent.lock.yml +++ b/.github/workflows/issue-triage-agent.lock.yml @@ -87,7 +87,6 @@ jobs: GH_AW_SAFE_OUTPUTS_CONFIG_PATH: /opt/gh-aw/safeoutputs/config.json GH_AW_SAFE_OUTPUTS_TOOLS_PATH: /opt/gh-aw/safeoutputs/tools.json outputs: - checkout_pr_success: ${{ steps.checkout-pr.outputs.checkout_pr_success || 'true' }} has_patch: ${{ steps.collect_output.outputs.has_patch }} model: ${{ steps.generate_aw_info.outputs.model }} output: ${{ steps.collect_output.outputs.output }} @@ -897,7 +896,6 @@ jobs: GH_AW_RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} GH_AW_AGENT_CONCLUSION: ${{ needs.agent.result }} GH_AW_SECRET_VERIFICATION_RESULT: ${{ needs.agent.outputs.secret_verification_result }} - GH_AW_CHECKOUT_PR_SUCCESS: ${{ needs.agent.outputs.checkout_pr_success }} with: github-token: ${{ secrets.GH_AW_GITHUB_TOKEN || secrets.GITHUB_TOKEN }} script: | diff --git a/.github/workflows/weekly-issue-summary.lock.yml b/.github/workflows/weekly-issue-summary.lock.yml index 0b65d4c804d..f5e7d3fac7c 100644 --- a/.github/workflows/weekly-issue-summary.lock.yml +++ b/.github/workflows/weekly-issue-summary.lock.yml @@ -90,7 +90,6 @@ jobs: GH_AW_SAFE_OUTPUTS_CONFIG_PATH: /opt/gh-aw/safeoutputs/config.json GH_AW_SAFE_OUTPUTS_TOOLS_PATH: /opt/gh-aw/safeoutputs/tools.json outputs: - checkout_pr_success: ${{ steps.checkout-pr.outputs.checkout_pr_success || 'true' }} has_patch: ${{ steps.collect_output.outputs.has_patch }} model: ${{ steps.generate_aw_info.outputs.model }} output: ${{ steps.collect_output.outputs.output }} @@ -1392,7 +1391,6 @@ jobs: GH_AW_RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} GH_AW_AGENT_CONCLUSION: ${{ needs.agent.result }} GH_AW_SECRET_VERIFICATION_RESULT: ${{ needs.agent.outputs.secret_verification_result }} - GH_AW_CHECKOUT_PR_SUCCESS: ${{ needs.agent.outputs.checkout_pr_success }} GH_AW_CREATE_DISCUSSION_ERRORS: ${{ needs.safe_outputs.outputs.create_discussion_errors }} GH_AW_CREATE_DISCUSSION_ERROR_COUNT: ${{ needs.safe_outputs.outputs.create_discussion_error_count }} with: diff --git a/pkg/workflow/compiler_activation_jobs.go b/pkg/workflow/compiler_activation_jobs.go index 562296b299c..6d6d7a3b14e 100644 --- a/pkg/workflow/compiler_activation_jobs.go +++ b/pkg/workflow/compiler_activation_jobs.go @@ -738,11 +738,16 @@ func (c *Compiler) buildMainJob(data *WorkflowData, activationJobCreated bool) ( outputs["has_patch"] = "${{ steps.collect_output.outputs.has_patch }}" } - // Add checkout_pr_success output to track PR checkout status + // Add checkout_pr_success output to track PR checkout status only if the checkout-pr step will be generated // This is used by the conclusion job to skip failure handling when checkout fails // (e.g., when PR is merged and branch is deleted) - outputs["checkout_pr_success"] = "${{ steps.checkout-pr.outputs.checkout_pr_success || 'true' }}" - compilerActivationJobsLog.Print("Added checkout_pr_success output") + // The checkout-pr step is only generated when the workflow has contents read permission + if ShouldGeneratePRCheckoutStep(data) { + outputs["checkout_pr_success"] = "${{ steps.checkout-pr.outputs.checkout_pr_success || 'true' }}" + compilerActivationJobsLog.Print("Added checkout_pr_success output (workflow has contents read access)") + } else { + compilerActivationJobsLog.Print("Skipped checkout_pr_success output (workflow lacks contents read access)") + } // Build job-level environment variables for safe outputs var env map[string]string diff --git a/pkg/workflow/notify_comment.go b/pkg/workflow/notify_comment.go index ec290ef9caf..4aed4b739a9 100644 --- a/pkg/workflow/notify_comment.go +++ b/pkg/workflow/notify_comment.go @@ -163,7 +163,10 @@ func (c *Compiler) buildConclusionJob(data *WorkflowData, mainJobName string, sa } // Add checkout_pr_success to detect PR checkout failures (e.g., PR merged and branch deleted) - agentFailureEnvVars = append(agentFailureEnvVars, fmt.Sprintf(" GH_AW_CHECKOUT_PR_SUCCESS: ${{ needs.%s.outputs.checkout_pr_success }}\n", mainJobName)) + // Only add if the checkout-pr step will be generated (requires contents read access) + if ShouldGeneratePRCheckoutStep(data) { + agentFailureEnvVars = append(agentFailureEnvVars, fmt.Sprintf(" GH_AW_CHECKOUT_PR_SUCCESS: ${{ needs.%s.outputs.checkout_pr_success }}\n", mainJobName)) + } // Pass assignment error outputs from safe_outputs job if assign-to-agent is configured if data.SafeOutputs != nil && data.SafeOutputs.AssignToAgent != nil { diff --git a/pkg/workflow/pr.go b/pkg/workflow/pr.go index 63a094827cd..87dff509c5b 100644 --- a/pkg/workflow/pr.go +++ b/pkg/workflow/pr.go @@ -9,12 +9,18 @@ import ( var prLog = logger.New("workflow:pr") +// ShouldGeneratePRCheckoutStep returns true if the checkout-pr step should be generated +// based on the workflow permissions. The step requires contents read access. +func ShouldGeneratePRCheckoutStep(data *WorkflowData) bool { + permParser := NewPermissionsParser(data.Permissions) + return permParser.HasContentsReadAccess() +} + // generatePRReadyForReviewCheckout generates a step to checkout the PR branch when PR context is available func (c *Compiler) generatePRReadyForReviewCheckout(yaml *strings.Builder, data *WorkflowData) { prLog.Print("Generating PR checkout step") // Check that permissions allow contents read access - permParser := NewPermissionsParser(data.Permissions) - if !permParser.HasContentsReadAccess() { + if !ShouldGeneratePRCheckoutStep(data) { prLog.Print("Skipping PR checkout step: no contents read access") return // No contents read access, cannot checkout } diff --git a/pkg/workflow/pr_test.go b/pkg/workflow/pr_test.go new file mode 100644 index 00000000000..485d3b7c154 --- /dev/null +++ b/pkg/workflow/pr_test.go @@ -0,0 +1,76 @@ +//go:build !integration + +package workflow + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestShouldGeneratePRCheckoutStep(t *testing.T) { + tests := []struct { + name string + permissions string + expected bool + }{ + { + name: "with contents read permission", + permissions: "contents: read", + expected: true, + }, + { + name: "with contents write permission", + permissions: "contents: write", + expected: true, + }, + { + name: "without contents permission", + permissions: "issues: read", + expected: false, + }, + { + name: "with read-all shorthand", + permissions: "read-all", + expected: true, + }, + { + name: "with write-all shorthand", + permissions: "write-all", + expected: true, + }, + { + name: "with none shorthand", + permissions: "none", + expected: false, + }, + { + name: "with all: read", + permissions: `all: read`, + expected: true, + }, + { + name: "multiple permissions including contents", + permissions: `contents: read +issues: write +pull-requests: read`, + expected: true, + }, + { + name: "multiple permissions without contents", + permissions: `issues: write +pull-requests: read`, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + data := &WorkflowData{ + Permissions: tt.permissions, + } + result := ShouldGeneratePRCheckoutStep(data) + assert.Equal(t, tt.expected, result, "ShouldGeneratePRCheckoutStep() result mismatch") + }) + } +}