From 990e9149e9a847060dd8d9a4094888ccd7631464 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 23 Jul 2026 10:41:26 +0000 Subject: [PATCH 1/7] Initial plan From 689166e06b21fbda8b34571e6223d81a8f7e0cb2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 23 Jul 2026 10:59:02 +0000 Subject: [PATCH 2/7] Fix custom checkout runtime insertion Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/checkout_runtime_order_test.go | 210 ++++++++++++++++++++ pkg/workflow/compiler_workflow_helpers.go | 34 +++- pkg/workflow/compiler_yaml_main_job_test.go | 10 +- pkg/workflow/compiler_yaml_runtime_setup.go | 38 ++-- pkg/workflow/permissions_parser_test.go | 8 +- 5 files changed, 259 insertions(+), 41 deletions(-) diff --git a/pkg/workflow/checkout_runtime_order_test.go b/pkg/workflow/checkout_runtime_order_test.go index 016a5874dc6..b13123b917e 100644 --- a/pkg/workflow/checkout_runtime_order_test.go +++ b/pkg/workflow/checkout_runtime_order_test.go @@ -220,6 +220,126 @@ steps: } } +func TestCheckoutRuntimeOrderInCustomStepVariants(t *testing.T) { + tests := []struct { + name string + customSteps string + orderedMarkers []string + wantSetupNodeNames int + wantSetupNodeActions int + }{ + { + name: "unnamed checkout with options as first custom step", + customSteps: ` - uses: actions/checkout@v5 + with: + fetch-depth: 0 + persist-credentials: false + - name: Prepare context + run: echo ready`, + orderedMarkers: []string{ + "uses: actions/checkout@", + "- name: Setup Node.js", + "- name: Prepare context", + }, + wantSetupNodeNames: 1, + wantSetupNodeActions: 1, + }, + { + name: "named checkout with options as first custom step", + customSteps: ` - name: Checkout repository + uses: actions/checkout@v5 + with: + fetch-depth: 0 + persist-credentials: false + - name: Prepare context + run: echo ready`, + orderedMarkers: []string{ + "uses: actions/checkout@", + "- name: Setup Node.js", + "- name: Prepare context", + }, + wantSetupNodeNames: 1, + wantSetupNodeActions: 1, + }, + { + name: "checkout after deterministic step", + customSteps: ` - name: Prepare context + run: echo before + - uses: actions/checkout@v5 + with: + persist-credentials: false + - name: After checkout + run: echo after`, + orderedMarkers: []string{ + "- name: Prepare context", + "uses: actions/checkout@", + "- name: Setup Node.js", + "- name: After checkout", + }, + wantSetupNodeNames: 1, + wantSetupNodeActions: 1, + }, + { + name: "multiple checkout steps insert runtime once after first checkout", + customSteps: ` - name: Prepare context + run: echo before + - uses: actions/checkout@v5 + with: + persist-credentials: false + - name: After first checkout + run: echo middle + - uses: actions/checkout@v5 + with: + persist-credentials: false + - name: After second checkout + run: echo done`, + orderedMarkers: []string{ + "- name: Prepare context", + "uses: actions/checkout@", + "- name: Setup Node.js", + "- name: After first checkout", + "uses: actions/checkout@", + "- name: After second checkout", + }, + wantSetupNodeNames: 1, + wantSetupNodeActions: 1, + }, + { + name: "customized runtime setup action is preserved without duplication", + customSteps: ` - uses: actions/checkout@v5 + with: + persist-credentials: false + - name: Setup Node from file + uses: actions/setup-node@v5 + with: + node-version-file: .nvmrc + - name: Prepare context + run: echo ready`, + orderedMarkers: []string{ + "uses: actions/checkout@", + "- name: Setup Node from file", + "- name: Prepare context", + }, + wantSetupNodeNames: 0, + wantSetupNodeActions: 1, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + agentJobSection := compileWorkflowAndGetAgentJobSection(t, tt.customSteps) + assertOrderedMarkers(t, agentJobSection, tt.orderedMarkers) + + if got := strings.Count(agentJobSection, "- name: Setup Node.js"); got != tt.wantSetupNodeNames { + t.Fatalf("expected %d generated Setup Node.js steps, got %d\n%s", tt.wantSetupNodeNames, got, agentJobSection) + } + if got := strings.Count(agentJobSection, "uses: actions/setup-node@"); got != tt.wantSetupNodeActions { + t.Fatalf("expected %d setup-node actions, got %d\n%s", tt.wantSetupNodeActions, got, agentJobSection) + } + }) + } +} + // TestCheckoutFirstWhenNoCustomSteps verifies that when there are no custom steps, // the automatic checkout is added first. func TestCheckoutFirstWhenNoCustomSteps(t *testing.T) { @@ -356,3 +476,93 @@ Run node --version to check the Node.js version. t.Logf(" 3. %s", stepNames[2]) t.Logf(" 4. %s", stepNames[3]) } + +func compileWorkflowAndGetAgentJobSection(t *testing.T, customSteps string) string { + t.Helper() + + workflowContent := `--- +on: workflow_dispatch +engine: copilot +runs-on: self-hosted +permissions: + contents: read + issues: read + pull-requests: read +steps: +` + customSteps + ` +--- + +# Test workflow with custom checkout steps +` + + tempDir, err := os.MkdirTemp("", "checkout-runtime-order-variant") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer os.RemoveAll(tempDir) + + 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() + compiler.SetActionMode(ActionModeDev) + if err := compiler.CompileWorkflow(workflowPath); err != nil { + t.Fatalf("Failed to compile workflow: %v", err) + } + + lockPath := filepath.Join(workflowsDir, "test-workflow.lock.yml") + lockContent, err := os.ReadFile(lockPath) + if err != nil { + t.Fatalf("Failed to read lock file: %v", err) + } + + return extractAgentJobSection(t, string(lockContent)) +} + +func extractAgentJobSection(t *testing.T, lockStr string) string { + t.Helper() + + agentJobStart := strings.Index(lockStr, " agent:") + if agentJobStart == -1 { + t.Fatal("Could not find agent job in compiled workflow") + } + + remainingContent := lockStr[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 := range i { + nextJobStart += len(lines[j]) + 1 + } + break + } + } + + if nextJobStart == -1 { + return lockStr[agentJobStart:] + } + + return lockStr[agentJobStart : agentJobStart+10+nextJobStart] +} + +func assertOrderedMarkers(t *testing.T, content string, markers []string) { + t.Helper() + + lastIndex := -1 + for _, marker := range markers { + index := strings.Index(content[lastIndex+1:], marker) + if index == -1 { + t.Fatalf("marker %q not found after index %d\n%s", marker, lastIndex, content) + } + lastIndex += index + 1 + } +} diff --git a/pkg/workflow/compiler_workflow_helpers.go b/pkg/workflow/compiler_workflow_helpers.go index 4073f3dc1b1..b64b57c0511 100644 --- a/pkg/workflow/compiler_workflow_helpers.go +++ b/pkg/workflow/compiler_workflow_helpers.go @@ -4,6 +4,8 @@ import ( "path/filepath" "strings" + "github.com/goccy/go-yaml" + "github.com/github/gh-aw/pkg/logger" ) @@ -11,26 +13,36 @@ var compilerWorkflowHelpersLog = logger.New("workflow:compiler_workflow_helpers" // ContainsCheckout returns true if the given custom steps contain an actions/checkout step func ContainsCheckout(customSteps string) bool { + _, found := findFirstCheckoutStepIndex(customSteps) + return found +} + +func findFirstCheckoutStepIndex(customSteps string) (int, bool) { if customSteps == "" { - return false + return 0, false } - // Look for actions/checkout usage patterns - checkoutPatterns := []string{ - "actions/checkout@", - "uses: actions/checkout", - "- uses: actions/checkout", + var wrapper struct { + Steps []map[string]any `yaml:"steps"` + } + if err := yaml.Unmarshal([]byte(customSteps), &wrapper); err != nil { + return 0, false } - lowerSteps := strings.ToLower(customSteps) - for _, pattern := range checkoutPatterns { - if strings.Contains(lowerSteps, strings.ToLower(pattern)) { + for i, step := range wrapper.Steps { + uses, ok := step["uses"].(string) + if ok && isCheckoutActionReference(uses) { compilerWorkflowHelpersLog.Print("Detected actions/checkout in custom steps") - return true + return i, true } } - return false + return 0, false +} + +func isCheckoutActionReference(uses string) bool { + normalized := strings.ToLower(strings.TrimSpace(strings.Trim(uses, `"'`))) + return normalized == "actions/checkout" || strings.HasPrefix(normalized, "actions/checkout@") } // GetWorkflowIDFromPath extracts the workflow ID from a markdown file path. diff --git a/pkg/workflow/compiler_yaml_main_job_test.go b/pkg/workflow/compiler_yaml_main_job_test.go index 34d1e3cd96b..3e35d4d15b8 100644 --- a/pkg/workflow/compiler_yaml_main_job_test.go +++ b/pkg/workflow/compiler_yaml_main_job_test.go @@ -433,7 +433,7 @@ func TestAddCustomStepsWithRuntimeInsertion(t *testing.T) { insertionHappened: false, }, { - name: "checkout with uses on same line - not detected as checkout", + name: "checkout with uses on same line", customSteps: `steps: - uses: actions/checkout@v4 - name: Build @@ -444,12 +444,14 @@ func TestAddCustomStepsWithRuntimeInsertion(t *testing.T) { tools: &ToolsConfig{}, expectInOutput: []string{ "- uses: actions/checkout@v4", + "- name: Setup Go", "- name: Build", }, - notInOutput: []string{ - "Setup Go", // Won't be inserted - function doesn't detect "- uses:" as checkout + expectStepOrder: []string{ + "Setup Go", + "Build", }, - insertionHappened: false, + insertionHappened: true, }, { name: "multiple runtime steps inserted", diff --git a/pkg/workflow/compiler_yaml_runtime_setup.go b/pkg/workflow/compiler_yaml_runtime_setup.go index adbadd6747d..d220db849d9 100644 --- a/pkg/workflow/compiler_yaml_runtime_setup.go +++ b/pkg/workflow/compiler_yaml_runtime_setup.go @@ -324,6 +324,7 @@ func (c *Compiler) addCustomStepsAsIs(yaml *strings.Builder, customSteps string) // Like addCustomStepsAsIs it sanitizes any ${{ ... }} expressions found in run: fields before writing. func (c *Compiler) addCustomStepsWithRuntimeInsertion(yaml *strings.Builder, customSteps string, runtimeSetupSteps []GitHubActionStep, tools *ToolsConfig) { customSteps = c.sanitizeAndWarnCustomSteps(customSteps) + checkoutStepIndex, hasCheckoutStep := findFirstCheckoutStepIndex(customSteps) // Remove "steps:" line and adjust indentation lines := strings.Split(customSteps, "\n") if len(lines) <= 1 { @@ -332,6 +333,8 @@ func (c *Compiler) addCustomStepsWithRuntimeInsertion(yaml *strings.Builder, cus insertedRuntime := false i := 1 // Start from index 1 to skip "steps:" line + currentStepIndex := -1 + stepIndent := -1 var blockScalarState yamlBlockScalarState for i < len(lines) { @@ -348,30 +351,20 @@ func (c *Compiler) addCustomStepsWithRuntimeInsertion(yaml *strings.Builder, cus // Add the line with proper indentation appendYAMLLine(yaml, " ", line, isBS) - // Check if this line starts a step with "- name:" or "- uses:" + // Check if this line starts a top-level step trimmed := strings.TrimSpace(line) - isStepStart := strings.HasPrefix(trimmed, "- name:") || strings.HasPrefix(trimmed, "- uses:") + indent := len(line) - len(strings.TrimLeft(line, " ")) + isStepStart := strings.HasPrefix(trimmed, "- ") + if isStepStart { + if stepIndent == -1 { + stepIndent = indent + } + isStepStart = indent == stepIndent + } if isStepStart && !insertedRuntime { - // This is the start of a step, check if it's a checkout step - isCheckoutStep := false - - // 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 - } - } + currentStepIndex++ + isCheckoutStep := hasCheckoutStep && currentStepIndex == checkoutStepIndex if isCheckoutStep { // This is a checkout step, copy all its lines until the next step @@ -379,9 +372,10 @@ func (c *Compiler) addCustomStepsWithRuntimeInsertion(yaml *strings.Builder, cus for i < len(lines) { nextLine := lines[i] nextTrimmed := strings.TrimSpace(nextLine) + nextIndent := len(nextLine) - len(strings.TrimLeft(nextLine, " ")) // Stop if we hit the next step - if strings.HasPrefix(nextTrimmed, "- name:") || strings.HasPrefix(nextTrimmed, "- uses:") { + if nextTrimmed != "" && strings.HasPrefix(nextTrimmed, "- ") && nextIndent == stepIndent { break } diff --git a/pkg/workflow/permissions_parser_test.go b/pkg/workflow/permissions_parser_test.go index 5ef8c9cc465..85475ad3b47 100644 --- a/pkg/workflow/permissions_parser_test.go +++ b/pkg/workflow/permissions_parser_test.go @@ -324,7 +324,7 @@ func TestContainsCheckout(t *testing.T) { customSteps: `steps: - name: Echo checkout run: echo "actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd"`, - expected: true, // Current implementation does simple string match + expected: false, }, { name: "checkout in comment (should not match)", @@ -332,14 +332,14 @@ func TestContainsCheckout(t *testing.T) { - name: Setup uses: actions/setup-node@395ad3262231945c25e8478fd5baf05154b1d79f # TODO: add actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd`, - expected: true, // Current implementation does simple string match + expected: false, }, { name: "similar but not checkout action", customSteps: `steps: - uses: actions/cache@v3 - uses: my-actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd`, - expected: true, // Current implementation matches substring + expected: false, }, { name: "checkout in different format", @@ -353,7 +353,7 @@ func TestContainsCheckout(t *testing.T) { name: "malformed YAML with checkout", customSteps: `steps - uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd`, - expected: true, // Still detects the string + expected: false, }, { name: "checkout with complex parameters", From 5efe4cd266fea6fb1fc0daed0e7cc621b6ad442e Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 23 Jul 2026 11:46:09 +0000 Subject: [PATCH 3/7] Add draft ADR-47545: YAML-parsed checkout detection for workflow compiler --- ...heckout-detection-for-workflow-compiler.md | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 docs/adr/47545-yaml-parsed-checkout-detection-for-workflow-compiler.md diff --git a/docs/adr/47545-yaml-parsed-checkout-detection-for-workflow-compiler.md b/docs/adr/47545-yaml-parsed-checkout-detection-for-workflow-compiler.md new file mode 100644 index 00000000000..6be15f3287f --- /dev/null +++ b/docs/adr/47545-yaml-parsed-checkout-detection-for-workflow-compiler.md @@ -0,0 +1,48 @@ +# ADR-47545: YAML-Parsed Checkout Detection for Workflow Compiler + +**Date**: 2026-07-23 +**Status**: Draft +**Deciders**: Unknown + +--- + +### Context + +The workflow compiler inserts runtime setup steps (e.g., Node.js, ARC/DinD) immediately after the first `actions/checkout` step in user-defined custom steps. This deferral is a two-phase operation: a detection phase determines whether custom steps contain a checkout (triggering deferral), and an insertion phase locates the checkout step to insert runtime setup after it. The original implementation used substring string matching in both phases. However, the insertion phase looked ahead from `i + 1` lines, so when a checkout appeared as `- uses: actions/checkout@v6` (the unnamed shorthand), the action reference was on the current line — never examined by the look-ahead — causing the deferred runtime setup to be silently dropped from the generated workflow. Named checkout forms (`- name: Checkout\n uses: actions/checkout@v6`) worked because the `uses:` key appeared on a subsequent line. Any workow using the unnamed shorthand was silently broken. + +### Decision + +We will replace the dual substring-based checkout matching with a single YAML-parsed helper (`findFirstCheckoutStepIndex`) that unmarshals the custom-steps YAML, iterates the parsed step list, and returns the zero-based index of the first step whose `uses` field exactly matches `actions/checkout` or starts with `actions/checkout@`. Both the `ContainsCheckout` detection function and the `addCustomStepsWithRuntimeInsertion` insertion function will share this helper, eliminating the detection/insertion mismatch. The insertion phase uses the pre-computed step index rather than per-line look-ahead to identify the target step during line-by-line YAML rendering. + +### Alternatives Considered + +#### Alternative 1: Fix the Look-Ahead in the Insertion Phase Only + +The minimal fix would be to also examine the current `- uses:` line (not just `i + 1` onwards) in the existing look-ahead loop inside `addCustomStepsWithRuntimeInsertion`. This would resolve the immediate bug without changing the detection architecture. + +Why not chosen: This fix would leave two independent and semantically inconsistent checkout detectors in the codebase — one using substring matching (detection phase) and one using look-ahead string scanning (insertion phase). Future checkout-form edge cases (e.g., quoted action references, whitespace variants) could reintroduce a mismatch. The substring detector also produced false positives on comments, `run:` field text, and action names containing "checkout" (as documented by the test cases changed in `permissions_parser_test.go`). + +#### Alternative 2: Parse YAML Once at a Higher Level and Pass a Shared Representation + +A more architecturally ambitious approach would parse the full custom-steps YAML once at the call site of `emitCustomSteps` and pass structured step data into both detection and insertion functions, eliminating re-parsing. + +Why not chosen: This approach requires refactoring multiple function signatures and callers that currently pass `customSteps` as a raw string throughout the compiler pipeline. The single shared `findFirstCheckoutStepIndex` helper achieves consistent detection at lower refactoring cost, since it is called once per compilation path and its result is threaded into both phases within the same call chain. + +### Consequences + +#### Positive +- Checkout detection is now exact: YAML parsing rejects false positives from comments, `run:` field text (`echo "actions/checkout@..."`), and action names that merely contain "checkout" (e.g., `my-actions/checkout@...`). +- Named and unnamed checkout forms (`- name: Checkout\n uses:` vs `- uses:`) are handled equivalently, eliminating the behavioral asymmetry that caused the bug. +- The first checkout step index is computed once and shared between detection and insertion, removing the structural possibility of the two phases diverging. + +#### Negative +- The `go-yaml` package is now imported directly in `compiler_workflow_helpers.go`; previously this helper file had no YAML dependency. If the YAML library ever changes parse behavior, checkout detection will change with it. +- Malformed custom-steps YAML (e.g., missing `steps:` key or a top-level syntax error) now causes `findFirstCheckoutStepIndex` to return `(0, false)`, meaning checkout is not detected and no deferral occurs. Previously the substring matcher would still detect checkout in malformed YAML. The practical impact is low: malformed YAML also fails later compilation stages. + +#### Neutral +- The `isCheckoutActionReference` helper normalizes the `uses` value (trims whitespace, strips surrounding quotes, lowercases) before comparison, making the check robust to minor formatting variation without adding regex complexity. +- Step boundary detection in the insertion loop is generalized from `- name:` or `- uses:` prefix checks to any `- ` prefix at the same indentation level, matching actual YAML list semantics more closely. + +--- + +*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.* From abf5655c8d7c5f81741300a8b1bc6feeb742d82d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 23 Jul 2026 12:15:11 +0000 Subject: [PATCH 4/7] Address 5 review threads: bare-sequence checkout, block-scalar guard, assertOrderedMarkers fix, comments Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- pkg/workflow/checkout_runtime_order_test.go | 10 +++---- pkg/workflow/compiler_workflow_helpers.go | 26 +++++++++++++++-- pkg/workflow/compiler_yaml_line_writer.go | 7 +++++ pkg/workflow/compiler_yaml_runtime_setup.go | 14 ++++++++-- pkg/workflow/permissions_parser_test.go | 31 +++++++++++++++++++++ 5 files changed, 78 insertions(+), 10 deletions(-) diff --git a/pkg/workflow/checkout_runtime_order_test.go b/pkg/workflow/checkout_runtime_order_test.go index b13123b917e..d234d59c37e 100644 --- a/pkg/workflow/checkout_runtime_order_test.go +++ b/pkg/workflow/checkout_runtime_order_test.go @@ -557,12 +557,12 @@ func extractAgentJobSection(t *testing.T, lockStr string) string { func assertOrderedMarkers(t *testing.T, content string, markers []string) { t.Helper() - lastIndex := -1 + cursor := 0 for _, marker := range markers { - index := strings.Index(content[lastIndex+1:], marker) - if index == -1 { - t.Fatalf("marker %q not found after index %d\n%s", marker, lastIndex, content) + idx := strings.Index(content[cursor:], marker) + if idx == -1 { + t.Fatalf("marker %q not found after offset %d\n%s", marker, cursor, content) } - lastIndex += index + 1 + cursor += idx + len(marker) } } diff --git a/pkg/workflow/compiler_workflow_helpers.go b/pkg/workflow/compiler_workflow_helpers.go index b64b57c0511..43ae32c31f1 100644 --- a/pkg/workflow/compiler_workflow_helpers.go +++ b/pkg/workflow/compiler_workflow_helpers.go @@ -17,19 +17,41 @@ func ContainsCheckout(customSteps string) bool { return found } +// findFirstCheckoutStepIndex returns the zero-based index of the first +// actions/checkout step in customSteps and true, or (0, false) if none is found. +// +// It accepts both the wrapped form (with a "steps:" key) and the bare sequence +// form (a YAML list) that older call sites may produce. If the YAML cannot be +// parsed in either form the function returns (0, false): the caller should not +// attempt checkout-step insertion when the step list is unparseable. func findFirstCheckoutStepIndex(customSteps string) (int, bool) { if customSteps == "" { return 0, false } + // Try the wrapped form first: "steps:\n - ...\n" var wrapper struct { Steps []map[string]any `yaml:"steps"` } - if err := yaml.Unmarshal([]byte(customSteps), &wrapper); err != nil { + if err := yaml.Unmarshal([]byte(customSteps), &wrapper); err == nil && len(wrapper.Steps) > 0 { + for i, step := range wrapper.Steps { + uses, ok := step["uses"].(string) + if ok && isCheckoutActionReference(uses) { + compilerWorkflowHelpersLog.Print("Detected actions/checkout in custom steps") + return i, true + } + } + return 0, false + } + + // Fall back to the bare sequence form: "- uses: ...\n" + var steps []map[string]any + if err := yaml.Unmarshal([]byte(customSteps), &steps); err != nil { + // Malformed YAML: we cannot safely determine a checkout step index. return 0, false } - for i, step := range wrapper.Steps { + for i, step := range steps { uses, ok := step["uses"].(string) if ok && isCheckoutActionReference(uses) { compilerWorkflowHelpersLog.Print("Detected actions/checkout in custom steps") diff --git a/pkg/workflow/compiler_yaml_line_writer.go b/pkg/workflow/compiler_yaml_line_writer.go index 7c9c55b7147..ed6a751b26c 100644 --- a/pkg/workflow/compiler_yaml_line_writer.go +++ b/pkg/workflow/compiler_yaml_line_writer.go @@ -72,6 +72,13 @@ func (s *yamlBlockScalarState) update(sourceLine string) bool { return false } +// IsInPayload reports whether the state machine is currently inside a block +// scalar payload (i.e. the last consumed line was block-scalar content). +// Unlike update, this is a pure read that does not advance the state. +func (s *yamlBlockScalarState) IsInPayload() bool { + return s.active +} + // appendYAMLLine writes content to b with the given prefix. // // - Blank content (empty or whitespace-only) is always written as a bare diff --git a/pkg/workflow/compiler_yaml_runtime_setup.go b/pkg/workflow/compiler_yaml_runtime_setup.go index d220db849d9..58bee0841ae 100644 --- a/pkg/workflow/compiler_yaml_runtime_setup.go +++ b/pkg/workflow/compiler_yaml_runtime_setup.go @@ -98,6 +98,12 @@ func (c *Compiler) prepareRuntimeSetupAndCheckoutInfo(data *WorkflowData) ([]Git runtimeSetupSteps := GenerateRuntimeSetupSteps(runtimeRequirements, data) compilerYamlLog.Printf("Detected runtime requirements: %d runtimes, %d setup steps", len(runtimeRequirements), len(runtimeSetupSteps)) + // Determine whether the (post-deduplication) custom steps contain a checkout + // step. This check runs before sanitizeAndWarnCustomSteps is called inside + // addCustomStepsWithRuntimeInsertion, but that is safe: sanitization only + // rewrites ${{ }} expressions inside `run:` fields and never touches `uses:` + // values, so the checkout-detection result is identical before and after + // sanitization. customStepsContainCheckout := data.CustomSteps != "" && ContainsCheckout(data.CustomSteps) return runtimeSetupSteps, customStepsContainCheckout @@ -374,12 +380,14 @@ func (c *Compiler) addCustomStepsWithRuntimeInsertion(yaml *strings.Builder, cus nextTrimmed := strings.TrimSpace(nextLine) nextIndent := len(nextLine) - len(strings.TrimLeft(nextLine, " ")) - // Stop if we hit the next step - if nextTrimmed != "" && strings.HasPrefix(nextTrimmed, "- ") && nextIndent == stepIndent { + // Stop if we hit the next step, but only when we are not inside a + // block scalar payload (e.g. "sparse-checkout: |\n - src" — the + // "- src" content line starts with "- " but is not a step boundary). + if !blockScalarState.IsInPayload() && nextTrimmed != "" && strings.HasPrefix(nextTrimmed, "- ") && nextIndent == stepIndent { break } - // Add the line + // Add the line (this also advances the block scalar state machine) nextIsBS := blockScalarState.update(nextLine) appendYAMLLine(yaml, " ", nextLine, nextIsBS) i++ diff --git a/pkg/workflow/permissions_parser_test.go b/pkg/workflow/permissions_parser_test.go index 85475ad3b47..62d98df67c3 100644 --- a/pkg/workflow/permissions_parser_test.go +++ b/pkg/workflow/permissions_parser_test.go @@ -350,6 +350,7 @@ func TestContainsCheckout(t *testing.T) { expected: true, }, { + // Malformed YAML: returns false — see findFirstCheckoutStepIndex godoc. name: "malformed YAML with checkout", customSteps: `steps - uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd`, @@ -405,6 +406,36 @@ func TestContainsCheckout(t *testing.T) { run: echo "fifth"`, expected: true, }, + // Bare sequence form (no "steps:" wrapper): older call sites pass the + // step list directly as a YAML sequence. + { + name: "bare sequence with unnamed checkout", + customSteps: `- uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd + with: + fetch-depth: 0 + persist-credentials: false`, + expected: true, + }, + { + name: "bare sequence with named checkout", + customSteps: `- name: Checkout + uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd`, + expected: true, + }, + { + name: "bare sequence without checkout", + customSteps: `- name: Setup Node + uses: actions/setup-node@v4`, + expected: false, + }, + // Malformed YAML: findFirstCheckoutStepIndex returns (0, false) rather + // than attempting substring detection on unparseable input. Callers + // that need checkout detection must supply valid YAML. + { + name: "malformed YAML — explicit parse-error path", + customSteps: `{invalid: [yaml`, + expected: false, + }, } for _, tt := range tests { From 23515c84e70caf14328601f02921f93be1f85e67 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 23 Jul 2026 12:16:24 +0000 Subject: [PATCH 5/7] Polish: split wrapped-form check, replace em dash in comment Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- pkg/workflow/compiler_workflow_helpers.go | 16 +++++++++------- pkg/workflow/compiler_yaml_runtime_setup.go | 2 +- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/pkg/workflow/compiler_workflow_helpers.go b/pkg/workflow/compiler_workflow_helpers.go index 43ae32c31f1..5768fdae709 100644 --- a/pkg/workflow/compiler_workflow_helpers.go +++ b/pkg/workflow/compiler_workflow_helpers.go @@ -33,15 +33,17 @@ func findFirstCheckoutStepIndex(customSteps string) (int, bool) { var wrapper struct { Steps []map[string]any `yaml:"steps"` } - if err := yaml.Unmarshal([]byte(customSteps), &wrapper); err == nil && len(wrapper.Steps) > 0 { - for i, step := range wrapper.Steps { - uses, ok := step["uses"].(string) - if ok && isCheckoutActionReference(uses) { - compilerWorkflowHelpersLog.Print("Detected actions/checkout in custom steps") - return i, true + if err := yaml.Unmarshal([]byte(customSteps), &wrapper); err == nil { + if len(wrapper.Steps) > 0 { + for i, step := range wrapper.Steps { + uses, ok := step["uses"].(string) + if ok && isCheckoutActionReference(uses) { + compilerWorkflowHelpersLog.Print("Detected actions/checkout in custom steps") + return i, true + } } + return 0, false } - return 0, false } // Fall back to the bare sequence form: "- uses: ...\n" diff --git a/pkg/workflow/compiler_yaml_runtime_setup.go b/pkg/workflow/compiler_yaml_runtime_setup.go index 58bee0841ae..294f152cd84 100644 --- a/pkg/workflow/compiler_yaml_runtime_setup.go +++ b/pkg/workflow/compiler_yaml_runtime_setup.go @@ -381,7 +381,7 @@ func (c *Compiler) addCustomStepsWithRuntimeInsertion(yaml *strings.Builder, cus nextIndent := len(nextLine) - len(strings.TrimLeft(nextLine, " ")) // Stop if we hit the next step, but only when we are not inside a - // block scalar payload (e.g. "sparse-checkout: |\n - src" — the + // block scalar payload (e.g. "sparse-checkout: |\n - src" -- the // "- src" content line starts with "- " but is not a step boundary). if !blockScalarState.IsInPayload() && nextTrimmed != "" && strings.HasPrefix(nextTrimmed, "- ") && nextIndent == stepIndent { break From 0eeec43d200f9c78e6c7e391c5f861a598a1aef8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 23 Jul 2026 15:01:24 +0000 Subject: [PATCH 6/7] Fix tolowerequalfold lint: use strings.EqualFold in isCheckoutActionReference Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/compiler_workflow_helpers.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/workflow/compiler_workflow_helpers.go b/pkg/workflow/compiler_workflow_helpers.go index 5768fdae709..f288ba93f1c 100644 --- a/pkg/workflow/compiler_workflow_helpers.go +++ b/pkg/workflow/compiler_workflow_helpers.go @@ -65,8 +65,9 @@ func findFirstCheckoutStepIndex(customSteps string) (int, bool) { } func isCheckoutActionReference(uses string) bool { - normalized := strings.ToLower(strings.TrimSpace(strings.Trim(uses, `"'`))) - return normalized == "actions/checkout" || strings.HasPrefix(normalized, "actions/checkout@") + trimmed := strings.TrimSpace(strings.Trim(uses, `"'`)) + return strings.EqualFold(trimmed, "actions/checkout") || + strings.HasPrefix(strings.ToLower(trimmed), "actions/checkout@") } // GetWorkflowIDFromPath extracts the workflow ID from a markdown file path. From 346a1d58b689ee91e1b58d1aa68ca34c7d4fd326 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 23 Jul 2026 15:12:17 +0000 Subject: [PATCH 7/7] Sync daily-team-evolution-insights.lock.yml after merge with main Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- .github/workflows/daily-team-evolution-insights.lock.yml | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/.github/workflows/daily-team-evolution-insights.lock.yml b/.github/workflows/daily-team-evolution-insights.lock.yml index 0d7acafb5dd..aa6276bb2d6 100644 --- a/.github/workflows/daily-team-evolution-insights.lock.yml +++ b/.github/workflows/daily-team-evolution-insights.lock.yml @@ -1,4 +1,4 @@ -# gh-aw-metadata: {"schema_version":"v4","frontmatter_hash":"ee2d2116c63e56c4a929fd5ffe41684993e6682afe089cfec7933c58a8c77feb","body_hash":"904e4974a0a7b002ba02bb4452aaa223a765f91abcade2c062b40b1f135783e8","strict":true,"agent_id":"claude","engine_versions":{"claude":"2.1.216"}} +# gh-aw-metadata: {"schema_version":"v4","frontmatter_hash":"ee2d2116c63e56c4a929fd5ffe41684993e6682afe089cfec7933c58a8c77feb","body_hash":"904e4974a0a7b002ba02bb4452aaa223a765f91abcade2c062b40b1f135783e8","agent_id":"claude","engine_versions":{"claude":"2.1.216"}} # gh-aw-manifest: {"version":1,"secrets":["ANTHROPIC_API_KEY","COPILOT_GITHUB_TOKEN","GH_AW_GITHUB_MCP_SERVER_TOKEN","GH_AW_GITHUB_TOKEN","GH_AW_OTEL_GRAFANA_AUTHORIZATION","GH_AW_OTEL_GRAFANA_ENDPOINT","GH_AW_OTEL_SENTRY_AUTHORIZATION","GH_AW_OTEL_SENTRY_ENDPOINT","GITHUB_TOKEN"],"actions":[{"repo":"actions/cache/restore","sha":"55cc8345863c7cc4c66a329aec7e433d2d1c52a9","version":"v6.1.0"},{"repo":"actions/cache/save","sha":"55cc8345863c7cc4c66a329aec7e433d2d1c52a9","version":"v6.1.0"},{"repo":"actions/checkout","sha":"9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0","version":"v7.0.0"},{"repo":"actions/download-artifact","sha":"3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c","version":"v8.0.1"},{"repo":"actions/github-script","sha":"3a2844b7e9c422d3c10d287c895573f7108da1b3","version":"v9.0.0"},{"repo":"actions/setup-node","sha":"820762786026740c76f36085b0efc47a31fe5020","version":"v7.0.0"},{"repo":"actions/upload-artifact","sha":"043fb46d1a93c77aae656e7c1c64a875d1fc6a0a","version":"v7.0.1"}],"containers":[{"image":"ghcr.io/github/gh-aw-firewall/agent:0.27.38","digest":"sha256:cb928eb62d9139a013c2d278dab19af232d35a2d83dca71a3d98eb431f786243","pinned_image":"ghcr.io/github/gh-aw-firewall/agent:0.27.38@sha256:cb928eb62d9139a013c2d278dab19af232d35a2d83dca71a3d98eb431f786243"},{"image":"ghcr.io/github/gh-aw-firewall/api-proxy:0.27.38","digest":"sha256:cd6145620d96acee46e1ede25180a13aa36002467e663db0caa453a8bc8eb60c","pinned_image":"ghcr.io/github/gh-aw-firewall/api-proxy:0.27.38@sha256:cd6145620d96acee46e1ede25180a13aa36002467e663db0caa453a8bc8eb60c"},{"image":"ghcr.io/github/gh-aw-firewall/squid:0.27.38","digest":"sha256:6c19094d95aad5f9f128ad5e583f0f2b894b158aa66c3b86dd9bcc90970a2917","pinned_image":"ghcr.io/github/gh-aw-firewall/squid:0.27.38@sha256:6c19094d95aad5f9f128ad5e583f0f2b894b158aa66c3b86dd9bcc90970a2917"},{"image":"ghcr.io/github/gh-aw-mcpg:v0.4.3","digest":"sha256:3c744710ea275cd5ee65db92a1099e0d980754bd9fafda9ce67704c67004dc83","pinned_image":"ghcr.io/github/gh-aw-mcpg:v0.4.3@sha256:3c744710ea275cd5ee65db92a1099e0d980754bd9fafda9ce67704c67004dc83"},{"image":"ghcr.io/github/gh-aw-node","digest":"sha256:529d02eb970b1161aa25c593a9c3df57fdfad5a8add328cb3b6eccef66f3183b","pinned_image":"ghcr.io/github/gh-aw-node@sha256:529d02eb970b1161aa25c593a9c3df57fdfad5a8add328cb3b6eccef66f3183b"},{"image":"ghcr.io/github/github-mcp-server:v1.6.0","digest":"sha256:2b0c48b070f61e9d3969269ead600f62d00fb237b60ac849ef3d166ee7de9ad3","pinned_image":"ghcr.io/github/github-mcp-server:v1.6.0@sha256:2b0c48b070f61e9d3969269ead600f62d00fb237b60ac849ef3d166ee7de9ad3"}]} # This file was automatically generated by gh-aw. DO NOT EDIT. To debug this workflow, load the skill at https://github.com/github/gh-aw/blob/main/debug.md # @@ -154,7 +154,7 @@ jobs: GH_AW_INFO_AWMG_VERSION: "" GH_AW_INFO_FIREWALL_TYPE: "squid" GH_AW_INFO_FRONTMATTER_EMOJI: "📊" - GH_AW_COMPILED_STRICT: "true" + GH_AW_COMPILED_STRICT: "false" uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 with: script: | @@ -162,6 +162,11 @@ jobs: setupGlobals(core, github, context, exec, io, getOctokit); const { main } = require('${{ runner.temp }}/gh-aw/actions/generate_aw_info.cjs'); await main(core, context); + - name: Enforce strict mode policy + if: ${{ vars.GH_AW_POLICY_STRICT == 'true' }} + run: | + echo "::error::GH_AW_POLICY_STRICT=true but this workflow was not compiled in strict mode. Recompile with --strict or strict: true." + exit 1 - name: Restore daily AIC usage cache id: restore-daily-aic-cache if: ${{ env.GH_AW_MAX_DAILY_AI_CREDITS != '' }}