Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion .github/workflows/smoke-call-workflow.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions actions/setup/js/safe_output_handler_manager.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ const HANDLER_MAP = {
create_code_scanning_alert: "./create_code_scanning_alert.cjs",
autofix_code_scanning_alert: "./autofix_code_scanning_alert.cjs",
dispatch_workflow: "./dispatch_workflow.cjs",
call_workflow: "./call_workflow.cjs",
create_missing_tool_issue: "./create_missing_tool_issue.cjs",
missing_tool: "./missing_tool.cjs",
create_missing_data_issue: "./create_missing_data_issue.cjs",
Expand Down
32 changes: 32 additions & 0 deletions actions/setup/js/safe_output_handler_manager.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -1108,4 +1108,36 @@ describe("Safe Output Handler Manager", () => {
expect(global.core.setOutput).toHaveBeenCalledWith("created_issue_url", "https://github.com/owner/repo/issues/7");
});
});

describe("call_workflow handler registration", () => {
it("processes call_workflow messages without no-handler warnings when handler is registered", async () => {
const messages = [{ type: "call_workflow", workflow_name: "worker-a" }];
const mockHandler = vi.fn().mockResolvedValue({
success: true,
workflow_name: "worker-a",
payload: "{}",
});
const handlers = new Map([["call_workflow", mockHandler]]);

const result = await processMessages(handlers, messages);

expect(result.results).toHaveLength(1);
expect(result.results[0].type).toBe("call_workflow");
// Handler was invoked, so no "no handler loaded" error
expect(result.results[0].error).toBeUndefined();
expect(mockHandler).toHaveBeenCalledTimes(1);
});

it("records no-handler error for call_workflow when handler map is missing entry", async () => {
const messages = [{ type: "call_workflow", workflow_name: "worker-a" }];
// Empty handler map - simulates the bug where call_workflow was not in HANDLER_MAP
const handlers = new Map();

const result = await processMessages(handlers, messages);

expect(result.results).toHaveLength(1);
expect(result.results[0].success).toBe(false);
expect(result.results[0].error).toContain("No handler loaded for type 'call_workflow'");
});
});
});
16 changes: 16 additions & 0 deletions pkg/workflow/compiler_safe_outputs_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -586,6 +586,22 @@ var handlerRegistry = map[string]handlerBuilder{
builder.AddIfNotEmpty("github-token", c.GitHubToken)
return builder.Build()
},
"call_workflow": func(cfg *SafeOutputsConfig) map[string]any {
if cfg.CallWorkflow == nil {
return nil
}
c := cfg.CallWorkflow
builder := newHandlerConfigBuilder().
AddTemplatableInt("max", c.Max).
AddStringSlice("workflows", c.Workflows)

// Add workflow_files map if it has entries
if len(c.WorkflowFiles) > 0 {
builder.AddDefault("workflow_files", c.WorkflowFiles)
}

return builder.Build()
},
"missing_tool": func(cfg *SafeOutputsConfig) map[string]any {
if cfg.MissingTool == nil {
return nil
Expand Down
86 changes: 86 additions & 0 deletions pkg/workflow/compiler_safe_outputs_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,23 @@ func TestAddHandlerManagerConfigEnvVar(t *testing.T) {
checkJSON: true,
expectedKeys: []string{"create_issue"},
},
{
name: "call_workflow config",
safeOutputs: &SafeOutputsConfig{
CallWorkflow: &CallWorkflowConfig{
BaseSafeOutputConfig: BaseSafeOutputConfig{
Max: strPtr("1"),
},
Workflows: []string{"worker-a", "worker-b"},
WorkflowFiles: map[string]string{"worker-a": "./.github/workflows/worker-a.lock.yml", "worker-b": "./.github/workflows/worker-b.lock.yml"},
},
},
checkContains: []string{
"GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG",
},
checkJSON: true,
expectedKeys: []string{"call_workflow"},
},
}

for _, tt := range tests {
Expand Down Expand Up @@ -1297,3 +1314,72 @@ func TestHandlerConfigStagedMode(t *testing.T) {
})
}
}

// TestAddHandlerManagerConfigEnvVar_CallWorkflow asserts that GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG
// contains call_workflow, workflows, and workflow_files when SafeOutputs.CallWorkflow is configured.
func TestAddHandlerManagerConfigEnvVar_CallWorkflow(t *testing.T) {
compiler := NewCompiler()

workflowData := &WorkflowData{
Name: "Test Workflow",
SafeOutputs: &SafeOutputsConfig{
CallWorkflow: &CallWorkflowConfig{
BaseSafeOutputConfig: BaseSafeOutputConfig{
Max: strPtr("1"),
},
Workflows: []string{"worker-a", "worker-b"},
WorkflowFiles: map[string]string{"worker-a": "./.github/workflows/worker-a.lock.yml", "worker-b": "./.github/workflows/worker-b.lock.yml"},
},
},
}

var steps []string
compiler.addHandlerManagerConfigEnvVar(&steps, workflowData)

require.NotEmpty(t, steps, "Steps should not be empty")

var callWorkflowConfig map[string]any
for _, step := range steps {
if strings.Contains(step, "GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG") {
parts := strings.Split(step, "GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG: ")
require.Len(t, parts, 2, "Should have two parts")

jsonStr := strings.TrimSpace(parts[1])
jsonStr = strings.Trim(jsonStr, "\"")
jsonStr = strings.ReplaceAll(jsonStr, "\\\"", "\"")

var config map[string]map[string]any
err := json.Unmarshal([]byte(jsonStr), &config)
require.NoError(t, err, "Handler config JSON should be valid")

cfg, ok := config["call_workflow"]
require.True(t, ok, "GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG should contain 'call_workflow' key")
callWorkflowConfig = cfg
break
}
}

require.NotNil(t, callWorkflowConfig, "call_workflow config should be present")

// Verify max
maxVal, ok := callWorkflowConfig["max"]
require.True(t, ok, "call_workflow config should have 'max' field")
assert.InDelta(t, float64(1), maxVal, 0.0001, "max should be 1")

// Verify workflows list
workflowsVal, ok := callWorkflowConfig["workflows"]
require.True(t, ok, "call_workflow config should have 'workflows' field")
workflowsSlice, ok := workflowsVal.([]any)
require.True(t, ok, "workflows should be an array")
assert.Len(t, workflowsSlice, 2, "Should have 2 workflows")
assert.Contains(t, workflowsSlice, "worker-a", "Should contain worker-a")
assert.Contains(t, workflowsSlice, "worker-b", "Should contain worker-b")

// Verify workflow_files map
filesVal, ok := callWorkflowConfig["workflow_files"]
require.True(t, ok, "call_workflow config should have 'workflow_files' field")
filesMap, ok := filesVal.(map[string]any)
require.True(t, ok, "workflow_files should be a map")
assert.Equal(t, "./.github/workflows/worker-a.lock.yml", filesMap["worker-a"], "worker-a path should match")
assert.Equal(t, "./.github/workflows/worker-b.lock.yml", filesMap["worker-b"], "worker-b path should match")
}
1 change: 1 addition & 0 deletions pkg/workflow/compiler_safe_outputs_job.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ func (c *Compiler) buildConsolidatedSafeOutputsJob(data *WorkflowData, mainJobNa
data.SafeOutputs.HideComment != nil ||
data.SafeOutputs.SetIssueType != nil ||
data.SafeOutputs.DispatchWorkflow != nil ||
data.SafeOutputs.CallWorkflow != nil ||
data.SafeOutputs.CreateCodeScanningAlerts != nil ||
data.SafeOutputs.AutofixCodeScanningAlert != nil ||
data.SafeOutputs.MissingTool != nil ||
Expand Down
29 changes: 29 additions & 0 deletions pkg/workflow/compiler_safe_outputs_job_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -737,3 +737,32 @@ func TestConclusionJobWithGitHubAppWorkflowCallUsesTargetRepoNameFallback(t *tes
assert.NotContains(t, stepsContent, "repositories: ${{ needs.activation.outputs.target_repo }}",
"Conclusion job GitHub App token step must not use target_repo (full slug) for workflow_call workflows")
}

// TestCallWorkflowOnly_UsesHandlerManagerStep asserts that a workflow configured with only
// call-workflow (no other handler-manager types) still compiles a "Process Safe Outputs" step.
func TestCallWorkflowOnly_UsesHandlerManagerStep(t *testing.T) {
compiler := NewCompiler()
compiler.jobManager = NewJobManager()

workflowData := &WorkflowData{
Name: "Test Workflow",
SafeOutputs: &SafeOutputsConfig{
CallWorkflow: &CallWorkflowConfig{
BaseSafeOutputConfig: BaseSafeOutputConfig{
Max: strPtr("1"),
},
Workflows: []string{"worker-a"},
},
},
}

job, stepNames, err := compiler.buildConsolidatedSafeOutputsJob(workflowData, string(constants.AgentJobName), "test-workflow.md")
require.NoError(t, err, "Should compile without error")
require.NotNil(t, job, "safe_outputs job should be generated when only call-workflow is configured")
require.NotNil(t, stepNames, "Step names should not be nil")

stepsContent := strings.Join(job.Steps, "")
assert.Contains(t, stepsContent, "Process Safe Outputs", "Compiled job should include 'Process Safe Outputs' step")
assert.Contains(t, stepsContent, "GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG", "Compiled job should include handler config env var")
assert.Contains(t, stepsContent, "call_workflow", "Handler config should reference call_workflow")
}
1 change: 1 addition & 0 deletions pkg/workflow/safe_outputs_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ var safeOutputFieldMapping = map[string]string{
"LinkSubIssue": "link_sub_issue",
"HideComment": "hide_comment",
"DispatchWorkflow": "dispatch_workflow",
"CallWorkflow": "call_workflow",
"MissingTool": "missing_tool",
"MissingData": "missing_data",
"SetIssueType": "set_issue_type",
Expand Down
3 changes: 3 additions & 0 deletions pkg/workflow/unified_prompt_step.go
Original file line number Diff line number Diff line change
Expand Up @@ -726,6 +726,9 @@ func buildSafeOutputsSections(safeOutputs *SafeOutputsConfig) []PromptSection {
if safeOutputs.DispatchWorkflow != nil {
tools = append(tools, "dispatch_workflow")
}
if safeOutputs.CallWorkflow != nil {
tools = append(tools, "call_workflow")
}
if safeOutputs.MissingTool != nil {
tools = append(tools, "missing_tool")
}
Expand Down
Loading