diff --git a/.github/workflows/refiner.lock.yml b/.github/workflows/refiner.lock.yml index c5abb655c9c..cfae4a140ff 100644 --- a/.github/workflows/refiner.lock.yml +++ b/.github/workflows/refiner.lock.yml @@ -141,7 +141,7 @@ jobs: GH_AW_GITHUB_EVENT_COMMENT_ID: ${{ github.event.comment.id }} GH_AW_GITHUB_EVENT_DISCUSSION_NUMBER: ${{ github.event.discussion.number }} GH_AW_GITHUB_EVENT_ISSUE_NUMBER: ${{ github.event.issue.number }} - GH_AW_GITHUB_EVENT_PULL_REQUEST_NUMBER: ${{ github.event.pull_request.number }} + GH_AW_GITHUB_EVENT_PULL_REQUEST_NUMBER: ${{ github.event.pull_request.number || inputs.item_number }} GH_AW_GITHUB_EVENT_PULL_REQUEST_TITLE: ${{ github.event.pull_request.title }} GH_AW_GITHUB_REPOSITORY: ${{ github.repository }} GH_AW_GITHUB_RUN_ID: ${{ github.run_id }} @@ -203,7 +203,7 @@ jobs: uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 env: GH_AW_PROMPT: /tmp/gh-aw/aw-prompts/prompt.txt - GH_AW_GITHUB_EVENT_PULL_REQUEST_NUMBER: ${{ github.event.pull_request.number }} + GH_AW_GITHUB_EVENT_PULL_REQUEST_NUMBER: ${{ github.event.pull_request.number || inputs.item_number }} GH_AW_GITHUB_EVENT_PULL_REQUEST_TITLE: ${{ github.event.pull_request.title }} GH_AW_GITHUB_REPOSITORY: ${{ github.repository }} GH_AW_GITHUB_RUN_ID: ${{ github.run_id }} @@ -221,7 +221,7 @@ jobs: GH_AW_GITHUB_EVENT_COMMENT_ID: ${{ github.event.comment.id }} GH_AW_GITHUB_EVENT_DISCUSSION_NUMBER: ${{ github.event.discussion.number }} GH_AW_GITHUB_EVENT_ISSUE_NUMBER: ${{ github.event.issue.number }} - GH_AW_GITHUB_EVENT_PULL_REQUEST_NUMBER: ${{ github.event.pull_request.number }} + GH_AW_GITHUB_EVENT_PULL_REQUEST_NUMBER: ${{ github.event.pull_request.number || inputs.item_number }} GH_AW_GITHUB_EVENT_PULL_REQUEST_TITLE: ${{ github.event.pull_request.title }} GH_AW_GITHUB_REPOSITORY: ${{ github.repository }} GH_AW_GITHUB_RUN_ID: ${{ github.run_id }} diff --git a/pkg/workflow/compiler_orchestrator_workflow.go b/pkg/workflow/compiler_orchestrator_workflow.go index 5919d87d398..7b1232b6b0d 100644 --- a/pkg/workflow/compiler_orchestrator_workflow.go +++ b/pkg/workflow/compiler_orchestrator_workflow.go @@ -220,6 +220,7 @@ func (c *Compiler) extractYAMLSections(frontmatter map[string]any, workflowData orchestratorWorkflowLog.Print("Extracting YAML sections from frontmatter") workflowData.On = c.extractTopLevelYAMLSection(frontmatter, "on") + workflowData.HasDispatchItemNumber = extractDispatchItemNumber(frontmatter) workflowData.Permissions = c.extractPermissions(frontmatter) workflowData.Network = c.extractTopLevelYAMLSection(frontmatter, "network") workflowData.Concurrency = c.extractTopLevelYAMLSection(frontmatter, "concurrency") @@ -237,6 +238,39 @@ func (c *Compiler) extractYAMLSections(frontmatter map[string]any, workflowData workflowData.Cache = c.extractTopLevelYAMLSection(frontmatter, "cache") } +// extractDispatchItemNumber reports whether the frontmatter's on.workflow_dispatch +// trigger exposes an item_number input. This is the signature produced by the label +// trigger shorthand (e.g. "on: pull_request labeled my-label"). Reading the +// structured map avoids re-parsing the rendered YAML string later. +func extractDispatchItemNumber(frontmatter map[string]any) bool { + onVal, ok := frontmatter["on"] + if !ok { + return false + } + onMap, ok := onVal.(map[string]any) + if !ok { + return false + } + wdVal, ok := onMap["workflow_dispatch"] + if !ok { + return false + } + wdMap, ok := wdVal.(map[string]any) + if !ok { + return false + } + inputsVal, ok := wdMap["inputs"] + if !ok { + return false + } + inputsMap, ok := inputsVal.(map[string]any) + if !ok { + return false + } + _, ok = inputsMap["item_number"] + return ok +} + // processAndMergeSteps handles the merging of imported steps with main workflow steps func (c *Compiler) processAndMergeSteps(frontmatter map[string]any, workflowData *WorkflowData, importsResult *parser.ImportsResult) { orchestratorWorkflowLog.Print("Processing and merging custom steps") diff --git a/pkg/workflow/compiler_orchestrator_workflow_test.go b/pkg/workflow/compiler_orchestrator_workflow_test.go index 0281b655bc3..0b2229108e7 100644 --- a/pkg/workflow/compiler_orchestrator_workflow_test.go +++ b/pkg/workflow/compiler_orchestrator_workflow_test.go @@ -1448,3 +1448,130 @@ func TestBuildInitialWorkflowData_FieldMapping(t *testing.T) { assert.Equal(t, []string{"/imported1"}, workflowData.ImportedFiles) assert.NotNil(t, workflowData.ImportInputs) } + +// TestExtractDispatchItemNumber tests that the item_number presence is detected +// directly from the frontmatter map rather than from re-parsing YAML strings. +func TestExtractDispatchItemNumber(t *testing.T) { + tests := []struct { + name string + frontmatter map[string]any + want bool + }{ + { + name: "label trigger shorthand PR workflow has item_number", + frontmatter: map[string]any{ + "on": map[string]any{ + "pull_request": map[string]any{"types": []any{"labeled"}}, + "workflow_dispatch": map[string]any{ + "inputs": map[string]any{ + "item_number": map[string]any{ + "description": "The number of the pull request", + "required": true, + "type": "string", + }, + }, + }, + }, + }, + want: true, + }, + { + name: "label trigger shorthand issue workflow has item_number", + frontmatter: map[string]any{ + "on": map[string]any{ + "issues": map[string]any{"types": []any{"labeled"}}, + "workflow_dispatch": map[string]any{ + "inputs": map[string]any{ + "item_number": map[string]any{ + "description": "The number of the issue", + "required": true, + "type": "string", + }, + }, + }, + }, + }, + want: true, + }, + { + name: "plain workflow_dispatch without item_number", + frontmatter: map[string]any{ + "on": map[string]any{ + "workflow_dispatch": nil, + }, + }, + want: false, + }, + { + name: "workflow_dispatch with unrelated inputs does not match", + frontmatter: map[string]any{ + "on": map[string]any{ + "workflow_dispatch": map[string]any{ + "inputs": map[string]any{ + "branch": map[string]any{"type": "string"}, + }, + }, + }, + }, + want: false, + }, + { + name: "no workflow_dispatch", + frontmatter: map[string]any{ + "on": map[string]any{ + "pull_request": map[string]any{"types": []any{"opened"}}, + }, + }, + want: false, + }, + { + name: "empty frontmatter", + frontmatter: map[string]any{}, + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := extractDispatchItemNumber(tt.frontmatter) + assert.Equal(t, tt.want, got, "extractDispatchItemNumber() mismatch") + }) + } +} + +// TestExtractYAMLSections_HasDispatchItemNumber verifies that extractYAMLSections +// populates WorkflowData.HasDispatchItemNumber from the frontmatter map. +func TestExtractYAMLSections_HasDispatchItemNumber(t *testing.T) { + compiler := NewCompiler() + + t.Run("label trigger shorthand workflow sets HasDispatchItemNumber", func(t *testing.T) { + workflowData := &WorkflowData{} + frontmatter := map[string]any{ + "on": map[string]any{ + "pull_request": map[string]any{"types": []any{"labeled"}}, + "workflow_dispatch": map[string]any{ + "inputs": map[string]any{ + "item_number": map[string]any{ + "description": "The number of the pull request", + "required": true, + "type": "string", + }, + }, + }, + }, + } + compiler.extractYAMLSections(frontmatter, workflowData) + assert.True(t, workflowData.HasDispatchItemNumber, "should detect item_number from label trigger shorthand") + }) + + t.Run("plain workflow does not set HasDispatchItemNumber", func(t *testing.T) { + workflowData := &WorkflowData{} + frontmatter := map[string]any{ + "on": map[string]any{ + "pull_request": map[string]any{"types": []any{"opened"}}, + }, + } + compiler.extractYAMLSections(frontmatter, workflowData) + assert.False(t, workflowData.HasDispatchItemNumber, "should not set HasDispatchItemNumber for plain workflow") + }) +} diff --git a/pkg/workflow/compiler_types.go b/pkg/workflow/compiler_types.go index bf7618b1d0f..9ae4004c5ae 100644 --- a/pkg/workflow/compiler_types.go +++ b/pkg/workflow/compiler_types.go @@ -430,6 +430,7 @@ type WorkflowData struct { HasExplicitGitHubTool bool // true if tools.github was explicitly configured in frontmatter InlinedImports bool // if true, inline all imports at compile time (from inlined-imports frontmatter field) CheckoutConfigs []*CheckoutConfig // user-configured checkout settings from frontmatter + HasDispatchItemNumber bool // true when workflow_dispatch has item_number input (generated by label trigger shorthand) } // BaseSafeOutputConfig holds common configuration fields for all safe output types diff --git a/pkg/workflow/compiler_yaml.go b/pkg/workflow/compiler_yaml.go index b1599186709..ca71eb2b531 100644 --- a/pkg/workflow/compiler_yaml.go +++ b/pkg/workflow/compiler_yaml.go @@ -422,6 +422,12 @@ func (c *Compiler) generatePrompt(yaml *strings.Builder, data *WorkflowData, pre userPromptChunks = append(userPromptChunks, runtimeImportMacro) } + // Enhance entity number expressions with || inputs.item_number fallback when the + // workflow has a workflow_dispatch trigger with item_number (generated by the label + // trigger shorthand). This is applied after all expression mappings (including inline + // mode ones) have been collected so that every entity number reference gets the fallback. + applyWorkflowDispatchFallbacks(expressionMappings, data.HasDispatchItemNumber) + // Generate a single unified prompt creation step WITHOUT known needs expressions // Known needs expressions are added later for the substitution step only // This returns the combined expression mappings for use in the substitution step diff --git a/pkg/workflow/concurrency.go b/pkg/workflow/concurrency.go index 28ae9ee52d7..bad46626e77 100644 --- a/pkg/workflow/concurrency.go +++ b/pkg/workflow/concurrency.go @@ -183,33 +183,78 @@ func isSlashCommandWorkflow(on string) bool { return strings.Contains(on, "slash_command") } +// entityConcurrencyKey builds a ${{ ... }} concurrency-group expression for entity-number +// based workflows. primaryParts are the event-number identifiers (e.g., +// "github.event.pull_request.number"), tailParts are the trailing fallbacks (e.g., +// "github.ref", "github.run_id"). When hasItemNumber is true, "inputs.item_number" is +// inserted between the primary identifiers and the tail, providing a stable per-item +// key for manual workflow_dispatch runs triggered via the label trigger shorthand. +func entityConcurrencyKey(primaryParts []string, tailParts []string, hasItemNumber bool) string { + parts := make([]string, 0, len(primaryParts)+len(tailParts)+1) + parts = append(parts, primaryParts...) + if hasItemNumber { + parts = append(parts, "inputs.item_number") + } + parts = append(parts, tailParts...) + return "${{ " + strings.Join(parts, " || ") + " }}" +} + // buildConcurrencyGroupKeys builds an array of keys for the concurrency group func buildConcurrencyGroupKeys(workflowData *WorkflowData, isCommandTrigger bool) []string { keys := []string{"gh-aw", "${{ github.workflow }}"} + // Whether this workflow exposes inputs.item_number via workflow_dispatch (label trigger shorthand). + // When true, include it in the concurrency key so that manual dispatches for different items + // use distinct groups and don't cancel each other. + hasItemNumber := workflowData.HasDispatchItemNumber + if isCommandTrigger || isSlashCommandWorkflow(workflowData.On) { // For command/slash_command workflows: use issue/PR number; fall back to run_id when // neither is available (e.g. manual workflow_dispatch of the outer workflow). keys = append(keys, "${{ github.event.issue.number || github.event.pull_request.number || github.run_id }}") } else if isPullRequestWorkflow(workflowData.On) && isIssueWorkflow(workflowData.On) { // Mixed workflows with both issue and PR triggers - keys = append(keys, "${{ github.event.issue.number || github.event.pull_request.number || github.run_id }}") + keys = append(keys, entityConcurrencyKey( + []string{"github.event.issue.number", "github.event.pull_request.number"}, + []string{"github.run_id"}, + hasItemNumber, + )) } else if isPullRequestWorkflow(workflowData.On) && isDiscussionWorkflow(workflowData.On) { // Mixed workflows with PR and discussion triggers - keys = append(keys, "${{ github.event.pull_request.number || github.event.discussion.number || github.run_id }}") + keys = append(keys, entityConcurrencyKey( + []string{"github.event.pull_request.number", "github.event.discussion.number"}, + []string{"github.run_id"}, + hasItemNumber, + )) } else if isIssueWorkflow(workflowData.On) && isDiscussionWorkflow(workflowData.On) { // Mixed workflows with issue and discussion triggers - keys = append(keys, "${{ github.event.issue.number || github.event.discussion.number || github.run_id }}") + keys = append(keys, entityConcurrencyKey( + []string{"github.event.issue.number", "github.event.discussion.number"}, + []string{"github.run_id"}, + hasItemNumber, + )) } else if isPullRequestWorkflow(workflowData.On) { // PR workflows: use PR number, fall back to ref then run_id - keys = append(keys, "${{ github.event.pull_request.number || github.ref || github.run_id }}") + keys = append(keys, entityConcurrencyKey( + []string{"github.event.pull_request.number"}, + []string{"github.ref", "github.run_id"}, + hasItemNumber, + )) } else if isIssueWorkflow(workflowData.On) { // Issue workflows: run_id is the fallback when no issue context is available // (e.g. when a mixed-trigger workflow is started via workflow_dispatch). - keys = append(keys, "${{ github.event.issue.number || github.run_id }}") + keys = append(keys, entityConcurrencyKey( + []string{"github.event.issue.number"}, + []string{"github.run_id"}, + hasItemNumber, + )) } else if isDiscussionWorkflow(workflowData.On) { // Discussion workflows: run_id is the fallback when no discussion context is available. - keys = append(keys, "${{ github.event.discussion.number || github.run_id }}") + keys = append(keys, entityConcurrencyKey( + []string{"github.event.discussion.number"}, + []string{"github.run_id"}, + hasItemNumber, + )) } else if isPushWorkflow(workflowData.On) { // Push workflows: use ref to differentiate between branches keys = append(keys, "${{ github.ref || github.run_id }}") diff --git a/pkg/workflow/concurrency_test.go b/pkg/workflow/concurrency_test.go index 7bf3d51eb61..fc034cac000 100644 --- a/pkg/workflow/concurrency_test.go +++ b/pkg/workflow/concurrency_test.go @@ -850,6 +850,60 @@ func TestBuildConcurrencyGroupKeys(t *testing.T) { expected: []string{"gh-aw", "${{ github.workflow }}", "${{ github.event.discussion.number || github.run_id }}"}, description: "Mixed discussion+workflow_dispatch workflows should fall back to run_id when discussion number is unavailable", }, + { + name: "Label trigger shorthand PR workflow should include inputs.item_number fallback", + workflowData: &WorkflowData{ + On: `on: + pull_request: + types: [labeled] + workflow_dispatch: + inputs: + item_number: + description: The number of the pull request + required: true + type: string`, + HasDispatchItemNumber: true, + }, + isAliasTrigger: false, + expected: []string{"gh-aw", "${{ github.workflow }}", "${{ github.event.pull_request.number || inputs.item_number || github.ref || github.run_id }}"}, + description: "Label trigger shorthand PR workflows should include inputs.item_number before ref fallback", + }, + { + name: "Label trigger shorthand issue workflow should include inputs.item_number fallback", + workflowData: &WorkflowData{ + On: `on: + issues: + types: [labeled] + workflow_dispatch: + inputs: + item_number: + description: The number of the issue + required: true + type: string`, + HasDispatchItemNumber: true, + }, + isAliasTrigger: false, + expected: []string{"gh-aw", "${{ github.workflow }}", "${{ github.event.issue.number || inputs.item_number || github.run_id }}"}, + description: "Label trigger shorthand issue workflows should include inputs.item_number fallback", + }, + { + name: "Label trigger shorthand discussion workflow should include inputs.item_number fallback", + workflowData: &WorkflowData{ + On: `on: + discussion: + types: [labeled] + workflow_dispatch: + inputs: + item_number: + description: The number of the discussion + required: true + type: string`, + HasDispatchItemNumber: true, + }, + isAliasTrigger: false, + expected: []string{"gh-aw", "${{ github.workflow }}", "${{ github.event.discussion.number || inputs.item_number || github.run_id }}"}, + description: "Label trigger shorthand discussion workflows should include inputs.item_number fallback", + }, } for _, tt := range tests { diff --git a/pkg/workflow/expression_extraction.go b/pkg/workflow/expression_extraction.go index bf9e9ea6803..f685b4dfa16 100644 --- a/pkg/workflow/expression_extraction.go +++ b/pkg/workflow/expression_extraction.go @@ -253,6 +253,39 @@ func (e *ExpressionExtractor) GetMappings() []*ExpressionMapping { // awInputsExprRegex matches ${{ github.aw.inputs. }} expressions var awInputsExprRegex = regexp.MustCompile(`\$\{\{\s*github\.aw\.inputs\.([a-zA-Z0-9_-]+)\s*\}\}`) +// applyWorkflowDispatchFallbacks enhances entity number expressions with an +// "|| inputs.item_number" fallback when the workflow has a workflow_dispatch +// trigger that includes the item_number input (generated by the label trigger +// shorthand). Without this fallback, manually dispatched runs receive an empty +// entity number because the event payload is absent. +// +// Only the three canonical entity number paths are patched: +// +// github.event.pull_request.number → github.event.pull_request.number || inputs.item_number +// github.event.issue.number → github.event.issue.number || inputs.item_number +// github.event.discussion.number → github.event.discussion.number || inputs.item_number +// +// The EnvVar field is intentionally left unchanged so that callers that already +// hold a reference to an env-var name continue to work. +func applyWorkflowDispatchFallbacks(mappings []*ExpressionMapping, hasItemNumber bool) { + if !hasItemNumber { + return + } + + fallbacks := map[string]string{ + "github.event.pull_request.number": "github.event.pull_request.number || inputs.item_number", + "github.event.issue.number": "github.event.issue.number || inputs.item_number", + "github.event.discussion.number": "github.event.discussion.number || inputs.item_number", + } + + for _, mapping := range mappings { + if enhanced, ok := fallbacks[mapping.Content]; ok { + expressionExtractionLog.Printf("Applying workflow_dispatch fallback: %s -> %s", mapping.Content, enhanced) + mapping.Content = enhanced + } + } +} + // SubstituteImportInputs replaces ${{ github.aw.inputs. }} expressions // with the corresponding values from the importInputs map. // This is called before expression extraction to inject import input values. diff --git a/pkg/workflow/expression_extraction_test.go b/pkg/workflow/expression_extraction_test.go index 1322dd0dd7b..a3a62c7915c 100644 --- a/pkg/workflow/expression_extraction_test.go +++ b/pkg/workflow/expression_extraction_test.go @@ -462,3 +462,96 @@ Other: ${{ needs.activation.outputs.comment_id }} }) } } + +func TestApplyWorkflowDispatchFallbacks(t *testing.T) { + tests := []struct { + name string + hasItemNumber bool + inputMappings []*ExpressionMapping + wantContents map[string]string // envVar -> expected Content after applying fallbacks + }{ + { + name: "PR number expression gets fallback when hasItemNumber is true", + hasItemNumber: true, + inputMappings: []*ExpressionMapping{ + {Original: "${{ github.event.pull_request.number }}", EnvVar: "GH_AW_GITHUB_EVENT_PULL_REQUEST_NUMBER", Content: "github.event.pull_request.number"}, + }, + wantContents: map[string]string{ + "GH_AW_GITHUB_EVENT_PULL_REQUEST_NUMBER": "github.event.pull_request.number || inputs.item_number", + }, + }, + { + name: "issue number expression gets fallback", + hasItemNumber: true, + inputMappings: []*ExpressionMapping{ + {Original: "${{ github.event.issue.number }}", EnvVar: "GH_AW_GITHUB_EVENT_ISSUE_NUMBER", Content: "github.event.issue.number"}, + }, + wantContents: map[string]string{ + "GH_AW_GITHUB_EVENT_ISSUE_NUMBER": "github.event.issue.number || inputs.item_number", + }, + }, + { + name: "discussion number expression gets fallback", + hasItemNumber: true, + inputMappings: []*ExpressionMapping{ + {Original: "${{ github.event.discussion.number }}", EnvVar: "GH_AW_GITHUB_EVENT_DISCUSSION_NUMBER", Content: "github.event.discussion.number"}, + }, + wantContents: map[string]string{ + "GH_AW_GITHUB_EVENT_DISCUSSION_NUMBER": "github.event.discussion.number || inputs.item_number", + }, + }, + { + name: "no fallback applied when hasItemNumber is false", + hasItemNumber: false, + inputMappings: []*ExpressionMapping{ + {Original: "${{ github.event.pull_request.number }}", EnvVar: "GH_AW_GITHUB_EVENT_PULL_REQUEST_NUMBER", Content: "github.event.pull_request.number"}, + }, + wantContents: map[string]string{ + "GH_AW_GITHUB_EVENT_PULL_REQUEST_NUMBER": "github.event.pull_request.number", + }, + }, + { + name: "unrelated expressions are not modified", + hasItemNumber: true, + inputMappings: []*ExpressionMapping{ + {Original: "${{ github.repository }}", EnvVar: "GH_AW_GITHUB_REPOSITORY", Content: "github.repository"}, + {Original: "${{ github.event.pull_request.number }}", EnvVar: "GH_AW_GITHUB_EVENT_PULL_REQUEST_NUMBER", Content: "github.event.pull_request.number"}, + }, + wantContents: map[string]string{ + "GH_AW_GITHUB_REPOSITORY": "github.repository", + "GH_AW_GITHUB_EVENT_PULL_REQUEST_NUMBER": "github.event.pull_request.number || inputs.item_number", + }, + }, + { + name: "EnvVar name is preserved after fallback is applied", + hasItemNumber: true, + inputMappings: []*ExpressionMapping{ + {Original: "${{ github.event.pull_request.number }}", EnvVar: "GH_AW_GITHUB_EVENT_PULL_REQUEST_NUMBER", Content: "github.event.pull_request.number"}, + }, + wantContents: map[string]string{ + "GH_AW_GITHUB_EVENT_PULL_REQUEST_NUMBER": "github.event.pull_request.number || inputs.item_number", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + applyWorkflowDispatchFallbacks(tt.inputMappings, tt.hasItemNumber) + + for _, mapping := range tt.inputMappings { + wantContent, ok := tt.wantContents[mapping.EnvVar] + if !ok { + t.Errorf("unexpected mapping with EnvVar %q", mapping.EnvVar) + continue + } + if mapping.Content != wantContent { + t.Errorf("mapping %q Content = %q, want %q", mapping.EnvVar, mapping.Content, wantContent) + } + // Verify the EnvVar name itself was not changed by the fallback + if !strings.HasPrefix(mapping.EnvVar, "GH_AW_") { + t.Errorf("mapping EnvVar %q lost GH_AW_ prefix after fallback", mapping.EnvVar) + } + } + }) + } +} diff --git a/pkg/workflow/schedule_preprocessing_test.go b/pkg/workflow/schedule_preprocessing_test.go index 0e8128f66dc..8b3ff510cfc 100644 --- a/pkg/workflow/schedule_preprocessing_test.go +++ b/pkg/workflow/schedule_preprocessing_test.go @@ -1553,3 +1553,186 @@ func TestFuzzyScheduleDevModeDifferentFromReleaseMode(t *testing.T) { t.Logf("Dev mode result: %s", devResult) t.Logf("Release mode result: %s", releaseResult) } + +// TestLabelTriggerShorthandPreprocessing verifies that preprocessScheduleFields correctly +// expands label trigger shorthands (e.g. "on: pull_request labeled my-label") into a +// structured frontmatter with workflow_dispatch.inputs.item_number. These tests cover +// the parsing side of the HasDispatchItemNumber feature so that extractDispatchItemNumber +// can reliably detect the presence of item_number from the in-memory map. +func TestLabelTriggerShorthandPreprocessing(t *testing.T) { + tests := []struct { + name string + onValue string + wantTriggerKey string // e.g. "pull_request", "issues", "discussion" + wantLabelNames []string // label names expected in trigger config + wantEntitySubstr string // substring expected in item_number description + wantItemNumber bool // whether extractDispatchItemNumber should return true + }{ + { + name: "pull_request labeled single label", + onValue: "pull_request labeled needs-review", + wantTriggerKey: "pull_request", + wantLabelNames: []string{"needs-review"}, + wantEntitySubstr: "pull request", + wantItemNumber: true, + }, + { + name: "pull_request labeled multiple labels", + onValue: "pull_request labeled bug fix", + wantTriggerKey: "pull_request", + wantLabelNames: []string{"bug", "fix"}, + wantEntitySubstr: "pull request", + wantItemNumber: true, + }, + { + name: "issue labeled single label", + onValue: "issue labeled bug", + wantTriggerKey: "issues", + wantLabelNames: []string{"bug"}, + wantEntitySubstr: "issue", + wantItemNumber: true, + }, + { + name: "discussion labeled single label", + onValue: "discussion labeled question", + wantTriggerKey: "discussion", + wantLabelNames: []string{"question"}, + wantEntitySubstr: "discussion", + wantItemNumber: true, + }, + { + name: "pull-request (hyphen) labeled single label", + onValue: "pull-request labeled needs-review", + wantTriggerKey: "pull_request", + wantLabelNames: []string{"needs-review"}, + wantEntitySubstr: "pull request", + wantItemNumber: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + frontmatter := map[string]any{"on": tt.onValue} + compiler := NewCompiler() + compiler.SetWorkflowIdentifier("test-workflow.md") + + err := compiler.preprocessScheduleFields(frontmatter, "", "") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // The "on" field should now be a map + onValue, exists := frontmatter["on"] + if !exists { + t.Fatal("expected 'on' field to exist after preprocessing") + } + onMap, ok := onValue.(map[string]any) + if !ok { + t.Fatalf("expected 'on' to be a map after preprocessing, got %T", onValue) + } + + // Trigger key (e.g. pull_request, issues, discussion) must be present + triggerVal, hasTrigger := onMap[tt.wantTriggerKey] + if !hasTrigger { + t.Errorf("expected trigger key %q in 'on' map, got keys: %v", tt.wantTriggerKey, onMap) + } else { + triggerMap, ok := triggerVal.(map[string]any) + if !ok { + t.Errorf("expected trigger %q to be a map, got %T", tt.wantTriggerKey, triggerVal) + } else { + // types: [labeled] + types, _ := triggerMap["types"].([]any) + if len(types) != 1 || types[0] != "labeled" { + t.Errorf("expected types=[labeled], got %v", types) + } + // names must match the expected labels + names, _ := triggerMap["names"].([]string) + if !slicesEqual(names, tt.wantLabelNames) { + t.Errorf("expected names %v, got %v", tt.wantLabelNames, names) + } + } + } + + // workflow_dispatch must be present with inputs.item_number + wdVal, hasWD := onMap["workflow_dispatch"] + if !hasWD { + t.Fatal("expected workflow_dispatch in 'on' map") + } + wdMap, ok := wdVal.(map[string]any) + if !ok { + t.Fatalf("expected workflow_dispatch to be a map, got %T", wdVal) + } + inputsVal, hasInputs := wdMap["inputs"] + if !hasInputs { + t.Fatal("expected inputs in workflow_dispatch") + } + inputsMap, ok := inputsVal.(map[string]any) + if !ok { + t.Fatalf("expected inputs to be a map, got %T", inputsVal) + } + itemNumberVal, hasItemNumber := inputsMap["item_number"] + if !hasItemNumber { + t.Fatal("expected item_number in workflow_dispatch.inputs") + } + itemNumberMap, ok := itemNumberVal.(map[string]any) + if !ok { + t.Fatalf("expected item_number to be a map, got %T", itemNumberVal) + } + desc, _ := itemNumberMap["description"].(string) + if !strings.Contains(desc, tt.wantEntitySubstr) { + t.Errorf("expected item_number description to contain %q, got %q", tt.wantEntitySubstr, desc) + } + + // Full pipeline: extractDispatchItemNumber must return true + got := extractDispatchItemNumber(frontmatter) + if got != tt.wantItemNumber { + t.Errorf("extractDispatchItemNumber() = %v, want %v", got, tt.wantItemNumber) + } + }) + } +} + +// TestLabelTriggerShorthandPreprocessingErrors verifies that invalid label trigger +// shorthands produce the expected errors, and that incomplete patterns fall through +// without being treated as label triggers. +func TestLabelTriggerShorthandPreprocessingErrors(t *testing.T) { + t.Run("pull_request labeled with only empty labels (comma-only)", func(t *testing.T) { + // A comma-only token is parsed as empty labels → error + frontmatter := map[string]any{"on": "pull_request labeled ,"} + compiler := NewCompiler() + err := compiler.preprocessScheduleFields(frontmatter, "", "") + if err == nil { + t.Error("expected an error for label trigger with empty label names, got nil") + return + } + if !strings.Contains(err.Error(), "label trigger shorthand requires at least one label name") { + t.Errorf("unexpected error: %v", err) + } + }) + + t.Run("pull_request labeled with no labels (2 tokens - parsed as PR activity trigger, no item_number)", func(t *testing.T) { + // "pull_request labeled" has only 2 tokens so it doesn't match the + // label trigger shorthand (which needs >= 3). It is parsed instead as a + // plain pull_request activity-type trigger (types: [labeled]) with a bare + // workflow_dispatch (no inputs). extractDispatchItemNumber must return false. + frontmatter := map[string]any{"on": "pull_request labeled"} + compiler := NewCompiler() + err := compiler.preprocessScheduleFields(frontmatter, "", "") + if err != nil { + t.Errorf("unexpected error: %v", err) + return + } + onMap, ok := frontmatter["on"].(map[string]any) + if !ok { + t.Fatalf("expected 'on' to be a map, got %T", frontmatter["on"]) + } + // workflow_dispatch should exist but have no item_number input + if _, hasWD := onMap["workflow_dispatch"]; !hasWD { + t.Error("expected workflow_dispatch in 'on' map") + } + // extractDispatchItemNumber must be false — no item_number input was added + if extractDispatchItemNumber(frontmatter) { + t.Error("extractDispatchItemNumber() should be false: workflow_dispatch has no item_number inputs") + } + }) +}