Skip to content

Commit 643018f

Browse files
Copilotpelikhan
andauthored
Fix invalid YAML from checkout GitHub App token steps in safe_outputs jobs (#40301)
* Initial plan * Plan regression fix Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * Fix duplicate if injection on checkout app token steps Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * Plan comment follow-up Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * Merge main and refresh checkout pin fixtures Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * Add regression coverage for push-to-pull-request-branch checkout app token condition Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> Co-authored-by: Peli de Halleux <pelikhan@users.noreply.github.com>
1 parent 2d9d6b1 commit 643018f

2 files changed

Lines changed: 107 additions & 4 deletions

File tree

pkg/workflow/checkout_step_generator.go

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,8 @@ func wikiRepository(repository string) string {
2727
// referenced in the corresponding checkout step.
2828
//
2929
// The step ID for each checkout is "checkout-app-token-{index}" where index is
30-
// the position in the ordered checkout list.
30+
// the position in the ordered checkout list. Each returned slice element is a
31+
// complete YAML step string, matching injectStepCondition's whole-step contract.
3132
func (cm *CheckoutManager) GenerateCheckoutAppTokenSteps(c *Compiler, permissions *Permissions) []string {
3233
checkoutManagerLog.Printf("Building app token minting steps for %d checkout entries", len(cm.ordered))
3334
var steps []string
@@ -38,14 +39,34 @@ func (cm *CheckoutManager) GenerateCheckoutAppTokenSteps(c *Compiler, permission
3839
checkoutManagerLog.Printf("Generating app token minting step for checkout index=%d repo=%q", checkoutIndex, entry.key.repository)
3940
// Pass empty fallback so the app token defaults to github.event.repository.name.
4041
// Checkout-specific cross-repo scoping is handled via the explicit repository field.
41-
steps = append(steps, c.buildGitHubAppTokenMintStepWithMeta(
42+
steps = append(steps, collapseYAMLLinesIntoSteps(c.buildGitHubAppTokenMintStepWithMeta(
4243
entry.githubApp,
4344
permissions,
4445
"",
4546
entry.key.repository,
4647
fmt.Sprintf("Generate GitHub App token for checkout (%d)", checkoutIndex),
4748
fmt.Sprintf("checkout-app-token-%d", checkoutIndex),
48-
)...)
49+
))...)
50+
}
51+
return steps
52+
}
53+
54+
func collapseYAMLLinesIntoSteps(lines []string) []string {
55+
if len(lines) == 0 {
56+
return nil
57+
}
58+
59+
var steps []string
60+
var current strings.Builder
61+
for _, line := range lines {
62+
if strings.HasPrefix(line, " - ") && current.Len() > 0 {
63+
steps = append(steps, current.String())
64+
current.Reset()
65+
}
66+
current.WriteString(line)
67+
}
68+
if current.Len() > 0 {
69+
steps = append(steps, current.String())
4970
}
5071
return steps
5172
}

pkg/workflow/duplicate_step_validation_integration_test.go

Lines changed: 83 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,10 @@ import (
99
"testing"
1010

1111
"github.com/github/gh-aw/pkg/stringutil"
12-
1312
"github.com/github/gh-aw/pkg/testutil"
13+
"github.com/goccy/go-yaml"
14+
"github.com/stretchr/testify/assert"
15+
"github.com/stretchr/testify/require"
1416
)
1517

1618
// TestDuplicateStepValidation_Integration tests that the duplicate step validation
@@ -188,3 +190,83 @@ compile without duplicate 'Generate GitHub App token' step errors in the activat
188190

189191
t.Logf("✓ No duplicate token steps: checkout (0) count=%d, checkout (1) count=%d, generic in agent=%d", count0, count1, genericCountInAgent)
190192
}
193+
194+
// TestDuplicateStepValidation_CheckoutAppTokenCondition_Integration verifies that
195+
// safe_outputs checkout app-token steps remain valid YAML when PR-producing
196+
// safe outputs inject a shared condition onto mirrored checkout steps.
197+
func TestDuplicateStepValidation_CheckoutAppTokenCondition_Integration(t *testing.T) {
198+
cases := []struct {
199+
name string
200+
safeOutputConfigLine string
201+
outputType string
202+
}{
203+
{
204+
name: "create_pull_request",
205+
safeOutputConfigLine: "create-pull-request: {}",
206+
outputType: "create_pull_request",
207+
},
208+
{
209+
name: "push_to_pull_request_branch",
210+
safeOutputConfigLine: "push-to-pull-request-branch: {}",
211+
outputType: "push_to_pull_request_branch",
212+
},
213+
}
214+
215+
for _, tc := range cases {
216+
t.Run(tc.name, func(t *testing.T) {
217+
tmpDir := testutil.TempDir(t, "duplicate-checkout-app-token-if-test")
218+
219+
mdContent := `---
220+
on:
221+
issues:
222+
types: [opened]
223+
engine: claude
224+
strict: false
225+
permissions:
226+
contents: read
227+
issues: read
228+
pull-requests: read
229+
checkout:
230+
github-app:
231+
app-id: ${{ secrets.APP_ID }}
232+
private-key: ${{ secrets.APP_PRIVATE_KEY }}
233+
safe-outputs:
234+
` + tc.safeOutputConfigLine + `
235+
---
236+
237+
# Test Workflow
238+
`
239+
240+
mdFile := filepath.Join(tmpDir, "test-checkout-app-token-condition.md")
241+
err := os.WriteFile(mdFile, []byte(mdContent), 0644)
242+
require.NoError(t, err, "Failed to create test file")
243+
244+
compiler := NewCompiler()
245+
err = compiler.CompileWorkflow(mdFile)
246+
require.NoError(t, err, "Regression: checkout github-app + PR-producing safe output should compile successfully")
247+
248+
lockFile := stringutil.MarkdownToLockFile(mdFile)
249+
lockContent, err := os.ReadFile(lockFile)
250+
require.NoError(t, err, "Failed to read lock file")
251+
252+
var workflow map[string]any
253+
require.NoError(t, yaml.Unmarshal(lockContent, &workflow), "compiled lock file should be valid YAML")
254+
255+
safeOutputsSection := extractJobSection(string(lockContent), "safe_outputs")
256+
stepStart := strings.Index(safeOutputsSection, " - name: Generate GitHub App token for checkout (0)\n")
257+
require.NotEqual(t, -1, stepStart, "safe_outputs job should contain the checkout app-token step")
258+
259+
stepEnd := strings.Index(safeOutputsSection[stepStart+1:], "\n - ")
260+
if stepEnd == -1 {
261+
stepEnd = len(safeOutputsSection)
262+
} else {
263+
stepEnd += stepStart + 1
264+
}
265+
stepBlock := safeOutputsSection[stepStart:stepEnd]
266+
267+
assert.Equal(t, 1, strings.Count(stepBlock, "\n if: "), "checkout app-token step should have exactly one injected if condition")
268+
assert.Contains(t, stepBlock, "id: checkout-app-token-0")
269+
assert.Contains(t, stepBlock, tc.outputType)
270+
})
271+
}
272+
}

0 commit comments

Comments
 (0)