Fix runtime setup steps inserted before checkout in custom steps

[Copilot]

When workflows define checkout in custom steps, runtime setup steps (e.g., Setup Node.js, Setup Python) were inserted before the checkout instead of after it.

Changes

Step generation logic (pkg/workflow/compiler_yaml.go)

• Detect when custom steps contain checkout action
• Insert runtime setup steps after first checkout in custom steps
• Add addCustomStepsWithRuntimeInsertion() to parse custom steps and inject runtime steps at correct position
• Add addCustomStepsAsIs() for simple passthrough case

Test coverage (pkg/workflow/checkout_runtime_order_test.go)

• TestCheckoutRuntimeOrderInCustomSteps: Verifies runtime steps inserted after custom checkout
• TestCheckoutFirstWhenNoCustomSteps: Verifies automatic checkout remains first when no custom steps

Example

Before:

- name: Setup Node.js        # ❌ Runtime before checkout
- name: Checkout code
- name: Run Node script

After:

- name: Checkout code        # ✅ Checkout first
- name: Setup Node.js        # ✅ Runtime after checkout
- name: Run Node script

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

• https://api.github.com/repos/githubnext/gh-aw/actions/runs/19531377115/jobs
  • Triggering command: curl -s -H Authorization: token REDACTED (http block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

Original prompt

A checkout step must always be placed as the first step. The runtime step should be placed afterwards.

See this run that failed https://github.com/githubnext/gh-aw/actions/runs/19531377115/job/55914978823

---

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

[Copilot]

Pull Request Overview

This PR fixes a bug where runtime setup steps (e.g., Setup Node.js, Setup Python) were incorrectly inserted before checkout steps when workflows define checkout in custom steps, violating the requirement that checkout must always be the first step.

Key Changes

• Added decision logic to detect when custom steps contain checkout and insert runtime setup steps after the first checkout step instead of before it
• Refactored custom step insertion into two functions: addCustomStepsAsIs for simple cases and addCustomStepsWithRuntimeInsertion for cases requiring runtime step injection
• Added comprehensive test coverage with two test cases verifying correct step ordering in both custom-checkout and automatic-checkout scenarios

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
pkg/workflow/compiler_yaml.go	Implements the core fix with decision logic for runtime step placement and new functions to handle custom step insertion with or without runtime injection
pkg/workflow/checkout_runtime_order_test.go	Adds test coverage for checkout-before-runtime ordering in custom steps and automatic checkout scenarios
.github/workflows/unbloat-docs.lock.yml	Demonstrates the fix working correctly by showing checkout now appears before runtime setup steps

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Significant code duplication: The agent job section extraction logic (lines 210-238) is identical to lines 66-94 in the first test function. Consider extracting this into a helper function:

func extractAgentJobSection(t *testing.T, lockContent string) string {
    agentJobStart := strings.Index(lockContent, "  agent:")
    if agentJobStart == -1 {
        t.Fatal("Could not find agent job in compiled workflow")
    }
    
    remainingContent := lockContent[agentJobStart+10:]
    nextJobStart := -1
    lines := strings.Split(remainingContent, "\n")
    for i, line := range lines {
        if len(line) > 2 && line[0] == ' ' && line[1] == ' ' && line[2] != ' ' && line[2] != '\t' {
            nextJobStart = 0
            for j := 0; j < i; j++ {
                nextJobStart += len(lines[j]) + 1
            }
            break
        }
    }
    
    if nextJobStart == -1 {
        return lockContent[agentJobStart:]
    }
    return lockContent[agentJobStart : agentJobStart+10+nextJobStart]
}

This would improve maintainability and reduce the chance of bugs from inconsistent updates.

[Copilot]

Code duplication: The step name extraction logic (lines 240-253) is identical to lines 100-113 in the first test function. Consider extracting this into a helper function:

func extractStepNames(agentJobSection string) []string {
    stepNames := []string{}
    stepLines := strings.Split(agentJobSection, "\n")
    for _, line := range stepLines {
        if strings.Contains(line, "- name:") {
            parts := strings.SplitN(line, "- name:", 2)
            if len(parts) == 2 {
                name := strings.TrimSpace(parts[1])
                stepNames = append(stepNames, name)
            }
        }
    }
    return stepNames
}

This would reduce code duplication and improve maintainability.

[Copilot]

Code duplication: The test setup logic (lines 177-200) is nearly identical to lines 33-56 in the first test function, with only the temp directory name and workflow content differing. Consider extracting this into a helper function:

func setupWorkflowTest(t *testing.T, testName, workflowContent string) (string, string) {
    tempDir, err := os.MkdirTemp("", testName)
    if err != nil {
        t.Fatalf("Failed to create temp dir: %v", err)
    }
    
    workflowsDir := filepath.Join(tempDir, constants.GetWorkflowDir())
    if err := os.MkdirAll(workflowsDir, 0755); err != nil {
        t.Fatalf("Failed to create workflows directory: %v", err)
    }
    
    workflowPath := filepath.Join(workflowsDir, "test-workflow.md")
    if err := os.WriteFile(workflowPath, []byte(workflowContent), 0644); err != nil {
        t.Fatalf("Failed to write workflow file: %v", err)
    }
    
    compiler := NewCompiler(false, "", "test-version")
    if err := compiler.CompileWorkflow(workflowPath); err != nil {
        t.Fatalf("Failed to compile workflow: %v", err)
    }
    
    lockPath := filepath.Join(workflowsDir, "test-workflow.lock.yml")
    return tempDir, lockPath
}

This would significantly reduce duplication and make the tests more concise.

[Copilot]

The checkout detection logic may fail when the step uses the compact format - uses: actions/checkout@v5. The look-ahead loop (lines 1084-1098) searches for a uses: line with "checkout" starting from i + 1, but when the step starts with - uses: (line i), the uses: is on the current line that was already processed and written to output at line 1073.

This means:

1. If the step is - name: Checkout\n uses: actions/checkout@v5 - works correctly (uses: is on the next line)
2. If the step is - uses: actions/checkout@v5 - fails to detect checkout (uses: is on current line, already written)

Suggested fix: Check the current line for checkout before the look-ahead:

// Check if this line itself contains uses: and checkout (compact format)
if strings.Contains(trimmed, "uses:") && strings.Contains(trimmed, "checkout") {
    isCheckoutStep = true
} else {
    // Look ahead to find "uses:" line with "checkout"
    for j := i + 1; j < len(lines); j++ {
        // ... existing look-ahead logic
    }
}

Suggested change

	// Look ahead to find "uses:" line with "checkout"
	for j := i + 1; j < len(lines); j++ {
	nextLine := lines[j]
	nextTrimmed := strings.TrimSpace(nextLine)
	// Stop if we hit the next step
	if strings.HasPrefix(nextTrimmed, "- name:") || strings.HasPrefix(nextTrimmed, "- uses:") {
	break
	}
	// Check if this is a uses line with checkout
	if strings.Contains(nextTrimmed, "uses:") && strings.Contains(nextTrimmed, "checkout") {
	isCheckoutStep = true
	break
	// Check if this line itself contains uses: and checkout (compact format)
	if strings.Contains(trimmed, "uses:") && strings.Contains(trimmed, "checkout") {
	isCheckoutStep = true
	} else {
	// Look ahead to find "uses:" line with "checkout"
	for j := i + 1; j < len(lines); j++ {
	nextLine := lines[j]
	nextTrimmed := strings.TrimSpace(nextLine)
	// Stop if we hit the next step
	if strings.HasPrefix(nextTrimmed, "- name:") || strings.HasPrefix(nextTrimmed, "- uses:") {
	break
	}
	// Check if this is a uses line with checkout
	if strings.Contains(nextTrimmed, "uses:") && strings.Contains(nextTrimmed, "checkout") {
	isCheckoutStep = true
	break
	}