Skip to content

Refactor dispatch/call workflow duplication with shared input, tool, and resolver helpers #33947

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pelikhan merged 4 commits into from
May 22, 2026
Merged

Refactor dispatch/call workflow duplication with shared input, tool, and resolver helpers #33947

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6693776
Initial plan
Copilot May 22, 2026
cbfa36d
Plan: refactor dispatch/call workflow duplication
Copilot May 22, 2026
c253a72
Refactor shared dispatch/call workflow helpers
Copilot May 22, 2026
96250ac
Fix workflow helper test imports
Copilot May 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
137 changes: 92 additions & 45 deletions .github/workflows/dataflow-pr-discussion-dataset.lock.yml
View file
Open in desktop

Large diffs are not rendered by default.

67 changes: 3 additions & 64 deletions pkg/workflow/call_workflow_validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,80 +182,19 @@ func (c *Compiler) validateCallWorkflow(data *WorkflowData, workflowPath string)
// extractWorkflowCallInputs parses a workflow file and extracts the workflow_call inputs schema.
// Returns a map of input definitions that can be used to generate MCP tool schemas.
func extractWorkflowCallInputs(workflowPath string) (map[string]any, error) {
workflow, err := readWorkflowYAML(workflowPath)
if err != nil {
return nil, err
}

return extractWorkflowCallInputsFromParsed(workflow), nil
return extractInputsFromYAML(workflowPath, "workflow_call")
}

// extractMDWorkflowCallInputs reads a .md workflow file's frontmatter and extracts
// the workflow_call inputs schema, mirroring extractWorkflowCallInputs for .md sources.
func extractMDWorkflowCallInputs(mdPath string) (map[string]any, error) {
content, err := os.ReadFile(mdPath) // #nosec G304 -- mdPath is validated via isPathWithinDir in findWorkflowFile
if err != nil {
return nil, err
}
result, err := parser.ExtractFrontmatterFromContent(string(content))
if err != nil || result == nil {
return make(map[string]any), nil
}
onSection, hasOn := result.Frontmatter["on"]
if !hasOn {
return make(map[string]any), nil
}
onMap, ok := onSection.(map[string]any)
if !ok {
return make(map[string]any), nil
}
workflowCall, hasWorkflowCall := onMap["workflow_call"]
if !hasWorkflowCall {
return make(map[string]any), nil
}
workflowCallMap, ok := workflowCall.(map[string]any)
if !ok {
return make(map[string]any), nil
}
inputs, hasInputs := workflowCallMap["inputs"]
if !hasInputs {
return make(map[string]any), nil
}
inputsMap, ok := inputs.(map[string]any)
if !ok {
return make(map[string]any), nil
}
return inputsMap, nil
return extractInputsFromMarkdown(mdPath, "workflow_call")
}

// extractWorkflowCallInputsFromParsed extracts workflow_call inputs from an already-parsed
// workflow map (used for both .lock.yml and .yml files).
func extractWorkflowCallInputsFromParsed(workflow map[string]any) map[string]any {
onSection, hasOn := workflow["on"]
if !hasOn {
return make(map[string]any)
}
onMap, ok := onSection.(map[string]any)
if !ok {
return make(map[string]any)
}
workflowCall, hasWorkflowCall := onMap["workflow_call"]
if !hasWorkflowCall {
return make(map[string]any)
}
workflowCallMap, ok := workflowCall.(map[string]any)
if !ok {
return make(map[string]any)
}
inputs, hasInputs := workflowCallMap["inputs"]
if !hasInputs {
return make(map[string]any)
}
inputsMap, ok := inputs.(map[string]any)
if !ok {
return make(map[string]any)
}
return inputsMap
return extractInputsFromParsedWorkflow(workflow, "workflow_call")
}

// mdHasWorkflowCall reads a .md workflow file's frontmatter and reports whether
Expand Down
37 changes: 3 additions & 34 deletions pkg/workflow/dispatch_workflow_file_resolver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,41 +119,10 @@ func mdHasWorkflowDispatch(mdPath string) (bool, error) {
// the workflow_dispatch inputs schema, mirroring extractWorkflowDispatchInputs for .md sources.
func extractMDWorkflowDispatchInputs(mdPath string) (map[string]any, error) {
dispatchWorkflowValidationLog.Printf("Extracting workflow_dispatch inputs from: %s", mdPath)
content, err := os.ReadFile(mdPath) // #nosec G304 -- mdPath is validated via isPathWithinDir in findWorkflowFile
inputs, err := extractInputsFromMarkdown(mdPath, "workflow_dispatch")
if err != nil {
return nil, err
}
result, err := parser.ExtractFrontmatterFromContent(string(content))
if err != nil || result == nil {
return make(map[string]any), nil
}
onSection, hasOn := result.Frontmatter["on"]
if !hasOn {
dispatchWorkflowValidationLog.Printf("No 'on' section found in: %s", mdPath)
return make(map[string]any), nil
}
onMap, ok := onSection.(map[string]any)
if !ok {
return make(map[string]any), nil
}
workflowDispatch, hasWorkflowDispatch := onMap["workflow_dispatch"]
if !hasWorkflowDispatch {
dispatchWorkflowValidationLog.Printf("No workflow_dispatch trigger in: %s", mdPath)
return make(map[string]any), nil
}
workflowDispatchMap, ok := workflowDispatch.(map[string]any)
if !ok {
return make(map[string]any), nil
}
inputs, hasInputs := workflowDispatchMap["inputs"]
if !hasInputs {
dispatchWorkflowValidationLog.Printf("No inputs defined in workflow_dispatch for: %s", mdPath)
return make(map[string]any), nil
}
inputsMap, ok := inputs.(map[string]any)
if !ok {
return make(map[string]any), nil
}
dispatchWorkflowValidationLog.Printf("Extracted %d workflow_dispatch input(s) from: %s", len(inputsMap), mdPath)
return inputsMap, nil
dispatchWorkflowValidationLog.Printf("Extracted %d workflow_dispatch input(s) from: %s", len(inputs), mdPath)
return inputs, nil
}
31 changes: 1 addition & 30 deletions pkg/workflow/dispatch_workflow_validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,36 +191,7 @@ func parseRepoSlugLiteral(slug string) (string, string, bool) {
// Returns a map of input definitions that can be used to generate MCP tool schemas
func extractWorkflowDispatchInputs(workflowPath string) (map[string]any, error) {
dispatchWorkflowValidationLog.Printf("Extracting workflow_dispatch inputs from: %s", workflowPath)
workflow, err := readWorkflowYAML(workflowPath)
if err != nil {
return nil, err
}

onSection, hasOn := workflow["on"]
if !hasOn {
return make(map[string]any), nil
}
onMap, ok := onSection.(map[string]any)
if !ok {
return make(map[string]any), nil
}
workflowDispatch, hasWorkflowDispatch := onMap["workflow_dispatch"]
if !hasWorkflowDispatch {
return make(map[string]any), nil
}
workflowDispatchMap, ok := workflowDispatch.(map[string]any)
if !ok {
return make(map[string]any), nil
}
inputs, hasInputs := workflowDispatchMap["inputs"]
if !hasInputs {
return make(map[string]any), nil
}
inputsMap, ok := inputs.(map[string]any)
if !ok {
return make(map[string]any), nil
}
return inputsMap, nil
return extractInputsFromYAML(workflowPath, "workflow_dispatch")
}

// containsWorkflowDispatch reports whether the given 'on:' section value includes
Expand Down
51 changes: 13 additions & 38 deletions pkg/workflow/safe_outputs_call_workflow.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,8 @@ package workflow

import (
"fmt"
"sort"

"github.com/github/gh-aw/pkg/logger"
"github.com/github/gh-aw/pkg/stringutil"
)

var safeOutputsCallWorkflowLog = logger.New("workflow:safe_outputs_call_workflow")
Expand Down Expand Up @@ -47,15 +45,8 @@ func populateCallWorkflowFiles(data *WorkflowData, markdownPath string) {
}

// Determine which file to use - priority: .lock.yml > .yml > .md (batch target)
var extension string
if fileResult.lockExists {
extension = ".lock.yml"
} else if fileResult.ymlExists {
extension = ".yml"
} else if fileResult.mdExists {
// .md-only: the workflow is a same-batch compilation target that will produce a .lock.yml
extension = ".lock.yml"
} else {
extension, found := resolveWorkflowExtension(fileResult)
if !found {
callWorkflowLog.Printf("Warning: no workflow file found for %s (checked .lock.yml, .yml, .md)", workflowName)
continue
}
Expand All @@ -74,35 +65,19 @@ func populateCallWorkflowFiles(data *WorkflowData, markdownPath string) {
// call_workflow_name and call_workflow_payload outputs for the conditional `uses:` jobs.
func generateCallWorkflowTool(workflowName string, workflowInputs map[string]any) map[string]any {
safeOutputsCallWorkflowLog.Printf("Generating call-workflow tool: workflow=%s, inputs=%d", workflowName, len(workflowInputs))

// Normalize workflow name to use underscores for tool name
toolName := stringutil.NormalizeSafeOutputIdentifier(workflowName)

// Build the description
description := fmt.Sprintf("Call the '%s' reusable workflow via workflow_call. This workflow must support workflow_call and be in .github/workflows/ directory in the same repository.", workflowName)

// Build input schema properties from workflow_call inputs
properties, required := buildInputSchema(workflowInputs, func(inputName string) string {
return fmt.Sprintf("Input parameter '%s' for workflow %s", inputName, workflowName)
tool := generateWorkflowToolDefinition(workflowToolDefinitionOptions{
workflowName: workflowName,
workflowInputs: workflowInputs,
descriptionFormat: "Call the '%s' reusable workflow via workflow_call. This workflow must support workflow_call and be in .github/workflows/ directory in the same repository.",
metadataKey: "_call_workflow_name",
})

// Build the complete tool definition
tool := map[string]any{
"name": toolName,
"description": description,
"_call_workflow_name": workflowName, // Internal metadata for handler routing
"inputSchema": map[string]any{
"type": "object",
"properties": properties,
"additionalProperties": false,
},
inputSchema, _ := tool["inputSchema"].(map[string]any)
properties, _ := inputSchema["properties"].(map[string]any)
requiredCount := 0
if required, ok := inputSchema["required"].([]string); ok {
requiredCount = len(required)
}

if len(required) > 0 {
sort.Strings(required)
tool["inputSchema"].(map[string]any)["required"] = required
}

safeOutputsCallWorkflowLog.Printf("Generated call-workflow tool: name=%s, properties=%d, required=%d", toolName, len(properties), len(required))
safeOutputsCallWorkflowLog.Printf("Generated call-workflow tool: name=%s, properties=%d, required=%d", tool["name"], len(properties), requiredCount)
return tool
}
56 changes: 13 additions & 43 deletions pkg/workflow/safe_outputs_dispatch.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,7 @@
package workflow

import (
"fmt"
"sort"

"github.com/github/gh-aw/pkg/logger"
"github.com/github/gh-aw/pkg/stringutil"
)

var safeOutputsDispatchWorkflowLog = logger.New("workflow:safe_outputs_dispatch")
Expand Down Expand Up @@ -51,15 +47,8 @@ func populateDispatchWorkflowFiles(data *WorkflowData, markdownPath string) {
}

// Determine which file to use - priority: .lock.yml > .yml > .md (batch target)
var extension string
if fileResult.lockExists {
extension = ".lock.yml"
} else if fileResult.ymlExists {
extension = ".yml"
} else if fileResult.mdExists {
// .md-only: the workflow is a same-batch compilation target that will produce a .lock.yml
extension = ".lock.yml"
} else {
extension, found := resolveWorkflowExtension(fileResult)
if !found {
safeOutputsConfigLog.Printf("Warning: no workflow file found for %s (checked .lock.yml, .yml, .md)", workflowName)
continue
}
Expand Down Expand Up @@ -107,38 +96,19 @@ func workflowHasAwContextInput(fileResult *findWorkflowFileResult, workflowName
// the workflow's defined workflow_dispatch inputs as parameters.
func generateDispatchWorkflowTool(workflowName string, workflowInputs map[string]any) map[string]any {
safeOutputsDispatchWorkflowLog.Printf("Generating dispatch-workflow tool: workflow=%s, inputs=%d", workflowName, len(workflowInputs))

// Normalize workflow name to use underscores for tool name
toolName := stringutil.NormalizeSafeOutputIdentifier(workflowName)

// Build the description
description := fmt.Sprintf("Dispatch the '%s' workflow with workflow_dispatch trigger. This workflow must support workflow_dispatch and be in .github/workflows/ directory in the same repository.", workflowName)

// Build input schema properties from workflow_dispatch inputs
properties, required := buildInputSchema(workflowInputs, func(inputName string) string {
return fmt.Sprintf("Input parameter '%s' for workflow %s", inputName, workflowName)
tool := generateWorkflowToolDefinition(workflowToolDefinitionOptions{
workflowName: workflowName,
workflowInputs: workflowInputs,
descriptionFormat: "Dispatch the '%s' workflow with workflow_dispatch trigger. This workflow must support workflow_dispatch and be in .github/workflows/ directory in the same repository.",
metadataKey: "_workflow_name",
})

// Add internal workflow_name parameter (hidden from description but used internally)
// This will be injected by the safe output handler

// Build the complete tool definition
tool := map[string]any{
"name": toolName,
"description": description,
"_workflow_name": workflowName, // Internal metadata for handler routing
"inputSchema": map[string]any{
"type": "object",
"properties": properties,
"additionalProperties": false,
},
}

if len(required) > 0 {
sort.Strings(required)
tool["inputSchema"].(map[string]any)["required"] = required
inputSchema, _ := tool["inputSchema"].(map[string]any)
properties, _ := inputSchema["properties"].(map[string]any)
requiredCount := 0
if required, ok := inputSchema["required"].([]string); ok {
requiredCount = len(required)
}

safeOutputsDispatchWorkflowLog.Printf("Generated dispatch-workflow tool: name=%s, properties=%d, required=%d", toolName, len(properties), len(required))
safeOutputsDispatchWorkflowLog.Printf("Generated dispatch-workflow tool: name=%s, properties=%d, required=%d", tool["name"], len(properties), requiredCount)
return tool
}
56 changes: 56 additions & 0 deletions pkg/workflow/safe_outputs_workflow_helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
package workflow

import (
"fmt"
"sort"

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/improve-codebase-architecture] Type name lacks domain context — workflowToolDefinitionOptions is too generic.

💡 Suggested naming

Consider one of these domain-specific names:

// Option 1: Emphasize the MCP tool aspect
type mcpWorkflowToolOptions struct {

// Option 2: Emphasize the safe-output aspect
type safeOutputToolOptions struct {

// Option 3: Be explicit about both
type workflowMCPToolConfig struct {

The current name "options" suggests configuration, but this is actually the definition of an MCP tool for workflow triggers. Using "options" for a struct that produces a tool schema can be confusing — consider Config or drop the suffix entirely.

"github.com/github/gh-aw/pkg/stringutil"
)

type workflowToolDefinitionOptions struct {
workflowName string
workflowInputs map[string]any
descriptionFormat string
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/zoom-out] Exported function lacks documentation.

💡 Suggested documentation 
// generateWorkflowToolDefinition creates an MCP tool definition for workflow
// dispatch or call operations. The returned tool schema includes normalized
// naming, input parameter validation, and metadata for handler routing.
//
// The metadataKey parameter determines which internal field is used for routing
// (e.g., "_workflow_name" for dispatch, "_call_workflow_name" for call).
func generateWorkflowToolDefinition(opts workflowToolDefinitionOptions) map[string]any {

This function is central to the refactoring — documenting its purpose and the significance of metadataKey helps future maintainers understand why dispatch and call use different keys.

metadataKey string
}

func generateWorkflowToolDefinition(opts workflowToolDefinitionOptions) map[string]any {
toolName := stringutil.NormalizeSafeOutputIdentifier(opts.workflowName)
description := fmt.Sprintf(opts.descriptionFormat, opts.workflowName)
properties, required := buildInputSchema(opts.workflowInputs, func(inputName string) string {
return fmt.Sprintf("Input parameter '%s' for workflow %s", inputName, opts.workflowName)
})

tool := map[string]any{
"name": toolName,
"description": description,
opts.metadataKey: opts.workflowName,
"inputSchema": map[string]any{
"type": "object",
"properties": properties,
"additionalProperties": false,
},
}

if len(required) > 0 {
sort.Strings(required)
tool["inputSchema"].(map[string]any)["required"] = required
}

return tool
}

func resolveWorkflowExtension(fileResult *findWorkflowFileResult) (string, bool) {
if fileResult.lockExists {
return ".lock.yml", true
}
if fileResult.ymlExists {
return ".yml", true
}
if fileResult.mdExists {
// .md-only: the workflow is a same-batch compilation target that will produce a .lock.yml
return ".lock.yml", true
}

return "", false
}
Loading
Loading