diff --git a/pkg/parser/import_field_extractor.go b/pkg/parser/import_field_extractor.go index b051e0a698..98b5d7fdaf 100644 --- a/pkg/parser/import_field_extractor.go +++ b/pkg/parser/import_field_extractor.go @@ -23,6 +23,7 @@ type importAccumulator struct { importPaths []string // Import paths for runtime-import macro generation stepsBuilder strings.Builder copilotSetupStepsBuilder strings.Builder // Steps from copilot-setup-steps.yml (inserted at start) + preStepsBuilder strings.Builder runtimesBuilder strings.Builder servicesBuilder strings.Builder networkBuilder strings.Builder @@ -72,7 +73,7 @@ func newImportAccumulator() *importAccumulator { // extractAllImportFields extracts all frontmatter fields from a single imported file // and accumulates the results. Handles tools, engines, mcp-servers, safe-outputs, // mcp-scripts, steps, runtimes, services, network, permissions, secret-masking, bots, -// skip-roles, skip-bots, post-steps, labels, cache, and features. +// skip-roles, skip-bots, pre-steps, post-steps, labels, cache, and features. func (acc *importAccumulator) extractAllImportFields(content []byte, item importQueueItem, visited map[string]bool) error { log.Printf("Extracting all import fields: path=%s, section=%s, inputs=%d, content_size=%d bytes", item.fullPath, item.sectionName, len(item.inputs), len(content)) @@ -310,6 +311,12 @@ func (acc *importAccumulator) extractAllImportFields(content []byte, item import } } + // Extract pre-steps from imported file (prepend in order) + preStepsContent, err := extractYAMLFieldFromMap(fm, "pre-steps") + if err == nil && preStepsContent != "" { + acc.preStepsBuilder.WriteString(preStepsContent + "\n") + } + // Extract post-steps from imported file (append in order) postStepsContent, err := extractYAMLFieldFromMap(fm, "post-steps") if err == nil && postStepsContent != "" { @@ -408,6 +415,7 @@ func (acc *importAccumulator) toImportsResult(topologicalOrder []string) *Import ImportPaths: acc.importPaths, MergedSteps: acc.stepsBuilder.String(), CopilotSetupSteps: acc.copilotSetupStepsBuilder.String(), + MergedPreSteps: acc.preStepsBuilder.String(), MergedRuntimes: acc.runtimesBuilder.String(), MergedRunInstallScripts: acc.runInstallScripts, MergedServices: acc.servicesBuilder.String(), diff --git a/pkg/parser/import_processor.go b/pkg/parser/import_processor.go index f1ab1ba9e2..31bb85a6f9 100644 --- a/pkg/parser/import_processor.go +++ b/pkg/parser/import_processor.go @@ -23,6 +23,7 @@ type ImportsResult struct { ImportPaths []string // List of import file paths for runtime-import macro generation (replaces MergedMarkdown) MergedSteps string // Merged steps configuration from all imports (excluding copilot-setup-steps) CopilotSetupSteps string // Steps from copilot-setup-steps.yml (inserted at start) + MergedPreSteps string // Merged pre-steps configuration from all imports (prepended in order) MergedRuntimes string // Merged runtimes configuration from all imports MergedRunInstallScripts bool // true if any imported workflow sets run-install-scripts: true (global or node-level) MergedServices string // Merged services configuration from all imports diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index 9b47e5a6ba..d15163ed5f 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -3317,6 +3317,41 @@ } ] }, + "pre-steps": { + "description": "Custom workflow steps to run at the very beginning of the agent job, before checkout and any other built-in steps. Use pre-steps to mint short-lived tokens or perform any setup that must happen before the repository is checked out. Step outputs are available via ${{ steps..outputs. }} and can be referenced in checkout.token to avoid masked-value cross-job-boundary issues.", + "oneOf": [ + { + "type": "object", + "additionalProperties": true + }, + { + "type": "array", + "items": { + "oneOf": [ + { + "type": "string" + }, + { + "type": "object", + "additionalProperties": true + } + ] + }, + "examples": [ + [ + { + "name": "Mint short-lived token", + "id": "mint", + "uses": "some-org/token-minting-action@v1", + "with": { + "scope": "target-org/target-repo" + } + } + ] + ] + } + ] + }, "post-steps": { "description": "Custom workflow steps to run after AI execution", "oneOf": [ diff --git a/pkg/workflow/compiler_orchestrator_workflow.go b/pkg/workflow/compiler_orchestrator_workflow.go index 6e43cc1f94..079677fc54 100644 --- a/pkg/workflow/compiler_orchestrator_workflow.go +++ b/pkg/workflow/compiler_orchestrator_workflow.go @@ -147,8 +147,11 @@ func (c *Compiler) ParseWorkflowFile(markdownPath string) (*WorkflowData, error) // Process and merge custom steps with imported steps c.processAndMergeSteps(result.Frontmatter, workflowData, engineSetup.importsResult) + // Process and merge pre-steps + c.processAndMergePreSteps(result.Frontmatter, workflowData, engineSetup.importsResult) + // Process and merge post-steps - c.processAndMergePostSteps(result.Frontmatter, workflowData) + c.processAndMergePostSteps(result.Frontmatter, workflowData, engineSetup.importsResult) // Process and merge services c.processAndMergeServices(result.Frontmatter, workflowData, engineSetup.importsResult) @@ -493,40 +496,121 @@ func (c *Compiler) processAndMergeSteps(frontmatter map[string]any, workflowData } } -// processAndMergePostSteps handles the processing of post-steps with action pinning -func (c *Compiler) processAndMergePostSteps(frontmatter map[string]any, workflowData *WorkflowData) { - orchestratorWorkflowLog.Print("Processing post-steps") - - workflowData.PostSteps = c.extractTopLevelYAMLSection(frontmatter, "post-steps") +// processAndMergePreSteps handles the processing and merging of pre-steps with action pinning. +// Pre-steps run at the very beginning of the agent job, before checkout and the subsequent +// built-in steps, allowing users to mint tokens or perform other setup that must happen +// before the repository is checked out. Imported pre-steps are merged before the main +// workflow's pre-steps so that the main workflow can override or extend the imports. +func (c *Compiler) processAndMergePreSteps(frontmatter map[string]any, workflowData *WorkflowData, importsResult *parser.ImportsResult) { + orchestratorWorkflowLog.Print("Processing and merging pre-steps") + + mainPreStepsYAML := c.extractTopLevelYAMLSection(frontmatter, "pre-steps") + + // Parse imported pre-steps if present (these go before the main workflow's pre-steps) + var importedPreSteps []any + if importsResult.MergedPreSteps != "" { + if err := yaml.Unmarshal([]byte(importsResult.MergedPreSteps), &importedPreSteps); err != nil { + orchestratorWorkflowLog.Printf("Failed to unmarshal imported pre-steps: %v", err) + } else { + typedImported, err := SliceToSteps(importedPreSteps) + if err != nil { + orchestratorWorkflowLog.Printf("Failed to convert imported pre-steps to typed steps: %v", err) + } else { + typedImported = ApplyActionPinsToTypedSteps(typedImported, workflowData) + importedPreSteps = StepsToSlice(typedImported) + } + } + } - // Apply action pinning to post-steps if any - if workflowData.PostSteps != "" { - var postStepsWrapper map[string]any - if err := yaml.Unmarshal([]byte(workflowData.PostSteps), &postStepsWrapper); err == nil { - if postStepsVal, hasPostSteps := postStepsWrapper["post-steps"]; hasPostSteps { - if postSteps, ok := postStepsVal.([]any); ok { - // Convert to typed steps for action pinning - typedPostSteps, err := SliceToSteps(postSteps) + // Parse main workflow pre-steps if present + var mainPreSteps []any + if mainPreStepsYAML != "" { + var mainWrapper map[string]any + if err := yaml.Unmarshal([]byte(mainPreStepsYAML), &mainWrapper); err == nil { + if mainVal, ok := mainWrapper["pre-steps"]; ok { + if steps, ok := mainVal.([]any); ok { + mainPreSteps = steps + typedMain, err := SliceToSteps(mainPreSteps) if err != nil { - orchestratorWorkflowLog.Printf("Failed to convert post-steps to typed steps: %v", err) + orchestratorWorkflowLog.Printf("Failed to convert main pre-steps to typed steps: %v", err) } else { - // Apply action pinning to post steps using type-safe version - typedPostSteps = ApplyActionPinsToTypedSteps(typedPostSteps, workflowData) - // Convert back to []any for YAML marshaling - postSteps = StepsToSlice(typedPostSteps) + typedMain = ApplyActionPinsToTypedSteps(typedMain, workflowData) + mainPreSteps = StepsToSlice(typedMain) } + } + } + } + } + + // Merge in order: imported pre-steps first, then main workflow's pre-steps + var allPreSteps []any + if len(importedPreSteps) > 0 || len(mainPreSteps) > 0 { + allPreSteps = append(allPreSteps, importedPreSteps...) + allPreSteps = append(allPreSteps, mainPreSteps...) + + stepsWrapper := map[string]any{"pre-steps": allPreSteps} + stepsYAML, err := yaml.Marshal(stepsWrapper) + if err == nil { + workflowData.PreSteps = unquoteUsesWithComments(string(stepsYAML)) + } + } +} + +// processAndMergePostSteps handles the processing and merging of post-steps with action pinning. +// Imported post-steps are appended after the main workflow's post-steps. +func (c *Compiler) processAndMergePostSteps(frontmatter map[string]any, workflowData *WorkflowData, importsResult *parser.ImportsResult) { + orchestratorWorkflowLog.Print("Processing and merging post-steps") - // Convert back to YAML with "post-steps:" wrapper - stepsWrapper := map[string]any{"post-steps": postSteps} - stepsYAML, err := yaml.Marshal(stepsWrapper) - if err == nil { - // Remove quotes from uses values with version comments - workflowData.PostSteps = unquoteUsesWithComments(string(stepsYAML)) + mainPostStepsYAML := c.extractTopLevelYAMLSection(frontmatter, "post-steps") + + // Parse imported post-steps if present (these go after the main workflow's post-steps) + var importedPostSteps []any + if importsResult.MergedPostSteps != "" { + if err := yaml.Unmarshal([]byte(importsResult.MergedPostSteps), &importedPostSteps); err != nil { + orchestratorWorkflowLog.Printf("Failed to unmarshal imported post-steps: %v", err) + } else { + typedImported, err := SliceToSteps(importedPostSteps) + if err != nil { + orchestratorWorkflowLog.Printf("Failed to convert imported post-steps to typed steps: %v", err) + } else { + typedImported = ApplyActionPinsToTypedSteps(typedImported, workflowData) + importedPostSteps = StepsToSlice(typedImported) + } + } + } + + // Parse main workflow post-steps if present + var mainPostSteps []any + if mainPostStepsYAML != "" { + var mainWrapper map[string]any + if err := yaml.Unmarshal([]byte(mainPostStepsYAML), &mainWrapper); err == nil { + if mainVal, ok := mainWrapper["post-steps"]; ok { + if steps, ok := mainVal.([]any); ok { + mainPostSteps = steps + typedMain, err := SliceToSteps(mainPostSteps) + if err != nil { + orchestratorWorkflowLog.Printf("Failed to convert main post-steps to typed steps: %v", err) + } else { + typedMain = ApplyActionPinsToTypedSteps(typedMain, workflowData) + mainPostSteps = StepsToSlice(typedMain) } } } } } + + // Merge in order: main workflow's post-steps first, then imported post-steps + var allPostSteps []any + if len(mainPostSteps) > 0 || len(importedPostSteps) > 0 { + allPostSteps = append(allPostSteps, mainPostSteps...) + allPostSteps = append(allPostSteps, importedPostSteps...) + + stepsWrapper := map[string]any{"post-steps": allPostSteps} + stepsYAML, err := yaml.Marshal(stepsWrapper) + if err == nil { + workflowData.PostSteps = unquoteUsesWithComments(string(stepsYAML)) + } + } } // processAndMergeServices handles the merging of imported services with main workflow services diff --git a/pkg/workflow/compiler_orchestrator_workflow_test.go b/pkg/workflow/compiler_orchestrator_workflow_test.go index 0887dbb6fe..1a75bb592f 100644 --- a/pkg/workflow/compiler_orchestrator_workflow_test.go +++ b/pkg/workflow/compiler_orchestrator_workflow_test.go @@ -401,8 +401,9 @@ func TestProcessAndMergePostSteps_NoPostSteps(t *testing.T) { compiler := NewCompiler() workflowData := &WorkflowData{} frontmatter := map[string]any{} + importsResult := &parser.ImportsResult{} - compiler.processAndMergePostSteps(frontmatter, workflowData) + compiler.processAndMergePostSteps(frontmatter, workflowData, importsResult) assert.Empty(t, workflowData.PostSteps) } @@ -430,14 +431,105 @@ func TestProcessAndMergePostSteps_WithPostSteps(t *testing.T) { }, }, } + importsResult := &parser.ImportsResult{} - compiler.processAndMergePostSteps(frontmatter, workflowData) + compiler.processAndMergePostSteps(frontmatter, workflowData, importsResult) assert.NotEmpty(t, workflowData.PostSteps) assert.Contains(t, workflowData.PostSteps, "Cleanup") assert.Contains(t, workflowData.PostSteps, "Upload logs") } +// TestProcessAndMergePostSteps_WithImportedPostSteps tests that imported post-steps are appended +func TestProcessAndMergePostSteps_WithImportedPostSteps(t *testing.T) { + compiler := NewCompiler() + workflowData := &WorkflowData{} + + frontmatter := map[string]any{ + "post-steps": []any{ + map[string]any{"name": "Main post step", "run": "echo 'main'"}, + }, + } + + importedPostStepsYAML, err := yaml.Marshal([]any{ + map[string]any{"name": "Imported post step", "run": "echo 'imported'"}, + }) + require.NoError(t, err, "yaml.Marshal should not fail for well-formed post-steps") + importsResult := &parser.ImportsResult{ + MergedPostSteps: string(importedPostStepsYAML), + } + + compiler.processAndMergePostSteps(frontmatter, workflowData, importsResult) + + assert.Contains(t, workflowData.PostSteps, "Main post step") + assert.Contains(t, workflowData.PostSteps, "Imported post step") + + // Main workflow's post-steps should come before imported ones + mainIdx := strings.Index(workflowData.PostSteps, "Main post step") + importedIdx := strings.Index(workflowData.PostSteps, "Imported post step") + assert.Less(t, mainIdx, importedIdx, "Main post-steps should come before imported ones") +} + +// TestProcessAndMergePreSteps_NoPreSteps tests processAndMergePreSteps with no pre-steps +func TestProcessAndMergePreSteps_NoPreSteps(t *testing.T) { + compiler := NewCompiler() + workflowData := &WorkflowData{} + frontmatter := map[string]any{} + importsResult := &parser.ImportsResult{} + + compiler.processAndMergePreSteps(frontmatter, workflowData, importsResult) + + assert.Empty(t, workflowData.PreSteps) +} + +// TestProcessAndMergePreSteps_WithPreSteps tests processAndMergePreSteps with pre-steps defined +func TestProcessAndMergePreSteps_WithPreSteps(t *testing.T) { + compiler := NewCompiler() + workflowData := &WorkflowData{} + + frontmatter := map[string]any{ + "pre-steps": []any{ + map[string]any{"name": "Mint token", "run": "echo 'minting'"}, + }, + } + importsResult := &parser.ImportsResult{} + + compiler.processAndMergePreSteps(frontmatter, workflowData, importsResult) + + assert.NotEmpty(t, workflowData.PreSteps) + assert.Contains(t, workflowData.PreSteps, "Mint token") +} + +// TestProcessAndMergePreSteps_WithImportedPreSteps tests that imported pre-steps are prepended +func TestProcessAndMergePreSteps_WithImportedPreSteps(t *testing.T) { + compiler := NewCompiler() + workflowData := &WorkflowData{} + + frontmatter := map[string]any{ + "pre-steps": []any{ + map[string]any{"name": "Main pre step", "run": "echo 'main'"}, + }, + } + + importedPreStepsYAML, err := yaml.Marshal([]any{ + map[string]any{"name": "Imported pre step", "run": "echo 'imported'"}, + }) + require.NoError(t, err, "yaml.Marshal should not fail for well-formed pre-steps") + importsResult := &parser.ImportsResult{ + MergedPreSteps: string(importedPreStepsYAML), + } + + compiler.processAndMergePreSteps(frontmatter, workflowData, importsResult) + + assert.Contains(t, workflowData.PreSteps, "Main pre step") + assert.Contains(t, workflowData.PreSteps, "Imported pre step") + + // Imported pre-steps should come before the main workflow's pre-steps + importedIdx := strings.Index(workflowData.PreSteps, "Imported pre step") + mainIdx := strings.Index(workflowData.PreSteps, "Main pre step") + assert.Less(t, importedIdx, mainIdx, "Imported pre-steps should come before main pre-steps") +} + // TestProcessAndMergeServices_NoServices tests processAndMergeServices with no services func TestProcessAndMergeServices_NoServices(t *testing.T) { compiler := NewCompiler() diff --git a/pkg/workflow/compiler_presteps_test.go b/pkg/workflow/compiler_presteps_test.go new file mode 100644 index 0000000000..53773b8cc9 --- /dev/null +++ b/pkg/workflow/compiler_presteps_test.go @@ -0,0 +1,288 @@ +//go:build !integration + +package workflow + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/github/gh-aw/pkg/testutil" +) + +// TestPreStepsGeneration verifies that pre-steps are emitted before checkout and all +// other built-in steps in the agent job. +func TestPreStepsGeneration(t *testing.T) { + tmpDir := testutil.TempDir(t, "pre-steps-test") + + testContent := `--- +on: push +permissions: + contents: read + issues: read + pull-requests: read +tools: + github: + allowed: [list_issues] +pre-steps: + - name: Mint short-lived token + id: mint + uses: some-org/token-minting-action@a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2 + with: + scope: target-org/target-repo +steps: + - name: Custom Setup Step + run: echo "Custom setup" +post-steps: + - name: Post AI Step + run: echo "This runs after AI" +engine: claude +strict: false +--- + +# Test Pre-Steps Workflow + +This workflow tests the pre-steps functionality. +` + + testFile := filepath.Join(tmpDir, "test-pre-steps.md") + if err := os.WriteFile(testFile, []byte(testContent), 0644); err != nil { + t.Fatal(err) + } + + compiler := NewCompiler() + + if err := compiler.CompileWorkflow(testFile); err != nil { + t.Fatalf("Unexpected error compiling workflow with pre-steps: %v", err) + } + + lockFile := filepath.Join(tmpDir, "test-pre-steps.lock.yml") + content, err := os.ReadFile(lockFile) + if err != nil { + t.Fatalf("Failed to read generated lock file: %v", err) + } + + lockContent := string(content) + + // Verify all three step types appear (check name value, not "- name:" prefix + // since steps with an id field have id: first in the YAML output) + if !strings.Contains(lockContent, "name: Mint short-lived token") { + t.Error("Expected pre-step 'Mint short-lived token' to be in generated workflow") + } + if !strings.Contains(lockContent, "name: Custom Setup Step") { + t.Error("Expected custom step 'Custom Setup Step' to be in generated workflow") + } + if !strings.Contains(lockContent, "name: Post AI Step") { + t.Error("Expected post-step 'Post AI Step' to be in generated workflow") + } + + // Pre-steps must appear before checkout, custom steps, and AI execution + preStepIndex := indexInNonCommentLines(lockContent, "name: Mint short-lived token") + checkoutIndex := indexInNonCommentLines(lockContent, "- name: Checkout repository") + customStepIndex := indexInNonCommentLines(lockContent, "- name: Custom Setup Step") + aiStepIndex := indexInNonCommentLines(lockContent, "- name: Execute Claude Code CLI") + postStepIndex := indexInNonCommentLines(lockContent, "- name: Post AI Step") + + if preStepIndex == -1 { + t.Fatal("Could not find pre-step in generated workflow") + } + if checkoutIndex == -1 { + t.Fatal("Could not find checkout step in generated workflow") + } + if customStepIndex == -1 { + t.Fatal("Could not find custom step in generated workflow") + } + if aiStepIndex == -1 { + t.Fatal("Could not find AI execution step in generated workflow") + } + if postStepIndex == -1 { + t.Fatal("Could not find post-step in generated workflow") + } + + if preStepIndex >= checkoutIndex { + t.Errorf("Pre-step (%d) should appear before checkout step (%d)", preStepIndex, checkoutIndex) + } + if preStepIndex >= customStepIndex { + t.Errorf("Pre-step (%d) should appear before custom step (%d)", preStepIndex, customStepIndex) + } + if preStepIndex >= aiStepIndex { + t.Errorf("Pre-step (%d) should appear before AI execution step (%d)", preStepIndex, aiStepIndex) + } + if postStepIndex <= aiStepIndex { + t.Errorf("Post-step (%d) should appear after AI execution step (%d)", postStepIndex, aiStepIndex) + } + + t.Logf("Step order verified: pre-step(%d) < checkout(%d) < custom(%d) < AI(%d) < post(%d)", + preStepIndex, checkoutIndex, customStepIndex, aiStepIndex, postStepIndex) +} + +// TestPreStepsTokenAvailableForCheckout verifies that a token minted in a pre-step +// can be referenced in checkout.token via a steps expression, avoiding the cross-job +// masked-value issue. +func TestPreStepsTokenAvailableForCheckout(t *testing.T) { + tmpDir := testutil.TempDir(t, "pre-steps-token-test") + + testContent := `--- +on: workflow_dispatch +permissions: + contents: read + id-token: write +pre-steps: + - name: Mint token + id: mint + uses: some-org/token-action@b1c2d3e4f5a6b1c2d3e4f5a6b1c2d3e4f5a6b1c2 + with: + scope: target-org/target-repo +checkout: + - repository: target-org/target-repo + path: target + token: ${{ steps.mint.outputs.token }} + current: false + - path: . +engine: claude +strict: false +--- + +Read a file from the checked-out repo. +` + + testFile := filepath.Join(tmpDir, "test-pre-steps-token.md") + if err := os.WriteFile(testFile, []byte(testContent), 0644); err != nil { + t.Fatal(err) + } + + compiler := NewCompiler() + + if err := compiler.CompileWorkflow(testFile); err != nil { + t.Fatalf("Unexpected error compiling workflow: %v", err) + } + + lockFile := filepath.Join(tmpDir, "test-pre-steps-token.lock.yml") + content, err := os.ReadFile(lockFile) + if err != nil { + t.Fatalf("Failed to read generated lock file: %v", err) + } + + lockContent := string(content) + + // The minting step must appear in the agent job + agentJobSection := extractJobSection(lockContent, "agent") + if agentJobSection == "" { + t.Fatal("Agent job section not found in generated workflow") + } + + if !strings.Contains(agentJobSection, "name: Mint token") { + t.Error("Expected pre-step 'Mint token' to be in the agent job") + } + + // The token reference must appear in the checkout step + if !strings.Contains(agentJobSection, "steps.mint.outputs.token") { + t.Error("Expected steps.mint.outputs.token reference in agent job checkout step") + } + + // The pre-step must appear before the checkout step + mintIndex := indexInNonCommentLines(agentJobSection, "name: Mint token") + checkoutIndex := indexInNonCommentLines(agentJobSection, "- name: Checkout target-org/target-repo into target") + if mintIndex == -1 { + t.Fatal("Could not find mint step in agent job") + } + if checkoutIndex == -1 { + t.Fatal("Could not find cross-repo checkout step in agent job") + } + if mintIndex >= checkoutIndex { + t.Errorf("Pre-step mint (%d) should appear before cross-repo checkout (%d)", mintIndex, checkoutIndex) + } +} + +// TestPreStepsOnly verifies that a workflow with only pre-steps (no custom steps or post-steps) +// compiles correctly. +func TestPreStepsOnly(t *testing.T) { + tmpDir := testutil.TempDir(t, "pre-steps-only-test") + + testContent := `--- +on: issues +permissions: + contents: read + issues: read +pre-steps: + - name: Only Pre Step + run: echo "This runs before checkout" +engine: claude +strict: false +--- + +# Test Pre-Steps Only Workflow + +This workflow tests pre-steps without custom steps or post-steps. +` + + testFile := filepath.Join(tmpDir, "test-pre-steps-only.md") + if err := os.WriteFile(testFile, []byte(testContent), 0644); err != nil { + t.Fatal(err) + } + + compiler := NewCompiler() + + if err := compiler.CompileWorkflow(testFile); err != nil { + t.Fatalf("Unexpected error compiling workflow with pre-steps only: %v", err) + } + + lockFile := filepath.Join(tmpDir, "test-pre-steps-only.lock.yml") + content, err := os.ReadFile(lockFile) + if err != nil { + t.Fatalf("Failed to read generated lock file: %v", err) + } + + lockContent := string(content) + + if !strings.Contains(lockContent, "- name: Only Pre Step") { + t.Error("Expected pre-step 'Only Pre Step' to be in generated workflow") + } + + // Default checkout must still be present and after the pre-step + preStepIndex := indexInNonCommentLines(lockContent, "- name: Only Pre Step") + checkoutIndex := indexInNonCommentLines(lockContent, "- name: Checkout repository") + aiStepIndex := indexInNonCommentLines(lockContent, "- name: Execute Claude Code CLI") + + if preStepIndex == -1 { + t.Fatal("Could not find pre-step in generated workflow") + } + if checkoutIndex == -1 { + t.Error("Expected default checkout step to still be present") + } + if aiStepIndex == -1 { + t.Fatal("Could not find AI execution step in generated workflow") + } + + if checkoutIndex != -1 && preStepIndex >= checkoutIndex { + t.Errorf("Pre-step (%d) should appear before checkout step (%d)", preStepIndex, checkoutIndex) + } + if preStepIndex >= aiStepIndex { + t.Errorf("Pre-step (%d) should appear before AI execution step (%d)", preStepIndex, aiStepIndex) + } +} + +// TestPreStepsSecretsValidation verifies that secrets in pre-steps trigger the same +// strict-mode error and non-strict warning as secrets in steps and post-steps. +func TestPreStepsSecretsValidation(t *testing.T) { + compiler := NewCompiler() + compiler.strictMode = true + + frontmatter := map[string]any{ + "pre-steps": []any{ + map[string]any{ + "name": "Use secret in pre-step", + "run": "echo ${{ secrets.MY_SECRET }}", + }, + }, + } + + err := compiler.validateStepsSecrets(frontmatter) + if err == nil { + t.Error("Expected strict-mode error for secrets in pre-steps but got nil") + } + if !strings.Contains(err.Error(), "pre-steps") { + t.Errorf("Expected error to mention 'pre-steps', got: %v", err) + } +} diff --git a/pkg/workflow/compiler_types.go b/pkg/workflow/compiler_types.go index dd9c1ad6e4..0245de556d 100644 --- a/pkg/workflow/compiler_types.go +++ b/pkg/workflow/compiler_types.go @@ -424,6 +424,7 @@ type WorkflowData struct { If string TimeoutMinutes string CustomSteps string + PreSteps string // steps to run at the very start of the agent job, before checkout PostSteps string // steps to run after AI execution RunsOn string RunsOnSlim string // runner override for all framework/generated jobs (activation, safe-outputs, unlock, etc.) diff --git a/pkg/workflow/compiler_yaml.go b/pkg/workflow/compiler_yaml.go index c4d56475ba..1337e0300d 100644 --- a/pkg/workflow/compiler_yaml.go +++ b/pkg/workflow/compiler_yaml.go @@ -603,6 +603,31 @@ func writePromptBashStep(yaml *strings.Builder, name, script string) { fmt.Fprintf(yaml, " run: bash ${RUNNER_TEMP}/gh-aw/actions/%s\n", script) } +func (c *Compiler) generatePreSteps(yaml *strings.Builder, data *WorkflowData) { + if data.PreSteps != "" { + // Remove "pre-steps:" line and adjust indentation, similar to PostSteps processing + lines := strings.Split(data.PreSteps, "\n") + if len(lines) > 1 { + for _, line := range lines[1:] { + // Trim trailing whitespace + trimmed := strings.TrimRight(line, " ") + // Skip empty lines + if strings.TrimSpace(trimmed) == "" { + yaml.WriteString("\n") + continue + } + // Steps need 6-space indentation ( - name:) + // Nested properties need 8-space indentation ( run:) + if strings.HasPrefix(line, " ") { + yaml.WriteString(" " + line[2:] + "\n") + } else { + yaml.WriteString(" " + line + "\n") + } + } + } + } +} + func (c *Compiler) generatePostSteps(yaml *strings.Builder, data *WorkflowData) { if data.PostSteps != "" { // Remove "post-steps:" line and adjust indentation, similar to CustomSteps processing diff --git a/pkg/workflow/compiler_yaml_main_job.go b/pkg/workflow/compiler_yaml_main_job.go index 392040f3b6..6d99417089 100644 --- a/pkg/workflow/compiler_yaml_main_job.go +++ b/pkg/workflow/compiler_yaml_main_job.go @@ -20,6 +20,15 @@ func (c *Compiler) generateMainJobSteps(yaml *strings.Builder, data *WorkflowDat yaml.WriteString(generateOTLPHeadersMaskStep()) } + // Add pre-steps before checkout and the subsequent built-in steps in this agent job. + // This allows users to mint short-lived tokens (via custom actions) in the same + // job as checkout, so the tokens are never dropped by the GitHub Actions runner's + // add-mask behaviour that silently suppresses masked values across job boundaries. + // Step outputs are available as ${{ steps..outputs. }} and can be + // referenced directly in checkout.token. Some compiler-injected setup steps may + // still be emitted earlier than these pre-steps. + c.generatePreSteps(yaml, data) + // Determine if we need to add a checkout step needsCheckout := c.shouldAddCheckoutStep(data) compilerYamlLog.Printf("Checkout step needed: %t", needsCheckout) diff --git a/pkg/workflow/frontmatter_types.go b/pkg/workflow/frontmatter_types.go index e0d8a38f0f..0bd65c9f2a 100644 --- a/pkg/workflow/frontmatter_types.go +++ b/pkg/workflow/frontmatter_types.go @@ -187,6 +187,7 @@ type FrontmatterConfig struct { RunsOn string `json:"runs-on,omitempty"` RunsOnSlim string `json:"runs-on-slim,omitempty"` // Runner for all framework/generated jobs (activation, safe-outputs, unlock, etc.) RunName string `json:"run-name,omitempty"` + PreSteps []any `json:"pre-steps,omitempty"` // Pre-workflow steps (run before checkout) Steps []any `json:"steps,omitempty"` // Custom workflow steps PostSteps []any `json:"post-steps,omitempty"` // Post-workflow steps Environment map[string]any `json:"environment,omitempty"` // GitHub environment @@ -666,6 +667,9 @@ func (fc *FrontmatterConfig) ToMap() map[string]any { if fc.RunName != "" { result["run-name"] = fc.RunName } + if fc.PreSteps != nil { + result["pre-steps"] = fc.PreSteps + } if fc.Steps != nil { result["steps"] = fc.Steps } diff --git a/pkg/workflow/strict_mode_steps_validation.go b/pkg/workflow/strict_mode_steps_validation.go index 815758e59c..d3fb14c7f6 100644 --- a/pkg/workflow/strict_mode_steps_validation.go +++ b/pkg/workflow/strict_mode_steps_validation.go @@ -19,13 +19,13 @@ import ( "github.com/github/gh-aw/pkg/sliceutil" ) -// validateStepsSecrets checks both the "steps" and "post-steps" frontmatter sections +// validateStepsSecrets checks the "pre-steps", "steps", and "post-steps" frontmatter sections // for secrets expressions (e.g. ${{ secrets.MY_SECRET }}). // // In strict mode the presence of any such expression is treated as an error. // In non-strict mode a warning is emitted instead. func (c *Compiler) validateStepsSecrets(frontmatter map[string]any) error { - for _, sectionName := range []string{"steps", "post-steps"} { + for _, sectionName := range []string{"pre-steps", "steps", "post-steps"} { if err := c.validateStepsSectionSecrets(frontmatter, sectionName); err != nil { return err }