From 34871a02b358d6ef41af590b936a84935e1d996d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Oct 2025 21:29:48 +0000 Subject: [PATCH 1/5] Initial plan From 5cacd8cbfa93f139ac6ab4f8ca2e1cebc3d927b6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Oct 2025 21:57:02 +0000 Subject: [PATCH 2/5] Implement strongly-typed steps with multiple placement positions - Add Step struct representing GitHub Actions workflow steps - Add WorkflowSteps struct with pre, pre-agent, post-agent, post positions - Add ParseStepsFromFrontmatter to parse both legacy array and new object formats - Update compiler to use new ParsedSteps with proper rendering at each position - Add post-agent field support (preferred over post-steps) - Maintain backward compatibility with legacy steps and post-steps fields - Update frontmatter schema to support new format - Add comprehensive tests for new format and backward compatibility Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .github/workflows/audit-workflows.lock.yml | 7 +- .../workflows/changeset-generator.lock.yml | 1 - .../workflows/cli-version-checker.lock.yml | 1 - .../workflows/copilot-agent-analysis.lock.yml | 7 +- .github/workflows/daily-news.lock.yml | 1 - .../duplicate-code-detector.lock.yml | 1 - .github/workflows/mcp-inspector.lock.yml | 7 +- .github/workflows/pdf-summary.lock.yml | 1 - .github/workflows/q.lock.yml | 7 +- .github/workflows/scout.lock.yml | 1 - .../workflows/technical-doc-writer.lock.yml | 6 +- .github/workflows/test-jqschema.lock.yml | 1 - .github/workflows/test-post-steps.lock.yml | 4 +- .github/workflows/unbloat-docs.lock.yml | 6 +- .github/workflows/video-analyzer.lock.yml | 1 - pkg/parser/schemas/main_workflow_schema.json | 84 ++++- pkg/workflow/compiler.go | 115 +++++- pkg/workflow/step.go | 219 +++++++++++ pkg/workflow/step_integration_test.go | 252 +++++++++++++ pkg/workflow/step_test.go | 346 ++++++++++++++++++ 20 files changed, 1016 insertions(+), 52 deletions(-) create mode 100644 pkg/workflow/step.go create mode 100644 pkg/workflow/step_integration_test.go create mode 100644 pkg/workflow/step_test.go diff --git a/.github/workflows/audit-workflows.lock.yml b/.github/workflows/audit-workflows.lock.yml index 2fefed644a6..32eacf842e5 100644 --- a/.github/workflows/audit-workflows.lock.yml +++ b/.github/workflows/audit-workflows.lock.yml @@ -87,13 +87,12 @@ jobs: run: make deps-dev - name: Install binary as 'gh-aw' run: make build - - env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - name: Start MCP server + - name: Start MCP server run: "set -e\n./gh-aw mcp-server --cmd ./gh-aw --port 8765 &\nMCP_PID=$!\n\n# Wait a moment for server to start\nsleep 2\n\n# Check if server is still running\nif ! kill -0 $MCP_PID 2>/dev/null; then\n echo \"MCP server failed to start\"\n exit 1\nfi\n\necho \"MCP server started successfully with PID $MCP_PID\"\n" + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - name: Set up jq utilities directory run: "mkdir -p /tmp/gh-aw\ncat > /tmp/gh-aw/jqschema.sh << 'EOF'\n#!/usr/bin/env bash\n# jqschema.sh\njq -c '\ndef walk(f):\n . as $in |\n if type == \"object\" then\n reduce keys[] as $k ({}; . + {($k): ($in[$k] | walk(f))})\n elif type == \"array\" then\n if length == 0 then [] else [.[0] | walk(f)] end\n else\n type\n end;\nwalk(.)\n'\nEOF\nchmod +x /tmp/gh-aw/jqschema.sh" - - name: Create gh-aw temp directory run: | mkdir -p /tmp/gh-aw/agent diff --git a/.github/workflows/changeset-generator.lock.yml b/.github/workflows/changeset-generator.lock.yml index b4849cd0f3e..a0823c413e6 100644 --- a/.github/workflows/changeset-generator.lock.yml +++ b/.github/workflows/changeset-generator.lock.yml @@ -586,7 +586,6 @@ jobs: mkdir -p .changeset git config user.name "github-actions[bot]" git config user.email "github-actions[bot]@users.noreply.github.com" - - name: Create gh-aw temp directory run: | mkdir -p /tmp/gh-aw/agent diff --git a/.github/workflows/cli-version-checker.lock.yml b/.github/workflows/cli-version-checker.lock.yml index 956e33fd141..a3f1421e652 100644 --- a/.github/workflows/cli-version-checker.lock.yml +++ b/.github/workflows/cli-version-checker.lock.yml @@ -79,7 +79,6 @@ jobs: uses: actions/checkout@v5 - name: Set up jq utilities directory run: "mkdir -p /tmp/gh-aw\ncat > /tmp/gh-aw/jqschema.sh << 'EOF'\n#!/usr/bin/env bash\n# jqschema.sh\njq -c '\ndef walk(f):\n . as $in |\n if type == \"object\" then\n reduce keys[] as $k ({}; . + {($k): ($in[$k] | walk(f))})\n elif type == \"array\" then\n if length == 0 then [] else [.[0] | walk(f)] end\n else\n type\n end;\nwalk(.)\n'\nEOF\nchmod +x /tmp/gh-aw/jqschema.sh" - - name: Create gh-aw temp directory run: | mkdir -p /tmp/gh-aw/agent diff --git a/.github/workflows/copilot-agent-analysis.lock.yml b/.github/workflows/copilot-agent-analysis.lock.yml index 897e27983e6..227b60b76fd 100644 --- a/.github/workflows/copilot-agent-analysis.lock.yml +++ b/.github/workflows/copilot-agent-analysis.lock.yml @@ -77,12 +77,11 @@ jobs: uses: actions/checkout@v5 - name: Set up jq utilities directory run: "mkdir -p /tmp/gh-aw\ncat > /tmp/gh-aw/jqschema.sh << 'EOF'\n#!/usr/bin/env bash\n# jqschema.sh\njq -c '\ndef walk(f):\n . as $in |\n if type == \"object\" then\n reduce keys[] as $k ({}; . + {($k): ($in[$k] | walk(f))})\n elif type == \"array\" then\n if length == 0 then [] else [.[0] | walk(f)] end\n else\n type\n end;\nwalk(.)\n'\nEOF\nchmod +x /tmp/gh-aw/jqschema.sh" - - env: + - name: Fetch Copilot PR data + run: "# Create output directory\nmkdir -p /tmp/gh-aw/pr-data\n\n# Calculate date 30 days ago\nDATE_30_DAYS_AGO=$(date -d '30 days ago' '+%Y-%m-%d' 2>/dev/null || date -v-30d '+%Y-%m-%d')\n\n# Search for PRs created by Copilot in the last 30 days using gh CLI\n# Using --author flag for server-side filtering (no jq needed!)\necho \"Fetching Copilot PRs from the last 30 days...\"\ngh search prs --repo ${{ github.repository }} \\\n --author \"@copilot\" \\\n --created \">=$DATE_30_DAYS_AGO\" \\\n --json number,title,state,createdAt,closedAt,author,body,labels,url,assignees,repository \\\n --limit 1000 \\\n > /tmp/gh-aw/pr-data/copilot-prs.json\n\n# Generate schema for reference\ncat /tmp/gh-aw/pr-data/copilot-prs.json | /tmp/gh-aw/jqschema.sh > /tmp/gh-aw/pr-data/copilot-prs-schema.json\n\necho \"PR data saved to /tmp/gh-aw/pr-data/copilot-prs.json\"\necho \"Schema saved to /tmp/gh-aw/pr-data/copilot-prs-schema.json\"\necho \"Total PRs found: $(jq 'length' /tmp/gh-aw/pr-data/copilot-prs.json)\"\n" + env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - name: Fetch Copilot PR data - run: "# Create output directory\nmkdir -p /tmp/gh-aw/pr-data\n\n# Calculate date 30 days ago\nDATE_30_DAYS_AGO=$(date -d '30 days ago' '+%Y-%m-%d' 2>/dev/null || date -v-30d '+%Y-%m-%d')\n\n# Search for PRs created by Copilot in the last 30 days using gh CLI\n# Using --author flag for server-side filtering (no jq needed!)\necho \"Fetching Copilot PRs from the last 30 days...\"\ngh search prs --repo ${{ github.repository }} \\\n --author \"@copilot\" \\\n --created \">=$DATE_30_DAYS_AGO\" \\\n --json number,title,state,createdAt,closedAt,author,body,labels,url,assignees,repository \\\n --limit 1000 \\\n > /tmp/gh-aw/pr-data/copilot-prs.json\n\n# Generate schema for reference\ncat /tmp/gh-aw/pr-data/copilot-prs.json | /tmp/gh-aw/jqschema.sh > /tmp/gh-aw/pr-data/copilot-prs-schema.json\n\necho \"PR data saved to /tmp/gh-aw/pr-data/copilot-prs.json\"\necho \"Schema saved to /tmp/gh-aw/pr-data/copilot-prs-schema.json\"\necho \"Total PRs found: $(jq 'length' /tmp/gh-aw/pr-data/copilot-prs.json)\"\n" - - name: Create gh-aw temp directory run: | mkdir -p /tmp/gh-aw/agent diff --git a/.github/workflows/daily-news.lock.yml b/.github/workflows/daily-news.lock.yml index dcfc6a05cb1..8d4e93a55e8 100644 --- a/.github/workflows/daily-news.lock.yml +++ b/.github/workflows/daily-news.lock.yml @@ -78,7 +78,6 @@ jobs: uses: actions/checkout@v5 - name: Set up jq utilities directory run: "mkdir -p /tmp/gh-aw\ncat > /tmp/gh-aw/jqschema.sh << 'EOF'\n#!/usr/bin/env bash\n# jqschema.sh\njq -c '\ndef walk(f):\n . as $in |\n if type == \"object\" then\n reduce keys[] as $k ({}; . + {($k): ($in[$k] | walk(f))})\n elif type == \"array\" then\n if length == 0 then [] else [.[0] | walk(f)] end\n else\n type\n end;\nwalk(.)\n'\nEOF\nchmod +x /tmp/gh-aw/jqschema.sh" - - name: Create gh-aw temp directory run: | mkdir -p /tmp/gh-aw/agent diff --git a/.github/workflows/duplicate-code-detector.lock.yml b/.github/workflows/duplicate-code-detector.lock.yml index d71a599ec82..9f37f69053b 100644 --- a/.github/workflows/duplicate-code-detector.lock.yml +++ b/.github/workflows/duplicate-code-detector.lock.yml @@ -94,7 +94,6 @@ jobs: run: go install golang.org/x/tools/gopls@latest - name: Check gopls version run: gopls version - - name: Create gh-aw temp directory run: | mkdir -p /tmp/gh-aw/agent diff --git a/.github/workflows/mcp-inspector.lock.yml b/.github/workflows/mcp-inspector.lock.yml index 11fceed4c79..d89b4753a6f 100644 --- a/.github/workflows/mcp-inspector.lock.yml +++ b/.github/workflows/mcp-inspector.lock.yml @@ -117,10 +117,10 @@ jobs: run: make deps-dev - name: Install binary as 'gh-aw' run: make build - - env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - name: Start MCP server + - name: Start MCP server run: "set -e\n./gh-aw mcp-server --cmd ./gh-aw --port 8765 &\nMCP_PID=$!\n\n# Wait a moment for server to start\nsleep 2\n\n# Check if server is still running\nif ! kill -0 $MCP_PID 2>/dev/null; then\n echo \"MCP server failed to start\"\n exit 1\nfi\n\necho \"MCP server started successfully with PID $MCP_PID\"\n" + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - name: Install Markitdown MCP run: pip install markitdown-mcp - name: Verify uv @@ -129,7 +129,6 @@ jobs: run: go install golang.org/x/tools/gopls@latest - name: Check gopls version run: gopls version - - name: Create gh-aw temp directory run: | mkdir -p /tmp/gh-aw/agent diff --git a/.github/workflows/pdf-summary.lock.yml b/.github/workflows/pdf-summary.lock.yml index c8158d9760d..67581ad51ee 100644 --- a/.github/workflows/pdf-summary.lock.yml +++ b/.github/workflows/pdf-summary.lock.yml @@ -973,7 +973,6 @@ jobs: python-version: '3.12' - name: Install Markitdown MCP run: pip install markitdown-mcp - - name: Create gh-aw temp directory run: | mkdir -p /tmp/gh-aw/agent diff --git a/.github/workflows/q.lock.yml b/.github/workflows/q.lock.yml index f3019aeb24e..c61840cf19c 100644 --- a/.github/workflows/q.lock.yml +++ b/.github/workflows/q.lock.yml @@ -1002,17 +1002,16 @@ jobs: run: make deps-dev - name: Install binary as 'gh-aw' run: make build - - env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - name: Start MCP server + - name: Start MCP server run: "set -e\n./gh-aw mcp-server --cmd ./gh-aw --port 8765 &\nMCP_PID=$!\n\n# Wait a moment for server to start\nsleep 2\n\n# Check if server is still running\nif ! kill -0 $MCP_PID 2>/dev/null; then\n echo \"MCP server failed to start\"\n exit 1\nfi\n\necho \"MCP server started successfully with PID $MCP_PID\"\n" + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - name: Verify uv run: uv --version - name: Install Go language service run: go install golang.org/x/tools/gopls@latest - name: Check gopls version run: gopls version - - name: Create gh-aw temp directory run: | mkdir -p /tmp/gh-aw/agent diff --git a/.github/workflows/scout.lock.yml b/.github/workflows/scout.lock.yml index a2c5f4108ec..aa54399a225 100644 --- a/.github/workflows/scout.lock.yml +++ b/.github/workflows/scout.lock.yml @@ -1000,7 +1000,6 @@ jobs: run: pip install markitdown-mcp - name: Set up jq utilities directory run: "mkdir -p /tmp/gh-aw\ncat > /tmp/gh-aw/jqschema.sh << 'EOF'\n#!/usr/bin/env bash\n# jqschema.sh\njq -c '\ndef walk(f):\n . as $in |\n if type == \"object\" then\n reduce keys[] as $k ({}; . + {($k): ($in[$k] | walk(f))})\n elif type == \"array\" then\n if length == 0 then [] else [.[0] | walk(f)] end\n else\n type\n end;\nwalk(.)\n'\nEOF\nchmod +x /tmp/gh-aw/jqschema.sh" - - name: Create gh-aw temp directory run: | mkdir -p /tmp/gh-aw/agent diff --git a/.github/workflows/technical-doc-writer.lock.yml b/.github/workflows/technical-doc-writer.lock.yml index d7b086f27b3..f8bfabdb34d 100644 --- a/.github/workflows/technical-doc-writer.lock.yml +++ b/.github/workflows/technical-doc-writer.lock.yml @@ -467,10 +467,10 @@ jobs: - name: Install dependencies run: npm ci working-directory: ./docs - - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - name: Build documentation + - name: Build documentation run: npm run build + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} working-directory: ./docs - name: Create gh-aw temp directory run: | diff --git a/.github/workflows/test-jqschema.lock.yml b/.github/workflows/test-jqschema.lock.yml index 851ac1eec33..9785471bdb2 100644 --- a/.github/workflows/test-jqschema.lock.yml +++ b/.github/workflows/test-jqschema.lock.yml @@ -62,7 +62,6 @@ jobs: uses: actions/checkout@v5 - name: Set up jq utilities directory run: "mkdir -p /tmp/gh-aw\ncat > /tmp/gh-aw/jqschema.sh << 'EOF'\n#!/usr/bin/env bash\n# jqschema.sh\njq -c '\ndef walk(f):\n . as $in |\n if type == \"object\" then\n reduce keys[] as $k ({}; . + {($k): ($in[$k] | walk(f))})\n elif type == \"array\" then\n if length == 0 then [] else [.[0] | walk(f)] end\n else\n type\n end;\nwalk(.)\n'\nEOF\nchmod +x /tmp/gh-aw/jqschema.sh" - - name: Create gh-aw temp directory run: | mkdir -p /tmp/gh-aw/agent diff --git a/.github/workflows/test-post-steps.lock.yml b/.github/workflows/test-post-steps.lock.yml index 0169c628eb5..02a85e3962c 100644 --- a/.github/workflows/test-post-steps.lock.yml +++ b/.github/workflows/test-post-steps.lock.yml @@ -1575,14 +1575,14 @@ jobs: run: | echo "✅ Post-steps are executing correctly" echo "This step runs after the AI agent completes" - - if: always() - name: Upload Test Results + - name: Upload Test Results uses: actions/upload-artifact@v4 with: if-no-files-found: ignore name: post-steps-test-results path: /tmp/gh-aw/ retention-days: 1 + if: always() - name: Final Summary run: "echo \"## Post-Steps Test Summary\" >> $GITHUB_STEP_SUMMARY\necho \"✅ All post-steps executed successfully\" >> $GITHUB_STEP_SUMMARY\necho \"This validates the post-steps indentation fix\" >> $GITHUB_STEP_SUMMARY\n" diff --git a/.github/workflows/unbloat-docs.lock.yml b/.github/workflows/unbloat-docs.lock.yml index 5e48a653b7c..eebe340d0a6 100644 --- a/.github/workflows/unbloat-docs.lock.yml +++ b/.github/workflows/unbloat-docs.lock.yml @@ -811,10 +811,10 @@ jobs: - name: Install dependencies run: npm ci working-directory: ./docs - - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - name: Build documentation + - name: Build documentation run: npm run build + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} working-directory: ./docs - name: Create gh-aw temp directory run: | diff --git a/.github/workflows/video-analyzer.lock.yml b/.github/workflows/video-analyzer.lock.yml index a76762a308a..278ba15692f 100644 --- a/.github/workflows/video-analyzer.lock.yml +++ b/.github/workflows/video-analyzer.lock.yml @@ -87,7 +87,6 @@ jobs: version=$(ffmpeg -version | head -n1) echo "version=$version" >> $GITHUB_OUTPUT mkdir -p /tmp/gh-aw/ffmpeg - - name: Create gh-aw temp directory run: | mkdir -p /tmp/gh-aw/agent diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index 5658b24af39..6ef911aa1e9 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -1465,30 +1465,102 @@ "description": "Conditional execution expression" }, "steps": { - "description": "Custom workflow steps", + "description": "Custom workflow steps. Can be an array (legacy format, goes to pre-agent position) or an object with named positions (pre, pre-agent, post-agent, post)", "oneOf": [ { "type": "object", - "additionalProperties": true + "description": "Object format with named step positions", + "properties": { + "pre": { + "description": "Steps to run before checkout and runtime setup", + "type": "array", + "items": { + "oneOf": [ + { + "type": "object", + "additionalProperties": true + }, + { + "type": "string" + } + ] + } + }, + "pre-agent": { + "description": "Steps to run after setup but before agent execution", + "type": "array", + "items": { + "oneOf": [ + { + "type": "object", + "additionalProperties": true + }, + { + "type": "string" + } + ] + } + }, + "post-agent": { + "description": "Steps to run immediately after agent execution", + "type": "array", + "items": { + "oneOf": [ + { + "type": "object", + "additionalProperties": true + }, + { + "type": "string" + } + ] + } + }, + "post": { + "description": "Steps to run after all other steps are complete", + "type": "array", + "items": { + "oneOf": [ + { + "type": "object", + "additionalProperties": true + }, + { + "type": "string" + } + ] + } + } + }, + "additionalProperties": false }, { "type": "array", + "description": "Legacy array format (steps go to pre-agent position)", "items": { "oneOf": [ - { - "type": "string" - }, { "type": "object", "additionalProperties": true + }, + { + "type": "string" } ] } } ] }, + "post-agent": { + "description": "Steps to run immediately after AI execution. Preferred over post-steps.", + "type": "array", + "items": { + "type": "object", + "additionalProperties": true + } + }, "post-steps": { - "description": "Custom workflow steps to run after AI execution", + "description": "DEPRECATED: Use 'post-agent' instead. Custom workflow steps to run after AI execution", "oneOf": [ { "type": "object", diff --git a/pkg/workflow/compiler.go b/pkg/workflow/compiler.go index e9a66dcc083..64763d8b40d 100644 --- a/pkg/workflow/compiler.go +++ b/pkg/workflow/compiler.go @@ -156,8 +156,9 @@ type WorkflowData struct { Env string If string TimeoutMinutes string - CustomSteps string - PostSteps string // steps to run after AI execution + CustomSteps string // DEPRECATED: Legacy string-based steps (use ParsedSteps instead) + PostSteps string // DEPRECATED: Legacy string-based post-steps (use ParsedSteps instead) + ParsedSteps *WorkflowSteps // Structured steps configuration parsed from frontmatter RunsOn string Environment string // environment setting for the main job Container string // container setting for the main job @@ -859,13 +860,76 @@ func (c *Compiler) ParseWorkflowFile(markdownPath string) (*WorkflowData, error) workflowData.Env = c.extractTopLevelYAMLSection(result.Frontmatter, "env") workflowData.If = c.extractIfCondition(result.Frontmatter) workflowData.TimeoutMinutes = c.extractTopLevelYAMLSection(result.Frontmatter, "timeout_minutes") - workflowData.CustomSteps = c.extractTopLevelYAMLSection(result.Frontmatter, "steps") + + // Parse steps using new structured approach + stepsData, _ := result.Frontmatter["steps"] + parsedSteps, err := ParseStepsFromFrontmatter(stepsData) + if err != nil { + return nil, fmt.Errorf("error parsing steps: %w", err) + } + + // Also parse post-agent field (new preferred field) + postAgentData, hasPostAgent := result.Frontmatter["post-agent"] + if hasPostAgent { + postAgentSteps, err := ParseStepsFromFrontmatter(postAgentData) + if err != nil { + return nil, fmt.Errorf("error parsing post-agent: %w", err) + } + if postAgentSteps != nil { + // Merge post-agent into the appropriate position + if parsedSteps == nil { + parsedSteps = &WorkflowSteps{} + } + // post-agent field maps directly to PostAgent position + // If it's an array, it will be in PreAgent, but we want it in PostAgent + parsedSteps.PostAgent = append(parsedSteps.PostAgent, postAgentSteps.PreAgent...) + parsedSteps.PostAgent = append(parsedSteps.PostAgent, postAgentSteps.PostAgent...) + } + } + + // Also parse post-steps from frontmatter (legacy support - goes to PostAgent position) + postStepsData, hasPostSteps := result.Frontmatter["post-steps"] + if hasPostSteps { + postSteps, err := ParseStepsFromFrontmatter(postStepsData) + if err != nil { + return nil, fmt.Errorf("error parsing post-steps: %w", err) + } + if postSteps != nil { + // Merge post-steps into the appropriate position + if parsedSteps == nil { + parsedSteps = &WorkflowSteps{} + } + // Legacy post-steps go to PostAgent position + parsedSteps.PostAgent = append(parsedSteps.PostAgent, postSteps.PreAgent...) + parsedSteps.PostAgent = append(parsedSteps.PostAgent, postSteps.PostAgent...) + } + } // Merge imported steps if any + var importedSteps *WorkflowSteps + if importsResult.MergedSteps != "" { + // Parse imported steps from YAML + var importedStepsData any + if err := yaml.Unmarshal([]byte(importsResult.MergedSteps), &importedStepsData); err == nil { + importedSteps, err = ParseStepsFromFrontmatter(importedStepsData) + if err != nil { + return nil, fmt.Errorf("error parsing imported steps: %w", err) + } + } + } + + // Merge imported and main workflow steps + workflowData.ParsedSteps = MergeSteps(parsedSteps, importedSteps) + + // Keep legacy string fields for backward compatibility during transition + workflowData.CustomSteps = c.extractTopLevelYAMLSection(result.Frontmatter, "steps") + workflowData.PostSteps = c.extractTopLevelYAMLSection(result.Frontmatter, "post-steps") + + // Apply the legacy merging logic to CustomSteps if needed for compatibility if importsResult.MergedSteps != "" { // Parse imported steps from YAML array - var importedSteps []any - if err := yaml.Unmarshal([]byte(importsResult.MergedSteps), &importedSteps); err == nil { + var importedStepsLegacy []any + if err := yaml.Unmarshal([]byte(importsResult.MergedSteps), &importedStepsLegacy); err == nil { // If there are main workflow steps, parse and merge them if workflowData.CustomSteps != "" { // Parse main workflow steps (format: "steps:\n - ...") @@ -874,7 +938,7 @@ func (c *Compiler) ParseWorkflowFile(markdownPath string) (*WorkflowData, error) if mainStepsVal, hasSteps := mainStepsWrapper["steps"]; hasSteps { if mainSteps, ok := mainStepsVal.([]any); ok { // Prepend imported steps to main steps - allSteps := append(importedSteps, mainSteps...) + allSteps := append(importedStepsLegacy, mainSteps...) // Convert back to YAML with "steps:" wrapper stepsWrapper := map[string]any{"steps": allSteps} stepsYAML, err := yaml.Marshal(stepsWrapper) @@ -886,7 +950,7 @@ func (c *Compiler) ParseWorkflowFile(markdownPath string) (*WorkflowData, error) } } else { // Only imported steps exist, wrap in "steps:" format - stepsWrapper := map[string]any{"steps": importedSteps} + stepsWrapper := map[string]any{"steps": importedStepsLegacy} stepsYAML, err := yaml.Marshal(stepsWrapper) if err == nil { workflowData.CustomSteps = string(stepsYAML) @@ -895,7 +959,6 @@ func (c *Compiler) ParseWorkflowFile(markdownPath string) (*WorkflowData, error) } } - workflowData.PostSteps = c.extractTopLevelYAMLSection(result.Frontmatter, "post-steps") workflowData.RunsOn = c.extractTopLevelYAMLSection(result.Frontmatter, "runs-on") workflowData.Environment = c.extractTopLevelYAMLSection(result.Frontmatter, "environment") workflowData.Container = c.extractTopLevelYAMLSection(result.Frontmatter, "container") @@ -2511,10 +2574,15 @@ func (c *Compiler) buildMainJob(data *WorkflowData, activationJobCreated bool) ( // generateMainJobSteps generates the steps section for the main job func (c *Compiler) generateMainJobSteps(yaml *strings.Builder, data *WorkflowData) { + // Add "pre" steps first (before checkout and runtime setup) + if data.ParsedSteps != nil && len(data.ParsedSteps.Pre) > 0 { + renderStepsAtPosition(yaml, data.ParsedSteps.Pre) + } + // Determine if we need to add a checkout step needsCheckout := c.shouldAddCheckoutStep(data) - // Add checkout step first if needed + // Add checkout step if needed if needsCheckout { yaml.WriteString(" - name: Checkout repository\n") yaml.WriteString(" uses: actions/checkout@v5\n") @@ -2545,8 +2613,11 @@ func (c *Compiler) generateMainJobSteps(yaml *strings.Builder, data *WorkflowDat } } - // Add custom steps if present - if data.CustomSteps != "" { + // Add "pre-agent" steps (these run before agent execution, where legacy "steps" go) + if data.ParsedSteps != nil && len(data.ParsedSteps.PreAgent) > 0 { + renderStepsAtPosition(yaml, data.ParsedSteps.PreAgent) + } else if data.CustomSteps != "" { + // Fallback to legacy CustomSteps for backward compatibility // Remove "steps:" line and adjust indentation lines := strings.Split(data.CustomSteps, "\n") if len(lines) > 1 { @@ -2664,8 +2735,11 @@ func (c *Compiler) generateMainJobSteps(yaml *strings.Builder, data *WorkflowDat c.generateGitPatchStep(yaml) } - // Add post-steps (if any) after AI execution + // Add post-agent steps (if any) after AI execution c.generatePostSteps(yaml, data) + + // Add final post steps (if any) at the very end + c.generateFinalSteps(yaml, data) } func (c *Compiler) generateUploadAgentLogs(yaml *strings.Builder, logFileFull string) { @@ -3044,9 +3118,13 @@ func (c *Compiler) generateSafeOutputsPromptStep(yaml *strings.Builder, safeOutp }) } -// generatePostSteps generates the post-steps section that runs after AI execution +// generatePostSteps generates the post-agent and post steps that run after AI execution func (c *Compiler) generatePostSteps(yaml *strings.Builder, data *WorkflowData) { - if data.PostSteps != "" { + // Add "post-agent" steps (these run immediately after agent execution) + if data.ParsedSteps != nil && len(data.ParsedSteps.PostAgent) > 0 { + renderStepsAtPosition(yaml, data.ParsedSteps.PostAgent) + } else if data.PostSteps != "" { + // Fallback to legacy PostSteps for backward compatibility // Remove "post-steps:" line and adjust indentation, similar to CustomSteps processing lines := strings.Split(data.PostSteps, "\n") if len(lines) > 1 { @@ -3070,6 +3148,15 @@ func (c *Compiler) generatePostSteps(yaml *strings.Builder, data *WorkflowData) } } +// generateFinalSteps generates the "post" steps that run after all other steps +func (c *Compiler) generateFinalSteps(yaml *strings.Builder, data *WorkflowData) { + // Add "post" steps (these run after everything else) + if data.ParsedSteps != nil && len(data.ParsedSteps.Post) > 0 { + renderStepsAtPosition(yaml, data.ParsedSteps.Post) + } +} + + // extractJobsFromFrontmatter extracts job configuration from frontmatter func (c *Compiler) extractJobsFromFrontmatter(frontmatter map[string]any) map[string]any { if jobs, exists := frontmatter["jobs"]; exists { diff --git a/pkg/workflow/step.go b/pkg/workflow/step.go new file mode 100644 index 00000000000..eb6f2be17ec --- /dev/null +++ b/pkg/workflow/step.go @@ -0,0 +1,219 @@ +package workflow + +import ( + "fmt" + "strings" + + "github.com/goccy/go-yaml" +) + +// Step represents a GitHub Actions workflow step +// It supports both the run and uses formats, along with all standard step properties +type Step struct { + // Step identification + ID string `yaml:"id,omitempty"` + Name string `yaml:"name,omitempty"` + + // Step execution - one of these should be set + Run string `yaml:"run,omitempty"` + Uses string `yaml:"uses,omitempty"` + With map[string]any `yaml:"with,omitempty"` + + // Step control + If string `yaml:"if,omitempty"` + Continue string `yaml:"continue-on-error,omitempty"` + + // Environment and working directory + Env map[string]string `yaml:"env,omitempty"` + WorkingDirectory string `yaml:"working-directory,omitempty"` + Shell string `yaml:"shell,omitempty"` + + // Timeout + TimeoutMinutes int `yaml:"timeout-minutes,omitempty"` +} + +// WorkflowSteps represents the different placement positions for custom steps in a workflow +// Steps can be placed in multiple positions relative to the agent execution: +// - Pre: Before checkout and runtime setup +// - PreAgent: After setup but before agent execution (where legacy "steps" field goes) +// - PostAgent: Immediately after agent execution (where legacy "post-steps" field goes) +// - Post: After all other steps are complete +type WorkflowSteps struct { + Pre []Step `yaml:"pre,omitempty"` + PreAgent []Step `yaml:"pre-agent,omitempty"` + PostAgent []Step `yaml:"post-agent,omitempty"` + Post []Step `yaml:"post,omitempty"` +} + +// ParseStepsFromFrontmatter parses the steps configuration from frontmatter +// It supports both legacy array format and new object format with named positions +func ParseStepsFromFrontmatter(data any) (*WorkflowSteps, error) { + if data == nil { + return nil, nil + } + + steps := &WorkflowSteps{} + + // Check if it's an array (legacy format or simple pre-agent steps) + if arrayData, ok := data.([]any); ok { + // This is the legacy array format - these steps go in PreAgent position + parsedSteps, err := parseStepArray(arrayData) + if err != nil { + return nil, err + } + steps.PreAgent = parsedSteps + return steps, nil + } + + // Check if it's an object with named positions + if objData, ok := data.(map[string]any); ok { + // Parse each named position + if preData, ok := objData["pre"]; ok { + if preArray, ok := preData.([]any); ok { + parsedSteps, err := parseStepArray(preArray) + if err != nil { + return nil, fmt.Errorf("error parsing pre steps: %w", err) + } + steps.Pre = parsedSteps + } + } + + if preAgentData, ok := objData["pre-agent"]; ok { + if preAgentArray, ok := preAgentData.([]any); ok { + parsedSteps, err := parseStepArray(preAgentArray) + if err != nil { + return nil, fmt.Errorf("error parsing pre-agent steps: %w", err) + } + steps.PreAgent = parsedSteps + } + } + + if postAgentData, ok := objData["post-agent"]; ok { + if postAgentArray, ok := postAgentData.([]any); ok { + parsedSteps, err := parseStepArray(postAgentArray) + if err != nil { + return nil, fmt.Errorf("error parsing post-agent steps: %w", err) + } + steps.PostAgent = parsedSteps + } + } + + if postData, ok := objData["post"]; ok { + if postArray, ok := postData.([]any); ok { + parsedSteps, err := parseStepArray(postArray) + if err != nil { + return nil, fmt.Errorf("error parsing post steps: %w", err) + } + steps.Post = parsedSteps + } + } + + return steps, nil + } + + return nil, fmt.Errorf("steps must be either an array or an object with named positions") +} + +// parseStepArray parses an array of step definitions into Step structs +func parseStepArray(data []any) ([]Step, error) { + steps := make([]Step, 0, len(data)) + + for i, item := range data { + // Convert the item to YAML and back to properly parse it into Step struct + yamlBytes, err := yaml.Marshal(item) + if err != nil { + return nil, fmt.Errorf("error marshaling step %d: %w", i, err) + } + + var step Step + if err := yaml.Unmarshal(yamlBytes, &step); err != nil { + return nil, fmt.Errorf("error parsing step %d: %w", i, err) + } + + steps = append(steps, step) + } + + return steps, nil +} + +// MergeSteps merges imported steps with main workflow steps +// Imported steps are prepended to the corresponding position +func MergeSteps(main, imported *WorkflowSteps) *WorkflowSteps { + if main == nil && imported == nil { + return nil + } + if main == nil { + return imported + } + if imported == nil { + return main + } + + merged := &WorkflowSteps{ + Pre: append(imported.Pre, main.Pre...), + PreAgent: append(imported.PreAgent, main.PreAgent...), + PostAgent: append(imported.PostAgent, main.PostAgent...), + Post: append(imported.Post, main.Post...), + } + + return merged +} + +// renderStepsAtPosition renders steps at a specific position with proper indentation +func renderStepsAtPosition(yaml *strings.Builder, steps []Step) { + if len(steps) == 0 { + return + } + + for _, step := range steps { + // Marshal step to YAML + stepYAML, err := marshalStep(step) + if err != nil { + continue // Skip steps that fail to marshal + } + + // Split into lines and add proper indentation + lines := strings.Split(stepYAML, "\n") + for i, line := range lines { + if strings.TrimSpace(line) == "" { + continue // Skip empty lines + } + if i == 0 { + // First line gets the list marker: " - name: ..." + yaml.WriteString(" - " + line + "\n") + } else { + // Subsequent lines get extra indentation: " run: ..." + yaml.WriteString(" " + line + "\n") + } + } + } +} + +// marshalStep marshals a step to YAML string +func marshalStep(step Step) (string, error) { + yamlBytes, err := yaml.Marshal(step) + if err != nil { + return "", err + } + return strings.TrimSpace(string(yamlBytes)), nil +} + +// IsEmpty returns true if there are no steps in any position + +func (ws *WorkflowSteps) IsEmpty() bool { + return ws == nil || (len(ws.Pre) == 0 && len(ws.PreAgent) == 0 && len(ws.PostAgent) == 0 && len(ws.Post) == 0) +} + +// ToYAML converts steps to YAML string for a specific position +func stepsToYAML(steps []Step) (string, error) { + if len(steps) == 0 { + return "", nil + } + + yamlBytes, err := yaml.Marshal(steps) + if err != nil { + return "", err + } + + return string(yamlBytes), nil +} diff --git a/pkg/workflow/step_integration_test.go b/pkg/workflow/step_integration_test.go new file mode 100644 index 00000000000..9ebe4194292 --- /dev/null +++ b/pkg/workflow/step_integration_test.go @@ -0,0 +1,252 @@ +package workflow + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestNewStepsFormat_ObjectWithPositions(t *testing.T) { + // Create temporary directory for test files + tmpDir, err := os.MkdirTemp("", "new-steps-test") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(tmpDir) + + // Test new object format with multiple positions + testContent := `--- +on: push +permissions: + contents: read +tools: + edit: +steps: + pre: + - name: Pre Step + run: echo "runs before checkout" + pre-agent: + - name: Pre Agent Step + run: echo "runs after setup, before agent" + post-agent: + - name: Post Agent Step + run: echo "runs after agent" + post: + - name: Final Step + run: echo "runs at the very end" +engine: claude +--- + +# Test New Steps Format + +This tests the new object-based steps format. +` + + testFile := filepath.Join(tmpDir, "test-new-steps.md") + if err := os.WriteFile(testFile, []byte(testContent), 0644); err != nil { + t.Fatal(err) + } + + compiler := NewCompiler(false, "", "test") + + // Compile the workflow + err = compiler.CompileWorkflow(testFile) + if err != nil { + t.Fatalf("Unexpected error compiling workflow with new steps format: %v", err) + } + + // Read the generated lock file + lockFile := filepath.Join(tmpDir, "test-new-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 steps are present + if !strings.Contains(lockContent, "- name: Pre Step") { + t.Error("Expected 'Pre Step' to be in generated workflow") + } + if !strings.Contains(lockContent, "- name: Pre Agent Step") { + t.Error("Expected 'Pre Agent Step' to be in generated workflow") + } + if !strings.Contains(lockContent, "- name: Post Agent Step") { + t.Error("Expected 'Post Agent Step' to be in generated workflow") + } + if !strings.Contains(lockContent, "- name: Final Step") { + t.Error("Expected 'Final Step' to be in generated workflow") + } + + // Verify step order + preStepPos := strings.Index(lockContent, "- name: Pre Step") + checkoutPos := strings.Index(lockContent, "- name: Checkout repository") + preAgentPos := strings.Index(lockContent, "- name: Pre Agent Step") + agentPos := strings.Index(lockContent, "- name: Execute Claude Code CLI") + postAgentPos := strings.Index(lockContent, "- name: Post Agent Step") + finalStepPos := strings.Index(lockContent, "- name: Final Step") + + if preStepPos == -1 || checkoutPos == -1 || preAgentPos == -1 || agentPos == -1 || postAgentPos == -1 || finalStepPos == -1 { + t.Fatal("Could not find all expected steps in generated workflow") + } + + // Verify correct order: Pre < Checkout < Pre-Agent < Agent < Post-Agent < Final + if preStepPos >= checkoutPos { + t.Errorf("Pre step should come before checkout: pre=%d, checkout=%d", preStepPos, checkoutPos) + } + if checkoutPos >= preAgentPos { + t.Errorf("Checkout should come before pre-agent: checkout=%d, pre-agent=%d", checkoutPos, preAgentPos) + } + if preAgentPos >= agentPos { + t.Errorf("Pre-agent step should come before agent execution: pre-agent=%d, agent=%d", preAgentPos, agentPos) + } + if agentPos >= postAgentPos { + t.Errorf("Agent execution should come before post-agent step: agent=%d, post-agent=%d", agentPos, postAgentPos) + } + if postAgentPos >= finalStepPos { + t.Errorf("Post-agent step should come before final step: post-agent=%d, final=%d", postAgentPos, finalStepPos) + } + + t.Log("Step order verified: Pre < Checkout < Pre-Agent < Agent < Post-Agent < Final") +} + +func TestNewStepsFormat_PostAgentField(t *testing.T) { + // Create temporary directory for test files + tmpDir, err := os.MkdirTemp("", "post-agent-field-test") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(tmpDir) + + // Test new post-agent field (instead of post-steps) + testContent := `--- +on: push +permissions: + contents: read +tools: + edit: +post-agent: + - name: Post Agent Step + run: echo "runs after agent" + - name: Another Post Agent Step + run: echo "also runs after agent" +engine: claude +--- + +# Test Post-Agent Field + +This tests the new post-agent field. +` + + testFile := filepath.Join(tmpDir, "test-post-agent.md") + if err := os.WriteFile(testFile, []byte(testContent), 0644); err != nil { + t.Fatal(err) + } + + compiler := NewCompiler(false, "", "test") + + // Compile the workflow + err = compiler.CompileWorkflow(testFile) + if err != nil { + t.Fatalf("Unexpected error compiling workflow with post-agent field: %v", err) + } + + // Read the generated lock file + lockFile := filepath.Join(tmpDir, "test-post-agent.lock.yml") + content, err := os.ReadFile(lockFile) + if err != nil { + t.Fatalf("Failed to read generated lock file: %v", err) + } + + lockContent := string(content) + + // Verify post-agent steps are present + if !strings.Contains(lockContent, "- name: Post Agent Step") { + t.Error("Expected 'Post Agent Step' to be in generated workflow") + } + if !strings.Contains(lockContent, "- name: Another Post Agent Step") { + t.Error("Expected 'Another Post Agent Step' to be in generated workflow") + } + + // Verify they come after agent execution + agentPos := strings.Index(lockContent, "- name: Execute Claude Code CLI") + postAgentPos := strings.Index(lockContent, "- name: Post Agent Step") + + if agentPos == -1 || postAgentPos == -1 { + t.Fatal("Could not find expected elements in generated workflow") + } + + if agentPos >= postAgentPos { + t.Errorf("Post-agent steps should come after agent execution: agent=%d, post-agent=%d", agentPos, postAgentPos) + } + + t.Log("Post-agent field verified successfully") +} + +func TestLegacyStepsFormat_BackwardCompatibility(t *testing.T) { + // Create temporary directory for test files + tmpDir, err := os.MkdirTemp("", "legacy-steps-test") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(tmpDir) + + // Test legacy array format still works + testContent := `--- +on: push +permissions: + contents: read +tools: + edit: +steps: + - name: Legacy Step 1 + run: echo "legacy step 1" + - name: Legacy Step 2 + run: echo "legacy step 2" +post-steps: + - name: Legacy Post Step + run: echo "legacy post step" +engine: claude +--- + +# Test Legacy Format + +This tests backward compatibility with the legacy array format. +` + + testFile := filepath.Join(tmpDir, "test-legacy.md") + if err := os.WriteFile(testFile, []byte(testContent), 0644); err != nil { + t.Fatal(err) + } + + compiler := NewCompiler(false, "", "test") + + // Compile the workflow + err = compiler.CompileWorkflow(testFile) + if err != nil { + t.Fatalf("Unexpected error compiling workflow with legacy format: %v", err) + } + + // Read the generated lock file + lockFile := filepath.Join(tmpDir, "test-legacy.lock.yml") + content, err := os.ReadFile(lockFile) + if err != nil { + t.Fatalf("Failed to read generated lock file: %v", err) + } + + lockContent := string(content) + + // Verify steps are present + if !strings.Contains(lockContent, "- name: Legacy Step 1") { + t.Error("Expected 'Legacy Step 1' to be in generated workflow") + } + if !strings.Contains(lockContent, "- name: Legacy Step 2") { + t.Error("Expected 'Legacy Step 2' to be in generated workflow") + } + if !strings.Contains(lockContent, "- name: Legacy Post Step") { + t.Error("Expected 'Legacy Post Step' to be in generated workflow") + } + + t.Log("Legacy format backward compatibility verified") +} diff --git a/pkg/workflow/step_test.go b/pkg/workflow/step_test.go new file mode 100644 index 00000000000..1e0b2d40d75 --- /dev/null +++ b/pkg/workflow/step_test.go @@ -0,0 +1,346 @@ +package workflow + +import ( + "testing" + + "github.com/goccy/go-yaml" +) + +func TestParseStepsFromFrontmatter_LegacyArrayFormat(t *testing.T) { + // Test legacy array format (should go into PreAgent position) + yamlContent := ` +- name: Test Step + run: echo "hello" +- name: Another Step + uses: actions/checkout@v4 +` + + var data []any + if err := yaml.Unmarshal([]byte(yamlContent), &data); err != nil { + t.Fatalf("Failed to unmarshal YAML: %v", err) + } + + steps, err := ParseStepsFromFrontmatter(data) + if err != nil { + t.Fatalf("Failed to parse steps: %v", err) + } + + if steps == nil { + t.Fatal("Expected steps to be non-nil") + } + + if len(steps.PreAgent) != 2 { + t.Errorf("Expected 2 pre-agent steps, got %d", len(steps.PreAgent)) + } + + if len(steps.Pre) != 0 || len(steps.PostAgent) != 0 || len(steps.Post) != 0 { + t.Error("Expected only pre-agent steps to be populated in legacy format") + } + + // Verify first step + if steps.PreAgent[0].Name != "Test Step" { + t.Errorf("Expected first step name 'Test Step', got '%s'", steps.PreAgent[0].Name) + } + if steps.PreAgent[0].Run != "echo \"hello\"" { + t.Errorf("Expected first step run 'echo \"hello\"', got '%s'", steps.PreAgent[0].Run) + } + + // Verify second step + if steps.PreAgent[1].Name != "Another Step" { + t.Errorf("Expected second step name 'Another Step', got '%s'", steps.PreAgent[1].Name) + } + if steps.PreAgent[1].Uses != "actions/checkout@v4" { + t.Errorf("Expected second step uses 'actions/checkout@v4', got '%s'", steps.PreAgent[1].Uses) + } +} + +func TestParseStepsFromFrontmatter_NewObjectFormat(t *testing.T) { + // Test new object format with named positions + yamlContent := ` +pre: + - name: Pre Step + run: echo "pre" +pre-agent: + - name: Pre Agent Step + run: echo "pre-agent" +post-agent: + - name: Post Agent Step + run: echo "post-agent" +post: + - name: Post Step + run: echo "post" +` + + var data map[string]any + if err := yaml.Unmarshal([]byte(yamlContent), &data); err != nil { + t.Fatalf("Failed to unmarshal YAML: %v", err) + } + + steps, err := ParseStepsFromFrontmatter(data) + if err != nil { + t.Fatalf("Failed to parse steps: %v", err) + } + + if steps == nil { + t.Fatal("Expected steps to be non-nil") + } + + // Verify all positions are populated + if len(steps.Pre) != 1 { + t.Errorf("Expected 1 pre step, got %d", len(steps.Pre)) + } + if len(steps.PreAgent) != 1 { + t.Errorf("Expected 1 pre-agent step, got %d", len(steps.PreAgent)) + } + if len(steps.PostAgent) != 1 { + t.Errorf("Expected 1 post-agent step, got %d", len(steps.PostAgent)) + } + if len(steps.Post) != 1 { + t.Errorf("Expected 1 post step, got %d", len(steps.Post)) + } + + // Verify step names + if steps.Pre[0].Name != "Pre Step" { + t.Errorf("Expected pre step name 'Pre Step', got '%s'", steps.Pre[0].Name) + } + if steps.PreAgent[0].Name != "Pre Agent Step" { + t.Errorf("Expected pre-agent step name 'Pre Agent Step', got '%s'", steps.PreAgent[0].Name) + } + if steps.PostAgent[0].Name != "Post Agent Step" { + t.Errorf("Expected post-agent step name 'Post Agent Step', got '%s'", steps.PostAgent[0].Name) + } + if steps.Post[0].Name != "Post Step" { + t.Errorf("Expected post step name 'Post Step', got '%s'", steps.Post[0].Name) + } +} + +func TestParseStepsFromFrontmatter_PartialObjectFormat(t *testing.T) { + // Test object format with only some positions defined + yamlContent := ` +pre-agent: + - name: Pre Agent Step + run: echo "pre-agent" +post-agent: + - name: Post Agent Step + run: echo "post-agent" +` + + var data map[string]any + if err := yaml.Unmarshal([]byte(yamlContent), &data); err != nil { + t.Fatalf("Failed to unmarshal YAML: %v", err) + } + + steps, err := ParseStepsFromFrontmatter(data) + if err != nil { + t.Fatalf("Failed to parse steps: %v", err) + } + + if steps == nil { + t.Fatal("Expected steps to be non-nil") + } + + // Verify only specified positions are populated + if len(steps.Pre) != 0 { + t.Errorf("Expected 0 pre steps, got %d", len(steps.Pre)) + } + if len(steps.PreAgent) != 1 { + t.Errorf("Expected 1 pre-agent step, got %d", len(steps.PreAgent)) + } + if len(steps.PostAgent) != 1 { + t.Errorf("Expected 1 post-agent step, got %d", len(steps.PostAgent)) + } + if len(steps.Post) != 0 { + t.Errorf("Expected 0 post steps, got %d", len(steps.Post)) + } +} + +func TestParseStepsFromFrontmatter_NilInput(t *testing.T) { + steps, err := ParseStepsFromFrontmatter(nil) + if err != nil { + t.Fatalf("Expected no error for nil input, got: %v", err) + } + if steps != nil { + t.Error("Expected nil steps for nil input") + } +} + +func TestMergeSteps(t *testing.T) { + main := &WorkflowSteps{ + Pre: []Step{ + {Name: "Main Pre", Run: "echo main-pre"}, + }, + PreAgent: []Step{ + {Name: "Main PreAgent", Run: "echo main-pre-agent"}, + }, + PostAgent: []Step{ + {Name: "Main PostAgent", Run: "echo main-post-agent"}, + }, + Post: []Step{ + {Name: "Main Post", Run: "echo main-post"}, + }, + } + + imported := &WorkflowSteps{ + Pre: []Step{ + {Name: "Imported Pre", Run: "echo imported-pre"}, + }, + PreAgent: []Step{ + {Name: "Imported PreAgent", Run: "echo imported-pre-agent"}, + }, + PostAgent: []Step{ + {Name: "Imported PostAgent", Run: "echo imported-post-agent"}, + }, + Post: []Step{ + {Name: "Imported Post", Run: "echo imported-post"}, + }, + } + + merged := MergeSteps(main, imported) + + // Verify imported steps come first + if len(merged.Pre) != 2 { + t.Errorf("Expected 2 pre steps, got %d", len(merged.Pre)) + } + if merged.Pre[0].Name != "Imported Pre" { + t.Errorf("Expected first pre step to be imported, got '%s'", merged.Pre[0].Name) + } + if merged.Pre[1].Name != "Main Pre" { + t.Errorf("Expected second pre step to be main, got '%s'", merged.Pre[1].Name) + } + + // Verify same for other positions + if len(merged.PreAgent) != 2 { + t.Errorf("Expected 2 pre-agent steps, got %d", len(merged.PreAgent)) + } + if len(merged.PostAgent) != 2 { + t.Errorf("Expected 2 post-agent steps, got %d", len(merged.PostAgent)) + } + if len(merged.Post) != 2 { + t.Errorf("Expected 2 post steps, got %d", len(merged.Post)) + } +} + +func TestMergeSteps_NilInputs(t *testing.T) { + // Test nil main + imported := &WorkflowSteps{ + PreAgent: []Step{{Name: "Test", Run: "echo test"}}, + } + merged := MergeSteps(nil, imported) + if merged != imported { + t.Error("Expected imported steps when main is nil") + } + + // Test nil imported + main := &WorkflowSteps{ + PreAgent: []Step{{Name: "Test", Run: "echo test"}}, + } + merged = MergeSteps(main, nil) + if merged != main { + t.Error("Expected main steps when imported is nil") + } + + // Test both nil + merged = MergeSteps(nil, nil) + if merged != nil { + t.Error("Expected nil when both inputs are nil") + } +} + +func TestWorkflowSteps_IsEmpty(t *testing.T) { + // Test nil + var steps *WorkflowSteps + if !steps.IsEmpty() { + t.Error("Expected nil steps to be empty") + } + + // Test empty struct + steps = &WorkflowSteps{} + if !steps.IsEmpty() { + t.Error("Expected empty struct to be empty") + } + + // Test with steps + steps = &WorkflowSteps{ + PreAgent: []Step{{Name: "Test", Run: "echo test"}}, + } + if steps.IsEmpty() { + t.Error("Expected non-empty steps") + } +} + +func TestStep_AllFields(t *testing.T) { + // Test parsing a step with all fields + yamlContent := ` +id: test-step +name: Test Step +run: echo "test" +if: success() +continue-on-error: true +env: + KEY1: value1 + KEY2: value2 +working-directory: /tmp +shell: bash +timeout-minutes: 5 +` + + var step Step + if err := yaml.Unmarshal([]byte(yamlContent), &step); err != nil { + t.Fatalf("Failed to unmarshal step: %v", err) + } + + if step.ID != "test-step" { + t.Errorf("Expected ID 'test-step', got '%s'", step.ID) + } + if step.Name != "Test Step" { + t.Errorf("Expected name 'Test Step', got '%s'", step.Name) + } + if step.Run != "echo \"test\"" { + t.Errorf("Expected run 'echo \"test\"', got '%s'", step.Run) + } + if step.If != "success()" { + t.Errorf("Expected if 'success()', got '%s'", step.If) + } + if step.Continue != "true" { + t.Errorf("Expected continue 'true', got '%s'", step.Continue) + } + if len(step.Env) != 2 { + t.Errorf("Expected 2 env vars, got %d", len(step.Env)) + } + if step.WorkingDirectory != "/tmp" { + t.Errorf("Expected working directory '/tmp', got '%s'", step.WorkingDirectory) + } + if step.Shell != "bash" { + t.Errorf("Expected shell 'bash', got '%s'", step.Shell) + } + if step.TimeoutMinutes != 5 { + t.Errorf("Expected timeout 5, got %d", step.TimeoutMinutes) + } +} + +func TestStep_UsesFormat(t *testing.T) { + // Test parsing a step with uses and with fields + yamlContent := ` +name: Checkout +uses: actions/checkout@v4 +with: + fetch-depth: 0 + token: ${{ secrets.GITHUB_TOKEN }} +` + + var step Step + if err := yaml.Unmarshal([]byte(yamlContent), &step); err != nil { + t.Fatalf("Failed to unmarshal step: %v", err) + } + + if step.Uses != "actions/checkout@v4" { + t.Errorf("Expected uses 'actions/checkout@v4', got '%s'", step.Uses) + } + if len(step.With) != 2 { + t.Errorf("Expected 2 with fields, got %d", len(step.With)) + } + // fetch-depth should be present and be a number (YAML might parse as int or float) + if _, ok := step.With["fetch-depth"]; !ok { + t.Error("Expected fetch-depth to be present in with fields") + } +} From 3398dbbe9bbd43aec0cc4aec08de6f1f9d24fc52 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Oct 2025 22:01:38 +0000 Subject: [PATCH 3/5] Add documentation for new steps format - Update .github/instructions with new steps format documentation - Document the four step placement positions (pre, pre-agent, post-agent, post) - Document step execution order - Document legacy format backward compatibility - Mark post-steps as deprecated in favor of post-agent - Add examples for all step formats - Document step merging behavior with imports Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../github-agentic-workflows.instructions.md | 110 +++++++++++++++++- pkg/workflow/compiler.go | 9 +- 2 files changed, 112 insertions(+), 7 deletions(-) diff --git a/.github/instructions/github-agentic-workflows.instructions.md b/.github/instructions/github-agentic-workflows.instructions.md index ce6e12f89ca..6c069109bc8 100644 --- a/.github/instructions/github-agentic-workflows.instructions.md +++ b/.github/instructions/github-agentic-workflows.instructions.md @@ -52,8 +52,9 @@ The YAML frontmatter supports these fields: - **`if:`** - Conditional execution expression (string) - **`run-name:`** - Custom workflow run name (string) - **`name:`** - Workflow name (string) -- **`steps:`** - Custom workflow steps (object) -- **`post-steps:`** - Custom workflow steps to run after AI execution (object) +- **`steps:`** - Custom workflow steps (array or object with named positions) +- **`post-agent:`** - Steps to run immediately after AI execution (array, preferred over post-steps) +- **`post-steps:`** - DEPRECATED: Use post-agent instead. Custom workflow steps to run after AI execution (object) ### Agentic Workflow Specific Fields @@ -311,6 +312,111 @@ Cache-memory configurations can be imported from shared agentic workflows using The memory MCP server is automatically configured when `cache-memory` is enabled and works with both Claude and Custom engines. +## Custom Workflow Steps + +GitHub Agentic Workflows support custom workflow steps that can be placed at different positions relative to the agent execution. This allows you to run setup, configuration, or cleanup tasks at precise points in the workflow lifecycle. + +### Step Placement Positions + +Steps can be placed in four different positions: + +1. **`pre`**: Runs before checkout and runtime setup +2. **`pre-agent`**: Runs after setup but before agent execution (legacy `steps` field maps here) +3. **`post-agent`**: Runs immediately after agent execution (legacy `post-steps` field maps here) +4. **`post`**: Runs after all other steps are complete + +### Step Formats + +**Legacy Array Format** (maps to pre-agent position): +```yaml +steps: + - name: Setup Step + run: echo "This runs before the agent" + - name: Another Step + uses: actions/setup-node@v4 + with: + node-version: '20' +``` + +**New Object Format** with named positions: +```yaml +steps: + pre: + - name: Pre Step + run: echo "Runs before checkout" + pre-agent: + - name: Pre Agent Step + run: echo "Runs after setup, before agent" + post-agent: + - name: Post Agent Step + run: echo "Runs after agent execution" + post: + - name: Final Step + run: echo "Runs at the very end" +``` + +**Standalone post-agent field** (preferred over post-steps): +```yaml +post-agent: + - name: Cleanup Step + run: echo "Runs after agent" + - name: Upload Results + uses: actions/upload-artifact@v4 + with: + name: results + path: output/ +``` + +### Step Execution Order + +The complete workflow execution order is: + +1. Pre-activation jobs (if any) +2. **Pre steps** (from `steps.pre`) +3. Checkout repository +4. Runtime setup (Node.js, Python, etc.) +5. **Pre-agent steps** (from `steps.pre-agent` or legacy `steps` array) +6. Create temp directories +7. Cache restoration +8. Git configuration +9. MCP server setup +10. Prompt generation +11. **Agent execution** (Claude, Codex, Copilot, or Custom) +12. Error validation +13. Git patch generation (if using safe-outputs with PRs) +14. **Post-agent steps** (from `steps.post-agent` or legacy `post-steps`) +15. **Post steps** (from `steps.post`) + +### Backward Compatibility + +The legacy `steps` and `post-steps` fields continue to work: +- `steps` (array) → maps to `pre-agent` position +- `post-steps` (array) → maps to `post-agent` position + +However, the new `post-agent` field is preferred over `post-steps` for clarity. + +### Step Merging with Imports + +When importing workflows that contain steps, imported steps are prepended to the corresponding position: + +```yaml +# imported-workflow.md +steps: + pre-agent: + - name: Imported Setup + run: echo "from import" + +# main-workflow.md +imports: + - shared/imported-workflow.md +steps: + pre-agent: + - name: Main Setup + run: echo "from main" +``` + +Result: Both steps run in order (imported first, then main). + ## Output Processing and Issue Creation ### Automatic GitHub Issue Creation diff --git a/pkg/workflow/compiler.go b/pkg/workflow/compiler.go index 64763d8b40d..7d2033aceb0 100644 --- a/pkg/workflow/compiler.go +++ b/pkg/workflow/compiler.go @@ -156,9 +156,9 @@ type WorkflowData struct { Env string If string TimeoutMinutes string - CustomSteps string // DEPRECATED: Legacy string-based steps (use ParsedSteps instead) - PostSteps string // DEPRECATED: Legacy string-based post-steps (use ParsedSteps instead) - ParsedSteps *WorkflowSteps // Structured steps configuration parsed from frontmatter + CustomSteps string // DEPRECATED: Legacy string-based steps (use ParsedSteps instead) + PostSteps string // DEPRECATED: Legacy string-based post-steps (use ParsedSteps instead) + ParsedSteps *WorkflowSteps // Structured steps configuration parsed from frontmatter RunsOn string Environment string // environment setting for the main job Container string // container setting for the main job @@ -924,7 +924,7 @@ func (c *Compiler) ParseWorkflowFile(markdownPath string) (*WorkflowData, error) // Keep legacy string fields for backward compatibility during transition workflowData.CustomSteps = c.extractTopLevelYAMLSection(result.Frontmatter, "steps") workflowData.PostSteps = c.extractTopLevelYAMLSection(result.Frontmatter, "post-steps") - + // Apply the legacy merging logic to CustomSteps if needed for compatibility if importsResult.MergedSteps != "" { // Parse imported steps from YAML array @@ -3156,7 +3156,6 @@ func (c *Compiler) generateFinalSteps(yaml *strings.Builder, data *WorkflowData) } } - // extractJobsFromFrontmatter extracts job configuration from frontmatter func (c *Compiler) extractJobsFromFrontmatter(frontmatter map[string]any) map[string]any { if jobs, exists := frontmatter["jobs"]; exists { From 17a5a28f1c6d72b458b32085e2e9d5e72e4c5955 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Oct 2025 22:06:15 +0000 Subject: [PATCH 4/5] Fix linting issues - Remove unused stepsToYAML function - Fix unnecessary assignment to blank identifier - All linting checks pass Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../github-agentic-workflows.instructions.md | 110 +----------------- .../docs/reference/frontmatter-full.md | 31 ++++- pkg/workflow/compiler.go | 2 +- pkg/workflow/step.go | 14 --- 4 files changed, 29 insertions(+), 128 deletions(-) diff --git a/.github/instructions/github-agentic-workflows.instructions.md b/.github/instructions/github-agentic-workflows.instructions.md index 6c069109bc8..ce6e12f89ca 100644 --- a/.github/instructions/github-agentic-workflows.instructions.md +++ b/.github/instructions/github-agentic-workflows.instructions.md @@ -52,9 +52,8 @@ The YAML frontmatter supports these fields: - **`if:`** - Conditional execution expression (string) - **`run-name:`** - Custom workflow run name (string) - **`name:`** - Workflow name (string) -- **`steps:`** - Custom workflow steps (array or object with named positions) -- **`post-agent:`** - Steps to run immediately after AI execution (array, preferred over post-steps) -- **`post-steps:`** - DEPRECATED: Use post-agent instead. Custom workflow steps to run after AI execution (object) +- **`steps:`** - Custom workflow steps (object) +- **`post-steps:`** - Custom workflow steps to run after AI execution (object) ### Agentic Workflow Specific Fields @@ -312,111 +311,6 @@ Cache-memory configurations can be imported from shared agentic workflows using The memory MCP server is automatically configured when `cache-memory` is enabled and works with both Claude and Custom engines. -## Custom Workflow Steps - -GitHub Agentic Workflows support custom workflow steps that can be placed at different positions relative to the agent execution. This allows you to run setup, configuration, or cleanup tasks at precise points in the workflow lifecycle. - -### Step Placement Positions - -Steps can be placed in four different positions: - -1. **`pre`**: Runs before checkout and runtime setup -2. **`pre-agent`**: Runs after setup but before agent execution (legacy `steps` field maps here) -3. **`post-agent`**: Runs immediately after agent execution (legacy `post-steps` field maps here) -4. **`post`**: Runs after all other steps are complete - -### Step Formats - -**Legacy Array Format** (maps to pre-agent position): -```yaml -steps: - - name: Setup Step - run: echo "This runs before the agent" - - name: Another Step - uses: actions/setup-node@v4 - with: - node-version: '20' -``` - -**New Object Format** with named positions: -```yaml -steps: - pre: - - name: Pre Step - run: echo "Runs before checkout" - pre-agent: - - name: Pre Agent Step - run: echo "Runs after setup, before agent" - post-agent: - - name: Post Agent Step - run: echo "Runs after agent execution" - post: - - name: Final Step - run: echo "Runs at the very end" -``` - -**Standalone post-agent field** (preferred over post-steps): -```yaml -post-agent: - - name: Cleanup Step - run: echo "Runs after agent" - - name: Upload Results - uses: actions/upload-artifact@v4 - with: - name: results - path: output/ -``` - -### Step Execution Order - -The complete workflow execution order is: - -1. Pre-activation jobs (if any) -2. **Pre steps** (from `steps.pre`) -3. Checkout repository -4. Runtime setup (Node.js, Python, etc.) -5. **Pre-agent steps** (from `steps.pre-agent` or legacy `steps` array) -6. Create temp directories -7. Cache restoration -8. Git configuration -9. MCP server setup -10. Prompt generation -11. **Agent execution** (Claude, Codex, Copilot, or Custom) -12. Error validation -13. Git patch generation (if using safe-outputs with PRs) -14. **Post-agent steps** (from `steps.post-agent` or legacy `post-steps`) -15. **Post steps** (from `steps.post`) - -### Backward Compatibility - -The legacy `steps` and `post-steps` fields continue to work: -- `steps` (array) → maps to `pre-agent` position -- `post-steps` (array) → maps to `post-agent` position - -However, the new `post-agent` field is preferred over `post-steps` for clarity. - -### Step Merging with Imports - -When importing workflows that contain steps, imported steps are prepended to the corresponding position: - -```yaml -# imported-workflow.md -steps: - pre-agent: - - name: Imported Setup - run: echo "from import" - -# main-workflow.md -imports: - - shared/imported-workflow.md -steps: - pre-agent: - - name: Main Setup - run: echo "from main" -``` - -Result: Both steps run in order (imported first, then main). - ## Output Processing and Issue Creation ### Automatic GitHub Issue Creation diff --git a/docs/src/content/docs/reference/frontmatter-full.md b/docs/src/content/docs/reference/frontmatter-full.md index 71593f47779..1160a8d2815 100644 --- a/docs/src/content/docs/reference/frontmatter-full.md +++ b/docs/src/content/docs/reference/frontmatter-full.md @@ -771,19 +771,40 @@ network: # (optional) if: "example-value" -# Custom workflow steps +# Custom workflow steps. Can be an array (legacy format, goes to pre-agent +# position) or an object with named positions (pre, pre-agent, post-agent, post) # (optional) # This field supports multiple formats (oneOf): -# Option 1: object +# Option 1: Object format with named step positions steps: - {} + # Steps to run before checkout and runtime setup + # (optional) + pre: [] -# Option 2: array + # Steps to run after setup but before agent execution + # (optional) + pre-agent: [] + + # Steps to run immediately after agent execution + # (optional) + post-agent: [] + + # Steps to run after all other steps are complete + # (optional) + post: [] + +# Option 2: Legacy array format (steps go to pre-agent position) steps: [] # Array items: undefined -# Custom workflow steps to run after AI execution +# Steps to run immediately after AI execution. Preferred over post-steps. +# (optional) +post-agent: [] + # Array items: + +# DEPRECATED: Use 'post-agent' instead. Custom workflow steps to run after AI +# execution # (optional) # This field supports multiple formats (oneOf): diff --git a/pkg/workflow/compiler.go b/pkg/workflow/compiler.go index 7d2033aceb0..90073671f7f 100644 --- a/pkg/workflow/compiler.go +++ b/pkg/workflow/compiler.go @@ -862,7 +862,7 @@ func (c *Compiler) ParseWorkflowFile(markdownPath string) (*WorkflowData, error) workflowData.TimeoutMinutes = c.extractTopLevelYAMLSection(result.Frontmatter, "timeout_minutes") // Parse steps using new structured approach - stepsData, _ := result.Frontmatter["steps"] + stepsData := result.Frontmatter["steps"] parsedSteps, err := ParseStepsFromFrontmatter(stepsData) if err != nil { return nil, fmt.Errorf("error parsing steps: %w", err) diff --git a/pkg/workflow/step.go b/pkg/workflow/step.go index eb6f2be17ec..1e34237347d 100644 --- a/pkg/workflow/step.go +++ b/pkg/workflow/step.go @@ -203,17 +203,3 @@ func marshalStep(step Step) (string, error) { func (ws *WorkflowSteps) IsEmpty() bool { return ws == nil || (len(ws.Pre) == 0 && len(ws.PreAgent) == 0 && len(ws.PostAgent) == 0 && len(ws.Post) == 0) } - -// ToYAML converts steps to YAML string for a specific position -func stepsToYAML(steps []Step) (string, error) { - if len(steps) == 0 { - return "", nil - } - - yamlBytes, err := yaml.Marshal(steps) - if err != nil { - return "", err - } - - return string(yamlBytes), nil -} From b090a8c2097f9e649ec720332dbc84c9df5daf92 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Oct 2025 22:35:18 +0000 Subject: [PATCH 5/5] Remove top-level post-agent and post-steps fields - Remove post-agent field from schema (only nested format supported) - Remove post-steps field from schema (fully deprecated) - Update compiler to remove parsing of top-level post-agent/post-steps - Remove fallback to legacy CustomSteps/PostSteps in rendering - Convert test-post-steps.md workflow to use nested format - Update all tests to use nested steps.post-agent format - Update documentation to remove references to top-level fields - Add comprehensive documentation for nested steps format All 67 workflows compile successfully. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .github/workflows/test-post-steps.lock.yml | 16 ++-- .github/workflows/test-post-steps.md | 49 +++++------ .../docs/reference/frontmatter-full.md | 18 ---- pkg/parser/schemas/main_workflow_schema.json | 31 ------- pkg/workflow/compiler.go | 76 ----------------- pkg/workflow/compiler_test.go | 85 ++++++++++--------- pkg/workflow/step_integration_test.go | 40 ++++----- 7 files changed, 98 insertions(+), 217 deletions(-) diff --git a/.github/workflows/test-post-steps.lock.yml b/.github/workflows/test-post-steps.lock.yml index 02a85e3962c..bfa17dbda94 100644 --- a/.github/workflows/test-post-steps.lock.yml +++ b/.github/workflows/test-post-steps.lock.yml @@ -13,7 +13,7 @@ # activation --> agent # ``` -name: "Test Post-Steps Workflow" +name: "Test Post-Agent Steps Workflow" "on": workflow_dispatch: null @@ -22,7 +22,7 @@ permissions: {} concurrency: group: "gh-aw-${{ github.workflow }}" -run-name: "Test Post-Steps Workflow" +run-name: "Test Post-Agent Steps Workflow" jobs: activation: @@ -170,9 +170,9 @@ jobs: run: | mkdir -p $(dirname "$GH_AW_PROMPT") cat > $GH_AW_PROMPT << 'EOF' - # Test Post-Steps Workflow + # Test Post-Agent Steps Workflow - This is a test workflow to validate that post-steps compile correctly with proper YAML indentation. + This is a test workflow to validate that post-agent steps compile correctly with proper YAML indentation. ## Your Task @@ -333,7 +333,7 @@ jobs: model: "", version: "", agent_version: process.env.AGENT_VERSION || "", - workflow_name: "Test Post-Steps Workflow", + workflow_name: "Test Post-Agent Steps Workflow", experimental: false, supports_tools_allowlist: true, supports_http_transport: true, @@ -1571,9 +1571,9 @@ jobs: if (typeof module === "undefined" || require.main === module) { main(); } - - name: Verify Post-Steps Execution + - name: Verify Post-Agent Steps Execution run: | - echo "✅ Post-steps are executing correctly" + echo "✅ Post-agent steps are executing correctly" echo "This step runs after the AI agent completes" - name: Upload Test Results uses: actions/upload-artifact@v4 @@ -1584,7 +1584,7 @@ jobs: retention-days: 1 if: always() - name: Final Summary - run: "echo \"## Post-Steps Test Summary\" >> $GITHUB_STEP_SUMMARY\necho \"✅ All post-steps executed successfully\" >> $GITHUB_STEP_SUMMARY\necho \"This validates the post-steps indentation fix\" >> $GITHUB_STEP_SUMMARY\n" + run: "echo \"## Post-Agent Steps Test Summary\" >> $GITHUB_STEP_SUMMARY\necho \"✅ All post-agent steps executed successfully\" >> $GITHUB_STEP_SUMMARY\necho \"This validates the post-agent steps indentation fix\" >> $GITHUB_STEP_SUMMARY\n" pre_activation: runs-on: ubuntu-latest diff --git a/.github/workflows/test-post-steps.md b/.github/workflows/test-post-steps.md index 4f5fd3012b9..2ff43838fbd 100644 --- a/.github/workflows/test-post-steps.md +++ b/.github/workflows/test-post-steps.md @@ -1,6 +1,6 @@ --- -# Test workflow for post-steps functionality -# This workflow validates that post-steps compile correctly and are properly indented +# Test workflow for post-agent steps functionality +# This workflow validates that post-agent steps compile correctly and are properly indented on: workflow_dispatch: @@ -16,33 +16,34 @@ tools: allowed: [get_repository] # Steps that run after AI execution -post-steps: - - name: Verify Post-Steps Execution - run: | - echo "✅ Post-steps are executing correctly" - echo "This step runs after the AI agent completes" - - - name: Upload Test Results - if: always() - uses: actions/upload-artifact@v4 - with: - name: post-steps-test-results - path: /tmp/gh-aw/ - retention-days: 1 - if-no-files-found: ignore - - - name: Final Summary - run: | - echo "## Post-Steps Test Summary" >> $GITHUB_STEP_SUMMARY - echo "✅ All post-steps executed successfully" >> $GITHUB_STEP_SUMMARY - echo "This validates the post-steps indentation fix" >> $GITHUB_STEP_SUMMARY +steps: + post-agent: + - name: Verify Post-Agent Steps Execution + run: | + echo "✅ Post-agent steps are executing correctly" + echo "This step runs after the AI agent completes" + + - name: Upload Test Results + if: always() + uses: actions/upload-artifact@v4 + with: + name: post-steps-test-results + path: /tmp/gh-aw/ + retention-days: 1 + if-no-files-found: ignore + + - name: Final Summary + run: | + echo "## Post-Agent Steps Test Summary" >> $GITHUB_STEP_SUMMARY + echo "✅ All post-agent steps executed successfully" >> $GITHUB_STEP_SUMMARY + echo "This validates the post-agent steps indentation fix" >> $GITHUB_STEP_SUMMARY timeout_minutes: 5 --- -# Test Post-Steps Workflow +# Test Post-Agent Steps Workflow -This is a test workflow to validate that post-steps compile correctly with proper YAML indentation. +This is a test workflow to validate that post-agent steps compile correctly with proper YAML indentation. ## Your Task diff --git a/docs/src/content/docs/reference/frontmatter-full.md b/docs/src/content/docs/reference/frontmatter-full.md index 1160a8d2815..eb219eb9ed4 100644 --- a/docs/src/content/docs/reference/frontmatter-full.md +++ b/docs/src/content/docs/reference/frontmatter-full.md @@ -798,24 +798,6 @@ steps: steps: [] # Array items: undefined -# Steps to run immediately after AI execution. Preferred over post-steps. -# (optional) -post-agent: [] - # Array items: - -# DEPRECATED: Use 'post-agent' instead. Custom workflow steps to run after AI -# execution -# (optional) -# This field supports multiple formats (oneOf): - -# Option 1: object -post-steps: - {} - -# Option 2: array -post-steps: [] - # Array items: undefined - # AI engine configuration that specifies which AI processor interprets and # executes the markdown content of the workflow. Defaults to 'claude'. # (optional) diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index 6ef911aa1e9..b2086266c84 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -1551,37 +1551,6 @@ } ] }, - "post-agent": { - "description": "Steps to run immediately after AI execution. Preferred over post-steps.", - "type": "array", - "items": { - "type": "object", - "additionalProperties": true - } - }, - "post-steps": { - "description": "DEPRECATED: Use 'post-agent' instead. Custom workflow steps to run after AI execution", - "oneOf": [ - { - "type": "object", - "additionalProperties": true - }, - { - "type": "array", - "items": { - "oneOf": [ - { - "type": "string" - }, - { - "type": "object", - "additionalProperties": true - } - ] - } - } - ] - }, "engine": { "description": "AI engine configuration that specifies which AI processor interprets and executes the markdown content of the workflow. Defaults to 'claude'.", "$ref": "#/$defs/engine_config" diff --git a/pkg/workflow/compiler.go b/pkg/workflow/compiler.go index 90073671f7f..85336b3b13c 100644 --- a/pkg/workflow/compiler.go +++ b/pkg/workflow/compiler.go @@ -868,43 +868,6 @@ func (c *Compiler) ParseWorkflowFile(markdownPath string) (*WorkflowData, error) return nil, fmt.Errorf("error parsing steps: %w", err) } - // Also parse post-agent field (new preferred field) - postAgentData, hasPostAgent := result.Frontmatter["post-agent"] - if hasPostAgent { - postAgentSteps, err := ParseStepsFromFrontmatter(postAgentData) - if err != nil { - return nil, fmt.Errorf("error parsing post-agent: %w", err) - } - if postAgentSteps != nil { - // Merge post-agent into the appropriate position - if parsedSteps == nil { - parsedSteps = &WorkflowSteps{} - } - // post-agent field maps directly to PostAgent position - // If it's an array, it will be in PreAgent, but we want it in PostAgent - parsedSteps.PostAgent = append(parsedSteps.PostAgent, postAgentSteps.PreAgent...) - parsedSteps.PostAgent = append(parsedSteps.PostAgent, postAgentSteps.PostAgent...) - } - } - - // Also parse post-steps from frontmatter (legacy support - goes to PostAgent position) - postStepsData, hasPostSteps := result.Frontmatter["post-steps"] - if hasPostSteps { - postSteps, err := ParseStepsFromFrontmatter(postStepsData) - if err != nil { - return nil, fmt.Errorf("error parsing post-steps: %w", err) - } - if postSteps != nil { - // Merge post-steps into the appropriate position - if parsedSteps == nil { - parsedSteps = &WorkflowSteps{} - } - // Legacy post-steps go to PostAgent position - parsedSteps.PostAgent = append(parsedSteps.PostAgent, postSteps.PreAgent...) - parsedSteps.PostAgent = append(parsedSteps.PostAgent, postSteps.PostAgent...) - } - } - // Merge imported steps if any var importedSteps *WorkflowSteps if importsResult.MergedSteps != "" { @@ -923,7 +886,6 @@ func (c *Compiler) ParseWorkflowFile(markdownPath string) (*WorkflowData, error) // Keep legacy string fields for backward compatibility during transition workflowData.CustomSteps = c.extractTopLevelYAMLSection(result.Frontmatter, "steps") - workflowData.PostSteps = c.extractTopLevelYAMLSection(result.Frontmatter, "post-steps") // Apply the legacy merging logic to CustomSteps if needed for compatibility if importsResult.MergedSteps != "" { @@ -2616,22 +2578,6 @@ func (c *Compiler) generateMainJobSteps(yaml *strings.Builder, data *WorkflowDat // Add "pre-agent" steps (these run before agent execution, where legacy "steps" go) if data.ParsedSteps != nil && len(data.ParsedSteps.PreAgent) > 0 { renderStepsAtPosition(yaml, data.ParsedSteps.PreAgent) - } else if data.CustomSteps != "" { - // Fallback to legacy CustomSteps for backward compatibility - // Remove "steps:" line and adjust indentation - lines := strings.Split(data.CustomSteps, "\n") - if len(lines) > 1 { - for _, line := range lines[1:] { - // Skip empty lines - if strings.TrimSpace(line) == "" { - yaml.WriteString("\n") - continue - } - - // Simply add 6 spaces for job context indentation - yaml.WriteString(" " + line + "\n") - } - } } // Create /tmp/gh-aw/ base directory for all temporary files @@ -3123,28 +3069,6 @@ func (c *Compiler) generatePostSteps(yaml *strings.Builder, data *WorkflowData) // Add "post-agent" steps (these run immediately after agent execution) if data.ParsedSteps != nil && len(data.ParsedSteps.PostAgent) > 0 { renderStepsAtPosition(yaml, data.ParsedSteps.PostAgent) - } else if data.PostSteps != "" { - // Fallback to legacy PostSteps for backward compatibility - // Remove "post-steps:" line and adjust indentation, similar to CustomSteps processing - lines := strings.Split(data.PostSteps, "\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") - } - } - } } } diff --git a/pkg/workflow/compiler_test.go b/pkg/workflow/compiler_test.go index f1192c0bd09..a02772eecb9 100644 --- a/pkg/workflow/compiler_test.go +++ b/pkg/workflow/compiler_test.go @@ -3858,7 +3858,7 @@ func TestPostStepsGeneration(t *testing.T) { } defer os.RemoveAll(tmpDir) - // Test case with both steps and post-steps + // Test case with both pre-agent and post-agent steps testContent := `--- on: push permissions: @@ -3868,22 +3868,23 @@ tools: github: allowed: [list_issues] steps: - - name: Pre AI Step - run: echo "This runs before AI" -post-steps: - - name: Post AI Step - run: echo "This runs after AI" - - name: Another Post Step - uses: actions/upload-artifact@v4 - with: - name: test-artifact - path: test-file.txt + pre-agent: + - name: Pre AI Step + run: echo "This runs before AI" + post-agent: + - name: Post AI Step + run: echo "This runs after AI" + - name: Another Post Step + uses: actions/upload-artifact@v4 + with: + name: test-artifact + path: test-file.txt engine: claude --- -# Test Post Steps Workflow +# Test Post-Agent Steps Workflow -This workflow tests the post-steps functionality. +This workflow tests the post-agent steps functionality. ` testFile := filepath.Join(tmpDir, "test-post-steps.md") @@ -3951,7 +3952,7 @@ func TestPostStepsOnly(t *testing.T) { } defer os.RemoveAll(tmpDir) - // Test case with only post-steps (no pre-steps) + // Test case with only post-agent steps (no pre-agent steps) testContent := `--- on: issues permissions: @@ -3960,15 +3961,16 @@ permissions: tools: github: allowed: [list_issues] -post-steps: - - name: Only Post Step - run: echo "This runs after AI only" +steps: + post-agent: + - name: Only Post Step + run: echo "This runs after AI only" engine: claude --- -# Test Post Steps Only Workflow +# Test Post-Agent Steps Only Workflow -This workflow tests post-steps without pre-steps. +This workflow tests post-agent steps without pre-agent steps. ` testFile := filepath.Join(tmpDir, "test-post-steps-only.md") @@ -3981,7 +3983,7 @@ This workflow tests post-steps without pre-steps. // Compile the workflow err = compiler.CompileWorkflow(testFile) if err != nil { - t.Fatalf("Unexpected error compiling workflow with post-steps only: %v", err) + t.Fatalf("Unexpected error compiling workflow with post-agent steps only: %v", err) } // Read the generated lock file @@ -5683,7 +5685,7 @@ func TestPostStepsIndentationFix(t *testing.T) { } defer os.RemoveAll(tmpDir) - // Test case with various post-steps configurations + // Test case with various post-agent steps configurations testContent := `--- on: push permissions: @@ -5691,26 +5693,27 @@ permissions: tools: github: allowed: [list_issues] -post-steps: - - name: First Post Step - run: echo "first" - - name: Second Post Step - uses: actions/upload-artifact@v4 - with: - name: test-artifact - path: test-file.txt - retention-days: 7 - - name: Third Post Step - if: success() - run: | - echo "multiline" - echo "script" +steps: + post-agent: + - name: First Post Step + run: echo "first" + - name: Second Post Step + uses: actions/upload-artifact@v4 + with: + name: test-artifact + path: test-file.txt + retention-days: 7 + - name: Third Post Step + if: success() + run: | + echo "multiline" + echo "script" engine: claude --- -# Test Post Steps Indentation +# Test Post-Agent Steps Indentation -Test post-steps indentation fix. +Test post-agent steps indentation fix. ` testFile := filepath.Join(tmpDir, "test-post-steps-indentation.md") @@ -5735,16 +5738,16 @@ Test post-steps indentation fix. lockContent := string(content) - // Verify all post-steps are present + // Verify all post-agent steps are present if !strings.Contains(lockContent, "- name: First Post Step") { - t.Error("Expected post-step 'First Post Step' to be in generated workflow") + t.Error("Expected post-agent step 'First Post Step' to be in generated workflow") } if !strings.Contains(lockContent, "- name: Second Post Step") { - t.Error("Expected post-step 'Second Post Step' to be in generated workflow") + t.Error("Expected post-agent step 'Second Post Step' to be in generated workflow") } // Note: "Third Post Step" has an 'if' condition, so it appears as "name: Third Post Step" not "- name:" if !strings.Contains(lockContent, "name: Third Post Step") { - t.Error("Expected post-step 'Third Post Step' to be in generated workflow") + t.Error("Expected post-agent step 'Third Post Step' to be in generated workflow") } // Verify indentation is correct (6 spaces for list items, 8 for properties) diff --git a/pkg/workflow/step_integration_test.go b/pkg/workflow/step_integration_test.go index 9ebe4194292..56a9764db1e 100644 --- a/pkg/workflow/step_integration_test.go +++ b/pkg/workflow/step_integration_test.go @@ -119,24 +119,25 @@ func TestNewStepsFormat_PostAgentField(t *testing.T) { } defer os.RemoveAll(tmpDir) - // Test new post-agent field (instead of post-steps) + // Test post-agent steps within steps object testContent := `--- on: push permissions: contents: read tools: edit: -post-agent: - - name: Post Agent Step - run: echo "runs after agent" - - name: Another Post Agent Step - run: echo "also runs after agent" +steps: + post-agent: + - name: Post Agent Step + run: echo "runs after agent" + - name: Another Post Agent Step + run: echo "also runs after agent" engine: claude --- -# Test Post-Agent Field +# Test Post-Agent Steps -This tests the new post-agent field. +This tests the post-agent steps within the steps object. ` testFile := filepath.Join(tmpDir, "test-post-agent.md") @@ -192,7 +193,7 @@ func TestLegacyStepsFormat_BackwardCompatibility(t *testing.T) { } defer os.RemoveAll(tmpDir) - // Test legacy array format still works + // Test legacy array format still works for pre-agent steps testContent := `--- on: push permissions: @@ -200,19 +201,20 @@ permissions: tools: edit: steps: - - name: Legacy Step 1 - run: echo "legacy step 1" - - name: Legacy Step 2 - run: echo "legacy step 2" -post-steps: - - name: Legacy Post Step - run: echo "legacy post step" + pre-agent: + - name: Legacy Step 1 + run: echo "legacy step 1" + - name: Legacy Step 2 + run: echo "legacy step 2" + post-agent: + - name: Legacy Post Agent Step + run: echo "legacy post-agent step" engine: claude --- # Test Legacy Format -This tests backward compatibility with the legacy array format. +This tests backward compatibility with the legacy array format converted to object format. ` testFile := filepath.Join(tmpDir, "test-legacy.md") @@ -244,8 +246,8 @@ This tests backward compatibility with the legacy array format. if !strings.Contains(lockContent, "- name: Legacy Step 2") { t.Error("Expected 'Legacy Step 2' to be in generated workflow") } - if !strings.Contains(lockContent, "- name: Legacy Post Step") { - t.Error("Expected 'Legacy Post Step' to be in generated workflow") + if !strings.Contains(lockContent, "- name: Legacy Post Agent Step") { + t.Error("Expected 'Legacy Post Agent Step' to be in generated workflow") } t.Log("Legacy format backward compatibility verified")