diff --git a/.github/workflows/smoke-call-workflow.lock.yml b/.github/workflows/smoke-call-workflow.lock.yml index 7213a368141..8784a9f2fd2 100644 --- a/.github/workflows/smoke-call-workflow.lock.yml +++ b/.github/workflows/smoke-call-workflow.lock.yml @@ -1204,7 +1204,7 @@ jobs: GH_AW_ALLOWED_DOMAINS: "*.githubusercontent.com,127.0.0.1,172.30.0.1,::1,api.openai.com,api.snapcraft.io,app.renovatebot.com,appveyor.com,archive.ubuntu.com,azure.archive.ubuntu.com,badgen.net,circleci.com,codacy.com,codeclimate.com,codecov.io,codeload.github.com,coveralls.io,crl.geotrust.com,crl.globalsign.com,crl.identrust.com,crl.sectigo.com,crl.thawte.com,crl.usertrust.com,crl.verisign.com,crl3.digicert.com,crl4.digicert.com,crls.ssl.com,deepsource.io,docs.github.com,drone.io,github-cloud.githubusercontent.com,github-cloud.s3.amazonaws.com,github.blog,github.com,github.githubassets.com,host.docker.internal,img.shields.io,json-schema.org,json.schemastore.org,keyserver.ubuntu.com,lfs.github.com,localhost,objects.githubusercontent.com,ocsp.digicert.com,ocsp.geotrust.com,ocsp.globalsign.com,ocsp.identrust.com,ocsp.sectigo.com,ocsp.ssl.com,ocsp.thawte.com,ocsp.usertrust.com,ocsp.verisign.com,openai.com,packagecloud.io,packages.cloud.google.com,packages.microsoft.com,ppa.launchpad.net,raw.githubusercontent.com,readthedocs.io,readthedocs.org,renovatebot.com,s.symcb.com,s.symcd.com,security.ubuntu.com,semaphoreci.com,shields.io,snyk.io,sonarcloud.io,sonarqube.com,travis-ci.com,ts-crl.ws.symantec.com,ts-ocsp.ws.symantec.com" GITHUB_SERVER_URL: ${{ github.server_url }} GITHUB_API_URL: ${{ github.api_url }} - GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG: "{\"create_issue\":{\"labels\":[\"smoke-call-workflow\"],\"max\":1,\"title_prefix\":\"[smoke-call-workflow]\"},\"missing_data\":{},\"missing_tool\":{}}" + GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG: "{\"call_workflow\":{\"max\":1,\"workflow_files\":{\"smoke-workflow-call\":\"./.github/workflows/smoke-workflow-call.lock.yml\"},\"workflows\":[\"smoke-workflow-call\"]},\"create_issue\":{\"labels\":[\"smoke-call-workflow\"],\"max\":1,\"title_prefix\":\"[smoke-call-workflow]\"},\"missing_data\":{},\"missing_tool\":{}}" with: github-token: ${{ secrets.GH_AW_GITHUB_TOKEN || secrets.GITHUB_TOKEN }} script: | diff --git a/actions/setup/js/safe_output_handler_manager.cjs b/actions/setup/js/safe_output_handler_manager.cjs index 035c9135008..ff09c736d65 100644 --- a/actions/setup/js/safe_output_handler_manager.cjs +++ b/actions/setup/js/safe_output_handler_manager.cjs @@ -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", diff --git a/actions/setup/js/safe_output_handler_manager.test.cjs b/actions/setup/js/safe_output_handler_manager.test.cjs index 7da8c48ecaa..699676d3650 100644 --- a/actions/setup/js/safe_output_handler_manager.test.cjs +++ b/actions/setup/js/safe_output_handler_manager.test.cjs @@ -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'"); + }); + }); }); diff --git a/pkg/workflow/compiler_safe_outputs_config.go b/pkg/workflow/compiler_safe_outputs_config.go index d8e55fed4bc..259bcd039a8 100644 --- a/pkg/workflow/compiler_safe_outputs_config.go +++ b/pkg/workflow/compiler_safe_outputs_config.go @@ -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 diff --git a/pkg/workflow/compiler_safe_outputs_config_test.go b/pkg/workflow/compiler_safe_outputs_config_test.go index ee46f711a0c..5d91cf87e61 100644 --- a/pkg/workflow/compiler_safe_outputs_config_test.go +++ b/pkg/workflow/compiler_safe_outputs_config_test.go @@ -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 { @@ -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") +} diff --git a/pkg/workflow/compiler_safe_outputs_job.go b/pkg/workflow/compiler_safe_outputs_job.go index f376b452eb8..8bae8781548 100644 --- a/pkg/workflow/compiler_safe_outputs_job.go +++ b/pkg/workflow/compiler_safe_outputs_job.go @@ -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 || diff --git a/pkg/workflow/compiler_safe_outputs_job_test.go b/pkg/workflow/compiler_safe_outputs_job_test.go index 4e7739003de..024209119fd 100644 --- a/pkg/workflow/compiler_safe_outputs_job_test.go +++ b/pkg/workflow/compiler_safe_outputs_job_test.go @@ -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") +} diff --git a/pkg/workflow/safe_outputs_state.go b/pkg/workflow/safe_outputs_state.go index b605a5068b6..c8a8ce387cf 100644 --- a/pkg/workflow/safe_outputs_state.go +++ b/pkg/workflow/safe_outputs_state.go @@ -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", diff --git a/pkg/workflow/unified_prompt_step.go b/pkg/workflow/unified_prompt_step.go index 5626859e3ed..f727c8c0e25 100644 --- a/pkg/workflow/unified_prompt_step.go +++ b/pkg/workflow/unified_prompt_step.go @@ -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") }