diff --git a/.github/aw/create-agentic-workflow.md b/.github/aw/create-agentic-workflow.md index 4ee440571b1..39b38ff04a1 100644 --- a/.github/aw/create-agentic-workflow.md +++ b/.github/aw/create-agentic-workflow.md @@ -664,6 +664,36 @@ This gives users the choice of triggering via comment (`/deploy`) or via label, - `slash_command` full reference: https://github.github.com/gh-aw/reference/command-triggers/ - `label_command` and LabelOps: https://github.github.com/gh-aw/patterns/label-ops/ +## Creating Monitoring Workflows + +Use `workflow_run` to react to CI/CD pipelines in the same repository. Set `on.workflow_run.conclusion` to filter by result — the compiler converts it into a job `if` condition automatically. + +```aw wrap +--- +on: + workflow_run: + workflows: ["CI"] + types: [completed] + conclusion: failure # or: [failure, timed_out] +permissions: + contents: read +tools: + github: + toolsets: [default] +safe-outputs: + add-comment: + max: 1 +--- + +The CI workflow failed for branch `${{ github.event.workflow_run.head_branch }}`. + +Use the GitHub MCP tools to find the open pull request for branch `${{ github.event.workflow_run.head_branch }}`. Post a concise comment on that PR summarising the failure and suggesting next steps for the author. +``` + +Valid conclusion values: `success`, `failure`, `cancelled`, `skipped`, `timed_out`, `action_required`, `neutral`, `stale`. + +> ⚠️ `workflow_run` only works for workflows in the **same repository**. Use `deployment_status` for external deployment services. + ## Best Practices ### Improver Coding Agents in Large Repositories diff --git a/actions/setup/js/aw_context.cjs b/actions/setup/js/aw_context.cjs index f4a73e0cda3..9652962febc 100644 --- a/actions/setup/js/aw_context.cjs +++ b/actions/setup/js/aw_context.cjs @@ -108,6 +108,7 @@ function resolveItemContext(payload) { * comment_id: string, * comment_node_id: string, * deployment_state: string, + * workflow_run_conclusion: string, * otel_trace_id: string, * otel_parent_span_id: string, * trigger_label: string @@ -128,6 +129,10 @@ function resolveItemContext(payload) { * "success") when the workflow was triggered by a deployment_status event. * Empty string for all other event types. Propagated to child workflows via * workflow_call so they can identify which state triggered the parent. + * - workflow_run_conclusion: The conclusion of the triggering workflow_run + * (e.g. "failure", "success", "cancelled", "timed_out") when the workflow was + * triggered by a workflow_run event. Empty string for all other event types. + * Propagated to child workflows via workflow_call. * - otel_trace_id: OTLP trace ID from the parent workflow's setup span. * Empty string when OTLP is not configured or the parent setup step has * not yet run. Used by child workflow setup steps to continue the same @@ -163,6 +168,10 @@ function buildAwContext() { // triggering event is deployment_status. Empty string for all other events. // Propagated to called workflows so they can access the deployment state. deployment_state: context.eventName === "deployment_status" ? (context.payload?.deployment_status?.state ?? "") : "", + // workflow_run_conclusion carries the conclusion of the triggering workflow_run + // when the event is workflow_run. Empty string for all other events. + // Propagated to called workflows so they can access the workflow run conclusion. + workflow_run_conclusion: context.eventName === "workflow_run" ? (context.payload?.workflow_run?.conclusion ?? "") : "", // Propagate the current OTLP trace ID to dispatched child workflows so that // composite actions share the same trace as their parent. Empty string when // OTLP is not configured or the parent setup step has not run yet. diff --git a/actions/setup/js/generate_aw_info.cjs b/actions/setup/js/generate_aw_info.cjs index 236d89c4277..f400da2040f 100644 --- a/actions/setup/js/generate_aw_info.cjs +++ b/actions/setup/js/generate_aw_info.cjs @@ -94,6 +94,14 @@ async function main(core, ctx) { awInfo.deployment_state = deploymentState; } + // Include workflow_run_conclusion when triggered by a workflow_run event. + // This makes the triggering run conclusion available to the agent without requiring it + // to read the raw event payload, and is propagated to child workflows via aw_context. + const workflowRunConclusion = ctx.payload?.workflow_run?.conclusion; + if (workflowRunConclusion && typeof workflowRunConclusion === "string") { + awInfo.workflow_run_conclusion = workflowRunConclusion; + } + // Include custom token weights when set (engine.token-weights in workflow frontmatter). // Deep structure validation is intentionally minimal here: the JSON schema and Go parser // already validate the structure at compile time. We only verify the top-level type to diff --git a/actions/setup/js/send_otlp_span.cjs b/actions/setup/js/send_otlp_span.cjs index 47875487556..6544a706cf3 100644 --- a/actions/setup/js/send_otlp_span.cjs +++ b/actions/setup/js/send_otlp_span.cjs @@ -525,6 +525,11 @@ async function sendJobSetupSpan(options = {}) { if (deploymentStateSetup) { attributes.push(buildAttr("gh-aw.deployment.state", deploymentStateSetup)); } + // Workflow run conclusion: from aw_info or aw_context propagation. + const workflowRunConclusion = (typeof awInfo.workflow_run_conclusion === "string" ? awInfo.workflow_run_conclusion : "") || (typeof awInfo.context?.workflow_run_conclusion === "string" ? awInfo.context.workflow_run_conclusion : ""); + if (workflowRunConclusion) { + attributes.push(buildAttr("gh-aw.workflow_run.conclusion", workflowRunConclusion)); + } attributes.push(buildAttr("gh-aw.staged", staged)); if (itemType) attributes.push(buildAttr("gh-aw.trigger.item_type", itemType)); if (itemNumber) attributes.push(buildAttr("gh-aw.trigger.item_number", itemNumber)); @@ -775,6 +780,11 @@ async function sendJobConclusionSpan(spanName, options = {}) { if (deploymentStateConclusion) { attributes.push(buildAttr("gh-aw.deployment.state", deploymentStateConclusion)); } + // Workflow run conclusion: from aw_info or aw_context propagation. + const workflowRunConclusion = (typeof awInfo.workflow_run_conclusion === "string" ? awInfo.workflow_run_conclusion : "") || (typeof awInfo.context?.workflow_run_conclusion === "string" ? awInfo.context.workflow_run_conclusion : ""); + if (workflowRunConclusion) { + attributes.push(buildAttr("gh-aw.workflow_run.conclusion", workflowRunConclusion)); + } attributes.push(buildAttr("gh-aw.staged", staged)); if (itemType) attributes.push(buildAttr("gh-aw.trigger.item_type", itemType)); if (itemNumber) attributes.push(buildAttr("gh-aw.trigger.item_number", itemNumber)); diff --git a/docs/adr/29089-compile-workflow-run-conclusion-into-if-condition.md b/docs/adr/29089-compile-workflow-run-conclusion-into-if-condition.md new file mode 100644 index 00000000000..eeb980916d1 --- /dev/null +++ b/docs/adr/29089-compile-workflow-run-conclusion-into-if-condition.md @@ -0,0 +1,80 @@ +# ADR-29089: Compile `on.workflow_run.conclusion` Filter into Job `if` Condition at Build Time + +**Date**: 2026-04-29 +**Status**: Draft +**Deciders**: pelikhan, Copilot + +--- + +## Part 1 — Narrative (Human-Friendly) + +### Context + +GitHub Actions' `workflow_run` trigger fires for any conclusion value (`success`, `failure`, `cancelled`, etc.) when `types: [completed]` is specified — there is no native way to filter by conclusion in the `on:` block. Users who want to react only to failed runs must write a `if:` expression guard, and that guard requires an additional `github.event_name != 'workflow_run'` prefix so the condition remains transparent when the workflow fires from other events (e.g., `workflow_dispatch`). This two-part guard is subtle, error-prone to hand-write, and inconsistent with the simpler `on.deployment_status.state` pattern the compiler already handles. + +### Decision + +We will add `on.workflow_run.conclusion: string | string[]` as a recognized frontmatter field and compile it at build time into a guarded GitHub Actions expression that is AND-merged with any existing `if:` condition on the activation job. The generated guard takes the form `github.event_name != 'workflow_run' || (github.event.workflow_run.conclusion == '')`, mirroring the pattern already established for `on.deployment_status.state`. The raw `conclusion:` field is commented out in the compiled YAML `on:` section to make clear it is not a native GitHub Actions key. + +### Alternatives Considered + +#### Alternative 1: Require Users to Write Manual `if:` Conditions + +Users would write `if: github.event.workflow_run.conclusion == 'failure'` directly in frontmatter. This was rejected because it omits the event_name guard, which silently breaks workflows that also respond to `workflow_dispatch` or other non-`workflow_run` triggers — the conclusion expression evaluates to false for those events, preventing the workflow from running at all. The correct two-part guard is non-obvious and would require documentation warnings; the compiler can generate it correctly every time instead. + +#### Alternative 2: Runtime Filtering in the Agent Context Script + +The `aw_context.cjs` setup action could check `workflow_run.conclusion` at runtime and exit early if it doesn't match. This was rejected because it still consumes GitHub Actions runner minutes to start the job before aborting, and it moves filtering logic into the runtime layer rather than the declarative compilation layer where similar patterns already live (`deployment_status.state`). + +### Consequences + +#### Positive +- Users declare intent declaratively (`conclusion: failure`) without needing to understand GitHub Actions expression syntax or the event_name guard pattern. +- Consistent with the existing `on.deployment_status.state` compiler feature, keeping the frontmatter DSL coherent and the compilation logic concentrated in one place. +- The event_name guard is generated correctly and automatically, eliminating a class of silent bugs. +- `workflow_run_conclusion` is propagated to child workflows via `aw_context`, making the triggering conclusion visible to the agent without reading the raw event payload. + +#### Negative +- Adds stateful flag complexity (`inWorkflowRun`, `inWorkflowRunConclusionArray`) to `commentOutProcessedFieldsInOnSection`, which already manages multiple section flags that can interact unexpectedly. +- The set of valid conclusion values (`success`, `failure`, `cancelled`, `skipped`, `timed_out`, `action_required`, `neutral`, `stale`) is documented only in the user-facing docs; if GitHub adds new conclusion values, the documentation must be updated manually. +- The compiler now silently transforms the `conclusion:` key — users inspecting compiled YAML must understand it has been moved to a job condition. + +#### Neutral +- The `workflow_run_conclusion` field is also added to `awInfo` and the OTEL spans, which slightly increases the payload size for all `workflow_run`-triggered runs. +- Tests for the new logic are in a dedicated file (`workflow_run_conclusion_test.go`), consistent with how the deployment_status tests are organized. + +--- + +## Part 2 — Normative Specification (RFC 2119) + +> The key words **MUST**, **MUST NOT**, **REQUIRED**, **SHALL**, **SHALL NOT**, **SHOULD**, **SHOULD NOT**, **RECOMMENDED**, **MAY**, and **OPTIONAL** in this section are to be interpreted as described in [RFC 2119](https://www.rfc-editor.org/rfc/rfc2119). + +### Frontmatter Compilation + +1. When `on.workflow_run.conclusion` is present in the workflow frontmatter, the compiler **MUST** convert it into a GitHub Actions job `if` expression of the form `github.event_name != 'workflow_run' || ()`. +2. When `on.workflow_run.conclusion` is a string, `` **MUST** be `github.event.workflow_run.conclusion == ''`. +3. When `on.workflow_run.conclusion` is an array of strings, `` **MUST** be the individual equality checks joined with ` || `. +4. The generated condition **MUST** be AND-merged with any existing `if:` expression from the frontmatter using the form `() && ()`. +5. The compiler **MUST** comment out the `conclusion:` key (and any array items beneath it) in the compiled YAML `on:` section, appending an explanatory comment. +6. The compiler **MUST NOT** comment out sibling keys of `workflow_run` (such as `workflows:` or `types:`) as a side effect of processing `conclusion:`. +7. If `on.workflow_run.conclusion` is absent or empty, the compiler **MUST NOT** add any conclusion condition to the job `if` expression. + +### Agent Context Propagation + +1. When a workflow is triggered by a `workflow_run` event, `buildAwContext()` **MUST** populate `workflow_run_conclusion` with the value of `context.payload.workflow_run.conclusion`. +2. For all other event types, `workflow_run_conclusion` **MUST** be set to an empty string. +3. Implementations **MUST** propagate `workflow_run_conclusion` to child workflows via `workflow_call` inputs, following the same pattern as `deployment_state`. +4. The `workflow_run_conclusion` field **SHOULD** be included in `awInfo` so agents can access the conclusion without reading the raw event payload. + +### OTEL Instrumentation + +1. When `workflow_run_conclusion` is non-empty, implementations **MUST** include a `gh-aw.workflow_run.conclusion` attribute on both the setup span and the conclusion span. +2. Implementations **MUST** read `workflow_run_conclusion` from `awInfo.workflow_run_conclusion` first, falling back to `awInfo.context.workflow_run_conclusion` for child workflows that received it via propagation. + +### Conformance + +An implementation is considered conformant with this ADR if it satisfies all **MUST** and **MUST NOT** requirements above. Failure to meet any **MUST** or **MUST NOT** requirement constitutes non-conformance. + +--- + +*This is a DRAFT ADR generated by the [Design Decision Gate](https://github.com/github/gh-aw/actions/runs/25108336032) workflow. The PR author must review, complete, and finalize this document before the PR can merge.* diff --git a/pkg/workflow/compiler_orchestrator_workflow.go b/pkg/workflow/compiler_orchestrator_workflow.go index c053f0d63d4..36720ae1f69 100644 --- a/pkg/workflow/compiler_orchestrator_workflow.go +++ b/pkg/workflow/compiler_orchestrator_workflow.go @@ -128,7 +128,9 @@ func (c *Compiler) ParseWorkflowFile(markdownPath string) (*WorkflowData, error) workflowData.ActionPinWarnings = c.actionPinWarnings // Extract YAML configuration sections from frontmatter - c.extractYAMLSections(result.Frontmatter, workflowData) + if err := c.extractYAMLSections(result.Frontmatter, workflowData); err != nil { + return nil, formatCompilerError(cleanPath, "error", err.Error(), err) + } // Merge observability config from imports into RawFrontmatter so that injectOTLPConfig // can see an OTLP endpoint defined in an imported workflow (first-wins from imports). diff --git a/pkg/workflow/compiler_string_api.go b/pkg/workflow/compiler_string_api.go index 0a9982d4655..6d824874a3d 100644 --- a/pkg/workflow/compiler_string_api.go +++ b/pkg/workflow/compiler_string_api.go @@ -166,7 +166,9 @@ func (c *Compiler) ParseWorkflowString(content string, virtualPath string) (*Wor workflowData.ActionPinWarnings = c.actionPinWarnings // Extract YAML configuration sections - c.extractYAMLSections(parseResult.frontmatterResult.Frontmatter, workflowData) + if err := c.extractYAMLSections(parseResult.frontmatterResult.Frontmatter, workflowData); err != nil { + return nil, fmt.Errorf("failed to extract YAML sections: %w", err) + } // Merge features from imports if len(engineSetup.importsResult.MergedFeatures) > 0 { diff --git a/pkg/workflow/frontmatter_extraction_yaml.go b/pkg/workflow/frontmatter_extraction_yaml.go index 71a06e41c50..e6515f1073d 100644 --- a/pkg/workflow/frontmatter_extraction_yaml.go +++ b/pkg/workflow/frontmatter_extraction_yaml.go @@ -132,6 +132,8 @@ func (c *Compiler) commentOutProcessedFieldsInOnSection(yamlStr string, frontmat inDiscussion := false inIssueComment := false inDeploymentStatus := false + inWorkflowRun := false + inWorkflowRunConclusionArray := false inForksArray := false inSkipIfMatch := false inSkipIfNoMatch := false @@ -158,6 +160,8 @@ func (c *Compiler) commentOutProcessedFieldsInOnSection(yamlStr string, frontmat inDiscussion = false inIssueComment = false inDeploymentStatus = false + inWorkflowRun = false + inWorkflowRunConclusionArray = false currentSection = "pull_request" result = append(result, line) continue @@ -168,6 +172,8 @@ func (c *Compiler) commentOutProcessedFieldsInOnSection(yamlStr string, frontmat inDiscussion = false inIssueComment = false inDeploymentStatus = false + inWorkflowRun = false + inWorkflowRunConclusionArray = false currentSection = "issues" result = append(result, line) continue @@ -178,6 +184,8 @@ func (c *Compiler) commentOutProcessedFieldsInOnSection(yamlStr string, frontmat inIssues = false inIssueComment = false inDeploymentStatus = false + inWorkflowRun = false + inWorkflowRunConclusionArray = false currentSection = "discussion" result = append(result, line) continue @@ -188,12 +196,26 @@ func (c *Compiler) commentOutProcessedFieldsInOnSection(yamlStr string, frontmat inIssues = false inDiscussion = false inDeploymentStatus = false + inWorkflowRun = false + inWorkflowRunConclusionArray = false currentSection = "issue_comment" result = append(result, line) continue } if strings.Contains(line, "deployment_status:") { inDeploymentStatus = true + inWorkflowRun = false + inPullRequest = false + inIssues = false + inDiscussion = false + inIssueComment = false + currentSection = "" + result = append(result, line) + continue + } + if strings.Contains(line, "workflow_run:") { + inWorkflowRun = true + inDeploymentStatus = false inPullRequest = false inIssues = false inDiscussion = false @@ -222,6 +244,12 @@ func (c *Compiler) commentOutProcessedFieldsInOnSection(yamlStr string, frontmat inDeploymentStatus = false } + // Check if we're leaving the workflow_run section + if inWorkflowRun && strings.TrimSpace(line) != "" && !strings.HasPrefix(line, " ") && !strings.HasPrefix(line, "\t") { + inWorkflowRun = false + inWorkflowRunConclusionArray = false + } + trimmedLine := strings.TrimSpace(line) // Skip marker lines in the YAML output @@ -544,6 +572,17 @@ func (c *Compiler) commentOutProcessedFieldsInOnSection(yamlStr string, frontmat // Comment out array items inside deployment_status.state shouldComment = true commentReason = " # State filtering compiled into if condition" + } else if inWorkflowRun && strings.HasPrefix(trimmedLine, "conclusion:") { + shouldComment = true + commentReason = " # Conclusion filtering compiled into if condition" + inWorkflowRunConclusionArray = true + } else if inWorkflowRunConclusionArray && strings.HasPrefix(trimmedLine, "-") { + // Comment out array items inside workflow_run.conclusion + shouldComment = true + commentReason = " # Conclusion filtering compiled into if condition" + } else if inWorkflowRun && !strings.HasPrefix(trimmedLine, "-") && strings.Contains(trimmedLine, ":") { + // Any new field inside workflow_run resets the conclusion array tracker + inWorkflowRunConclusionArray = false } else if (inPullRequest || inIssues || inDiscussion || inIssueComment) && strings.HasPrefix(trimmedLine, "lock-for-agent:") { shouldComment = true commentReason = " # Lock-for-agent processed as issue locking in activation job" @@ -703,8 +742,9 @@ func (c *Compiler) extractPermissions(frontmatter map[string]any) string { } // extractIfCondition extracts the if condition from frontmatter, returning just the expression -// without the "if: " prefix. Also merges any condition derived from on.deployment_status.state. -func (c *Compiler) extractIfCondition(frontmatter map[string]any) string { +// without the "if: " prefix. Also merges any condition derived from on.deployment_status.state +// and on.workflow_run.conclusion. +func (c *Compiler) extractIfCondition(frontmatter map[string]any) (string, error) { var ifExpr string if value, exists := frontmatter["if"]; exists { if strValue, ok := value.(string); ok { @@ -725,7 +765,21 @@ func (c *Compiler) extractIfCondition(frontmatter map[string]any) string { } } - return ifExpr + // Merge any condition generated from on.workflow_run.conclusion + conclusionCondition, err := extractWorkflowRunConclusionCondition(frontmatter) + if err != nil { + return "", err + } + if conclusionCondition != "" { + frontmatterLog.Printf("Merging workflow_run conclusion condition: %s", conclusionCondition) + if ifExpr != "" { + ifExpr = "(" + ifExpr + ") && (" + conclusionCondition + ")" + } else { + ifExpr = conclusionCondition + } + } + + return ifExpr, nil } // extractDeploymentStatusStateCondition reads on.deployment_status.state and converts it @@ -781,6 +835,91 @@ func extractDeploymentStatusStateCondition(frontmatter map[string]any) string { return "github.event_name != 'deployment_status' || (" + stateExpr + ")" } +// validWorkflowRunConclusions is the exhaustive list of conclusion values that GitHub +// Actions emits for workflow_run events. Values outside this set are rejected at +// compile time to prevent expression injection (a raw value is interpolated directly +// into a GitHub Actions expression string). +var validWorkflowRunConclusions = []string{ + "success", + "failure", + "neutral", + "cancelled", + "skipped", + "timed_out", + "action_required", + "stale", +} + +// isValidWorkflowRunConclusion reports whether v is a recognised conclusion value. +func isValidWorkflowRunConclusion(v string) bool { + for _, valid := range validWorkflowRunConclusions { + if v == valid { + return true + } + } + return false +} + +// extractWorkflowRunConclusionCondition reads on.workflow_run.conclusion and converts it +// into a GitHub Actions expression string (without ${{ }} wrappers). Returns "" if not set. +func extractWorkflowRunConclusionCondition(frontmatter map[string]any) (string, error) { + onValue, ok := frontmatter["on"] + if !ok { + return "", nil + } + onMap, ok := onValue.(map[string]any) + if !ok { + return "", nil + } + wrValue, ok := onMap["workflow_run"] + if !ok { + return "", nil + } + wrMap, ok := wrValue.(map[string]any) + if !ok { + return "", nil + } + conclusionValue, ok := wrMap["conclusion"] + if !ok { + return "", nil + } + + var conclusions []string + switch v := conclusionValue.(type) { + case string: + conclusions = []string{v} + case []any: + for _, s := range v { + if str, ok := s.(string); ok { + conclusions = append(conclusions, str) + } + } + } + + if len(conclusions) == 0 { + return "", nil + } + + for _, c := range conclusions { + if !isValidWorkflowRunConclusion(c) { + return "", fmt.Errorf("invalid on.workflow_run.conclusion value %q: must be one of %s", + c, strings.Join(validWorkflowRunConclusions, ", ")) + } + } + + parts := make([]string, 0, len(conclusions)) + for _, c := range conclusions { + parts = append(parts, "github.event.workflow_run.conclusion == '"+c+"'") + } + conclusionExpr := strings.Join(parts, " || ") + + // Guard the conclusion check with an event_name test so the condition remains true + // when the workflow is triggered by other events (e.g. workflow_dispatch). + // Without the guard, a non-workflow_run event would see conclusion as + // empty/undefined and the entire activation condition would evaluate to false. + return "github.event_name != 'workflow_run' || (" + conclusionExpr + ")", nil +} + // extractExpressionFromIfString extracts the expression part from a string that might // contain "if: expression" or just "expression", returning just the expression func (c *Compiler) extractExpressionFromIfString(ifString string) string { diff --git a/pkg/workflow/workflow_builder.go b/pkg/workflow/workflow_builder.go index 79ca71537b0..6e7349e9204 100644 --- a/pkg/workflow/workflow_builder.go +++ b/pkg/workflow/workflow_builder.go @@ -143,7 +143,7 @@ func resolveInlinedImports(rawFrontmatter map[string]any) bool { } // extractYAMLSections extracts YAML configuration sections from frontmatter -func (c *Compiler) extractYAMLSections(frontmatter map[string]any, workflowData *WorkflowData) { +func (c *Compiler) extractYAMLSections(frontmatter map[string]any, workflowData *WorkflowData) error { workflowBuilderLog.Print("Extracting YAML sections from frontmatter") workflowData.On = c.extractTopLevelYAMLSection(frontmatter, "on") @@ -155,7 +155,12 @@ func (c *Compiler) extractYAMLSections(frontmatter map[string]any, workflowData workflowData.RunName = c.extractTopLevelYAMLSection(frontmatter, "run-name") workflowData.Env = c.extractTopLevelYAMLSection(frontmatter, "env") workflowData.Features = c.extractFeatures(frontmatter) - workflowData.If = c.extractIfCondition(frontmatter) + + ifCondition, err := c.extractIfCondition(frontmatter) + if err != nil { + return err + } + workflowData.If = ifCondition // Extract timeout-minutes (canonical form) workflowData.TimeoutMinutes = c.extractTopLevelYAMLSection(frontmatter, "timeout-minutes") @@ -170,6 +175,7 @@ func (c *Compiler) extractYAMLSections(frontmatter map[string]any, workflowData workflowData.Environment = c.extractTopLevelYAMLSection(frontmatter, "environment") workflowData.Container = c.extractTopLevelYAMLSection(frontmatter, "container") workflowData.Cache = c.extractTopLevelYAMLSection(frontmatter, "cache") + return nil } // extractConcurrencyJobDiscriminator reads the job-discriminator value from the diff --git a/pkg/workflow/workflow_run_conclusion_test.go b/pkg/workflow/workflow_run_conclusion_test.go new file mode 100644 index 00000000000..89e048dc9ed --- /dev/null +++ b/pkg/workflow/workflow_run_conclusion_test.go @@ -0,0 +1,332 @@ +//go:build !integration + +package workflow + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestExtractWorkflowRunConclusionCondition tests the standalone helper that converts +// on.workflow_run.conclusion into a GitHub Actions expression. +func TestExtractWorkflowRunConclusionCondition(t *testing.T) { + tests := []struct { + name string + frontmatter map[string]any + want string + wantErr bool + }{ + { + name: "no on field", + frontmatter: map[string]any{}, + want: "", + }, + { + name: "no workflow_run key", + frontmatter: map[string]any{ + "on": map[string]any{ + "push": map[string]any{}, + }, + }, + want: "", + }, + { + name: "workflow_run with no conclusion", + frontmatter: map[string]any{ + "on": map[string]any{ + "workflow_run": map[string]any{ + "workflows": []any{"CI"}, + "types": []any{"completed"}, + }, + }, + }, + want: "", + }, + { + name: "single conclusion string", + frontmatter: map[string]any{ + "on": map[string]any{ + "workflow_run": map[string]any{ + "workflows": []any{"CI"}, + "types": []any{"completed"}, + "conclusion": "failure", + }, + }, + }, + want: "github.event_name != 'workflow_run' || (github.event.workflow_run.conclusion == 'failure')", + }, + { + name: "single conclusion in array", + frontmatter: map[string]any{ + "on": map[string]any{ + "workflow_run": map[string]any{ + "workflows": []any{"CI"}, + "types": []any{"completed"}, + "conclusion": []any{"failure"}, + }, + }, + }, + want: "github.event_name != 'workflow_run' || (github.event.workflow_run.conclusion == 'failure')", + }, + { + name: "multiple conclusions in array", + frontmatter: map[string]any{ + "on": map[string]any{ + "workflow_run": map[string]any{ + "workflows": []any{"CI"}, + "types": []any{"completed"}, + "conclusion": []any{"failure", "timed_out"}, + }, + }, + }, + want: "github.event_name != 'workflow_run' || (github.event.workflow_run.conclusion == 'failure' || github.event.workflow_run.conclusion == 'timed_out')", + }, + { + name: "success conclusion", + frontmatter: map[string]any{ + "on": map[string]any{ + "workflow_run": map[string]any{ + "workflows": []any{"Deploy"}, + "types": []any{"completed"}, + "conclusion": "success", + }, + }, + }, + want: "github.event_name != 'workflow_run' || (github.event.workflow_run.conclusion == 'success')", + }, + { + name: "workflow_run value is not a map", + frontmatter: map[string]any{ + "on": map[string]any{ + "workflow_run": "completed", + }, + }, + want: "", + }, + { + name: "invalid conclusion value is rejected", + frontmatter: map[string]any{ + "on": map[string]any{ + "workflow_run": map[string]any{ + "workflows": []any{"CI"}, + "types": []any{"completed"}, + "conclusion": "invalid_value", + }, + }, + }, + wantErr: true, + }, + { + name: "conclusion with single quote injection is rejected", + frontmatter: map[string]any{ + "on": map[string]any{ + "workflow_run": map[string]any{ + "workflows": []any{"CI"}, + "types": []any{"completed"}, + "conclusion": "failure' || true || '", + }, + }, + }, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := extractWorkflowRunConclusionCondition(tt.frontmatter) + if tt.wantErr { + assert.Error(t, err, "should return error for invalid conclusion value") + return + } + require.NoError(t, err, "should not error for valid conclusion values") + assert.Equal(t, tt.want, got, "condition should match expected expression") + }) + } +} + +// TestExtractIfConditionMergesWorkflowRunConclusion tests that extractIfCondition +// merges an on.workflow_run.conclusion filter with the existing if expression. +func TestExtractIfConditionMergesWorkflowRunConclusion(t *testing.T) { + tests := []struct { + name string + frontmatter map[string]any + want string + wantErr bool + }{ + { + name: "conclusion only - no existing if", + frontmatter: map[string]any{ + "on": map[string]any{ + "workflow_run": map[string]any{ + "workflows": []any{"CI"}, + "types": []any{"completed"}, + "conclusion": "failure", + }, + }, + }, + want: "github.event_name != 'workflow_run' || (github.event.workflow_run.conclusion == 'failure')", + }, + { + name: "conclusion merges with existing bare if", + frontmatter: map[string]any{ + "on": map[string]any{ + "workflow_run": map[string]any{ + "workflows": []any{"CI"}, + "types": []any{"completed"}, + "conclusion": "failure", + }, + }, + "if": "github.actor != 'bot'", + }, + want: "(github.actor != 'bot') && (github.event_name != 'workflow_run' || (github.event.workflow_run.conclusion == 'failure'))", + }, + { + name: "conclusion merges with wrapped ${{ }} if", + frontmatter: map[string]any{ + "on": map[string]any{ + "workflow_run": map[string]any{ + "workflows": []any{"CI"}, + "types": []any{"completed"}, + "conclusion": "failure", + }, + }, + "if": "${{ github.actor != 'bot' }}", + }, + want: "(github.actor != 'bot') && (github.event_name != 'workflow_run' || (github.event.workflow_run.conclusion == 'failure'))", + }, + { + name: "multiple conclusions merge with existing if", + frontmatter: map[string]any{ + "on": map[string]any{ + "workflow_run": map[string]any{ + "workflows": []any{"CI", "Deploy"}, + "types": []any{"completed"}, + "conclusion": []any{"failure", "timed_out"}, + }, + }, + "if": "github.actor != 'dependabot[bot]'", + }, + want: "(github.actor != 'dependabot[bot]') && (github.event_name != 'workflow_run' || (github.event.workflow_run.conclusion == 'failure' || github.event.workflow_run.conclusion == 'timed_out'))", + }, + { + name: "no conclusion - existing if preserved", + frontmatter: map[string]any{ + "on": map[string]any{ + "workflow_run": map[string]any{ + "workflows": []any{"CI"}, + "types": []any{"completed"}, + }, + }, + "if": "${{ github.event.workflow_run.conclusion == 'failure' }}", + }, + want: "github.event.workflow_run.conclusion == 'failure'", + }, + { + name: "invalid conclusion propagates error", + frontmatter: map[string]any{ + "on": map[string]any{ + "workflow_run": map[string]any{ + "workflows": []any{"CI"}, + "types": []any{"completed"}, + "conclusion": "not_a_real_conclusion", + }, + }, + }, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + compiler := NewCompiler() + got, err := compiler.extractIfCondition(tt.frontmatter) + if tt.wantErr { + assert.Error(t, err, "should propagate error for invalid conclusion value") + return + } + require.NoError(t, err, "should not error for valid conclusion values") + assert.Equal(t, tt.want, got, "merged if condition should match expected expression") + }) + } +} + +// TestWorkflowRunConclusionCommentedInYAML verifies that the conclusion field is +// commented out in the compiled on: YAML section and a comment explaining the filtering +// is appended to the line. +func TestWorkflowRunConclusionCommentedInYAML(t *testing.T) { + tests := []struct { + name string + frontmatter map[string]any + wantCommented bool + wantOnContains []string + wantOnAbsent []string + }{ + { + name: "conclusion string is commented out", + frontmatter: map[string]any{ + "on": map[string]any{ + "workflow_run": map[string]any{ + "workflows": []any{"CI"}, + "types": []any{"completed"}, + "conclusion": "failure", + }, + }, + }, + wantCommented: true, + wantOnContains: []string{"workflow_run:", "workflows:", "types:", "# conclusion: failure"}, + }, + { + name: "conclusion array is commented out", + frontmatter: map[string]any{ + "on": map[string]any{ + "workflow_run": map[string]any{ + "workflows": []any{"CI"}, + "types": []any{"completed"}, + "conclusion": []any{"failure", "timed_out"}, + }, + }, + }, + wantCommented: true, + wantOnContains: []string{"workflow_run:", "# conclusion:", "# - failure"}, + }, + { + name: "no conclusion - unmodified", + frontmatter: map[string]any{ + "on": map[string]any{ + "workflow_run": map[string]any{ + "workflows": []any{"CI"}, + "types": []any{"completed"}, + }, + }, + }, + wantCommented: false, + wantOnContains: []string{"workflow_run:", "workflows:", "types:"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + compiler := NewCompiler() + compiler.SetWorkflowIdentifier("test.md") + + onSection := compiler.extractTopLevelYAMLSection(tt.frontmatter, "on") + require.NotEmpty(t, onSection, "on section should not be empty") + + if tt.wantCommented { + assert.Contains(t, onSection, "# Conclusion filtering compiled into if condition", + "on section should contain conclusion filter comment") + } + + for _, want := range tt.wantOnContains { + assert.Contains(t, onSection, want, + "on section should contain %q", want) + } + for _, absent := range tt.wantOnAbsent { + assert.NotContains(t, onSection, absent, + "on section should NOT contain %q", absent) + } + }) + } +}