From 0cc7571b8959c66bd82efbe08f7a63e58d44cded Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 18 Mar 2026 12:35:30 +0000 Subject: [PATCH 1/9] Initial plan From 0c031fec819a4fd53d401ba2bded93565a83f1a9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 18 Mar 2026 13:15:31 +0000 Subject: [PATCH 2/9] Changes before error encountered Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../setup/js/safe_output_handler_manager.cjs | 34 +- actions/setup/js/safe_output_helpers.cjs | 29 ++ pkg/workflow/compiler_safe_outputs_job.go | 50 ++- pkg/workflow/compiler_safe_outputs_steps.go | 7 + pkg/workflow/compiler_types.go | 1 + pkg/workflow/safe_outputs_config.go | 8 + .../safe_outputs_config_generation.go | 40 ++ pkg/workflow/safe_outputs_state.go | 11 + pkg/workflow/safe_outputs_tools_filtering.go | 39 +- pkg/workflow/safe_scripts.go | 141 +++++++ pkg/workflow/safe_scripts_test.go | 377 ++++++++++++++++++ 11 files changed, 734 insertions(+), 3 deletions(-) create mode 100644 pkg/workflow/safe_scripts.go create mode 100644 pkg/workflow/safe_scripts_test.go diff --git a/actions/setup/js/safe_output_handler_manager.cjs b/actions/setup/js/safe_output_handler_manager.cjs index ff09c736d65..2d6dc14d566 100644 --- a/actions/setup/js/safe_output_handler_manager.cjs +++ b/actions/setup/js/safe_output_handler_manager.cjs @@ -20,7 +20,7 @@ const { getIssuesToAssignCopilot } = require("./create_issue.cjs"); const { createReviewBuffer } = require("./pr_review_buffer.cjs"); const { sanitizeContent } = require("./sanitize_content.cjs"); const { createManifestLogger, ensureManifestExists, extractCreatedItemFromResult } = require("./safe_output_manifest.cjs"); -const { loadCustomSafeOutputJobTypes } = require("./safe_output_helpers.cjs"); +const { loadCustomSafeOutputJobTypes, loadCustomSafeOutputScriptHandlers } = require("./safe_output_helpers.cjs"); const { emitSafeOutputActionOutputs } = require("./safe_outputs_action_outputs.cjs"); /** @@ -162,6 +162,38 @@ async function loadHandlers(config, prReviewBuffer) { } } + // Load custom script handlers from GH_AW_SAFE_OUTPUT_SCRIPTS + // These are inline scripts defined in safe-outputs.scripts that run in the handler loop + const customScriptHandlers = loadCustomSafeOutputScriptHandlers(); + if (customScriptHandlers.size > 0) { + core.info(`Loading ${customScriptHandlers.size} custom script handler(s): ${[...customScriptHandlers.keys()].join(", ")}`); + for (const [scriptType, scriptFilename] of customScriptHandlers) { + const scriptPath = require("path").join(process.env.RUNNER_TEMP || "/tmp", "gh-aw", "actions", scriptFilename); + try { + const scriptModule = require(scriptPath); + if (scriptModule && typeof scriptModule.main === "function") { + const handlerConfig = config[scriptType] || {}; + const messageHandler = await scriptModule.main(handlerConfig); + if (typeof messageHandler !== "function") { + const error = new Error(`Script handler ${scriptType} main() did not return a function - expected a message handler function but got ${typeof messageHandler}`); + core.error(`✗ Fatal error loading script handler ${scriptType}: ${error.message}`); + throw error; + } + messageHandlers.set(scriptType, messageHandler); + core.info(`✓ Loaded and initialized custom script handler for: ${scriptType}`); + } else { + core.warning(`Custom script handler module ${scriptType} does not export a main function`); + } + } catch (error) { + const errorMessage = getErrorMessage(error); + if (errorMessage.includes("did not return a function")) { + throw error; + } + core.warning(`Failed to load custom script handler for ${scriptType}: ${errorMessage}`); + } + } + } + core.info(`Loaded ${messageHandlers.size} handler(s)`); return messageHandlers; } diff --git a/actions/setup/js/safe_output_helpers.cjs b/actions/setup/js/safe_output_helpers.cjs index 3dc9897fb14..6fe9dc01c39 100644 --- a/actions/setup/js/safe_output_helpers.cjs +++ b/actions/setup/js/safe_output_helpers.cjs @@ -335,11 +335,40 @@ function isUsernameBlocked(username, blockedPatterns) { return blockedPatterns.some(pattern => matchesBlockedPattern(username, pattern)); } +/** + * Load custom safe output script handlers from environment variable + * These are inline scripts defined in safe-outputs.scripts that run in the handler loop + * @returns {Map} Map of script type names to their .cjs filenames + */ +function loadCustomSafeOutputScriptHandlers() { + const safeOutputScriptsEnv = process.env.GH_AW_SAFE_OUTPUT_SCRIPTS; + if (!safeOutputScriptsEnv) { + return new Map(); + } + + try { + const safeOutputScripts = JSON.parse(safeOutputScriptsEnv); + // The environment variable is a map of normalized script names to .cjs filenames + const scriptHandlers = new Map(Object.entries(safeOutputScripts)); + if (typeof core !== "undefined") { + core.debug(`Loaded ${scriptHandlers.size} custom safe output script handler(s): ${[...scriptHandlers.keys()].join(", ")}`); + } + return scriptHandlers; + } catch (error) { + if (typeof core !== "undefined") { + const { getErrorMessage } = require("./error_helpers.cjs"); + core.warning(`Failed to parse GH_AW_SAFE_OUTPUT_SCRIPTS: ${getErrorMessage(error)}`); + } + return new Map(); + } +} + module.exports = { parseAllowedItems, parseMaxCount, resolveTarget, loadCustomSafeOutputJobTypes, + loadCustomSafeOutputScriptHandlers, resolveIssueNumber, extractAssignees, matchesBlockedPattern, diff --git a/pkg/workflow/compiler_safe_outputs_job.go b/pkg/workflow/compiler_safe_outputs_job.go index b6029f72da5..a5e9fe4d936 100644 --- a/pkg/workflow/compiler_safe_outputs_job.go +++ b/pkg/workflow/compiler_safe_outputs_job.go @@ -2,9 +2,12 @@ package workflow import ( "fmt" + "sort" + "strings" "github.com/github/gh-aw/pkg/constants" "github.com/github/gh-aw/pkg/logger" + "github.com/github/gh-aw/pkg/stringutil" ) var consolidatedSafeOutputsJobLog = logger.New("workflow:compiler_safe_outputs_job") @@ -145,11 +148,20 @@ func (c *Compiler) buildConsolidatedSafeOutputsJob(data *WorkflowData, mainJobNa data.SafeOutputs.CreateCodeScanningAlerts != nil || data.SafeOutputs.AutofixCodeScanningAlert != nil || data.SafeOutputs.MissingTool != nil || - data.SafeOutputs.MissingData != nil + data.SafeOutputs.MissingData != nil || + len(data.SafeOutputs.Scripts) > 0 // Custom scripts run in the handler loop // Note: All project-related operations are now handled by the unified handler. // The project handler manager has been removed. + // Add custom script files step (writes inline scripts to the actions folder) + // This must run before the handler manager step so the files are available for require() + if len(data.SafeOutputs.Scripts) > 0 { + consolidatedSafeOutputsJobLog.Printf("Adding setup step for %d custom safe-output script(s)", len(data.SafeOutputs.Scripts)) + scriptSetupSteps := buildCustomScriptFilesStep(data.SafeOutputs.Scripts) + steps = append(steps, scriptSetupSteps...) + } + // 1. Handler Manager step (processes create_issue, update_issue, add_comment, etc.) // This processes all safe output types that are handled by the unified handler // Critical for workflows that create projects and then add issues/PRs to those projects @@ -501,3 +513,39 @@ func buildSafeOutputItemsManifestUploadStep(prefix string) []string { " if-no-files-found: warn\n", } } + +// buildCustomScriptFilesStep generates a run step that writes inline safe-output script files +// to the setup action destination folder so they can be required by the handler manager. +// Each script is written using a heredoc to avoid shell quoting issues. +func buildCustomScriptFilesStep(scripts map[string]*SafeScriptConfig) []string { + if len(scripts) == 0 { + return nil + } + + // Sort script names for deterministic output + scriptNames := make([]string, 0, len(scripts)) + for name := range scripts { + scriptNames = append(scriptNames, name) + } + sort.Strings(scriptNames) + + var steps []string + steps = append(steps, " - name: Setup Safe Output Custom Scripts\n") + steps = append(steps, " run: |\n") + + for _, scriptName := range scriptNames { + scriptConfig := scripts[scriptName] + normalizedName := stringutil.NormalizeSafeOutputIdentifier(scriptName) + filename := safeOutputScriptFilename(normalizedName) + filepath := SetupActionDestinationShell + "/" + filename + delimiter := GenerateHeredocDelimiter("SAFE_OUTPUT_SCRIPT_" + strings.ToUpper(normalizedName)) + + steps = append(steps, fmt.Sprintf(" cat > %s << '%s'\n", filepath, delimiter)) + for line := range strings.SplitSeq(scriptConfig.Script, "\n") { + steps = append(steps, " "+line+"\n") + } + steps = append(steps, " "+delimiter+"\n") + } + + return steps +} diff --git a/pkg/workflow/compiler_safe_outputs_steps.go b/pkg/workflow/compiler_safe_outputs_steps.go index a218fab6b36..438360066dc 100644 --- a/pkg/workflow/compiler_safe_outputs_steps.go +++ b/pkg/workflow/compiler_safe_outputs_steps.go @@ -342,6 +342,13 @@ func (c *Compiler) buildHandlerManagerStep(data *WorkflowData) []string { consolidatedSafeOutputsStepsLog.Print("Added GH_AW_SAFE_OUTPUT_JOBS env var for custom safe job types") } + // Add GH_AW_SAFE_OUTPUT_SCRIPTS so the handler manager can load inline script handlers. + // The env var maps normalized script names to their .cjs filenames in the actions folder. + if customScriptsJSON := buildCustomSafeOutputScriptsJSON(data); customScriptsJSON != "" { + steps = append(steps, fmt.Sprintf(" GH_AW_SAFE_OUTPUT_SCRIPTS: %q\n", customScriptsJSON)) + consolidatedSafeOutputsStepsLog.Print("Added GH_AW_SAFE_OUTPUT_SCRIPTS env var for custom script handlers") + } + // Add custom safe output env vars c.addCustomSafeOutputEnvVars(&steps, data) diff --git a/pkg/workflow/compiler_types.go b/pkg/workflow/compiler_types.go index dc3ed383288..71c2b86ee9e 100644 --- a/pkg/workflow/compiler_types.go +++ b/pkg/workflow/compiler_types.go @@ -482,6 +482,7 @@ type SafeOutputsConfig struct { NoOp *NoOpConfig `yaml:"noop,omitempty"` // No-op output for logging only (always available as fallback) ThreatDetection *ThreatDetectionConfig `yaml:"threat-detection,omitempty"` // Threat detection configuration Jobs map[string]*SafeJobConfig `yaml:"jobs,omitempty"` // Safe-jobs configuration (moved from top-level) + Scripts map[string]*SafeScriptConfig `yaml:"scripts,omitempty"` // Custom inline handlers that run in the safe-output handler loop GitHubApp *GitHubAppConfig `yaml:"github-app,omitempty"` // GitHub App credentials for token minting AllowedDomains []string `yaml:"allowed-domains,omitempty"` // Allowed domains for URL redaction, unioned with network.allowed; supports ecosystem identifiers AllowGitHubReferences []string `yaml:"allowed-github-references,omitempty"` // Allowed repositories for GitHub references (e.g., ["repo", "org/repo2"]) diff --git a/pkg/workflow/safe_outputs_config.go b/pkg/workflow/safe_outputs_config.go index 61ce26df1cf..d0c124a7ff5 100644 --- a/pkg/workflow/safe_outputs_config.go +++ b/pkg/workflow/safe_outputs_config.go @@ -531,6 +531,14 @@ func (c *Compiler) extractSafeOutputsConfig(frontmatter map[string]any) *SafeOut } } + // Handle scripts (inline handlers that run in the safe-output handler loop) + if scripts, exists := outputMap["scripts"]; exists { + if scriptsMap, ok := scripts.(map[string]any); ok { + config.Scripts = parseSafeScriptsConfig(scriptsMap) + safeOutputsConfigLog.Printf("Configured %d custom safe-output script(s)", len(config.Scripts)) + } + } + // Handle app configuration for GitHub App token minting if app, exists := outputMap["github-app"]; exists { if appMap, ok := app.(map[string]any); ok { diff --git a/pkg/workflow/safe_outputs_config_generation.go b/pkg/workflow/safe_outputs_config_generation.go index 8b745fa4d40..e6d555ccfb7 100644 --- a/pkg/workflow/safe_outputs_config_generation.go +++ b/pkg/workflow/safe_outputs_config_generation.go @@ -6,6 +6,8 @@ import ( "sort" "strconv" "strings" + + "github.com/github/gh-aw/pkg/stringutil" ) // ======================================== @@ -438,6 +440,44 @@ func generateSafeOutputsConfig(data *WorkflowData) string { } } + // Add safe-scripts configuration from SafeOutputs.Scripts + // Scripts run in the handler loop, so they are registered the same way as jobs in the config + if len(data.SafeOutputs.Scripts) > 0 { + safeOutputsConfigLog.Printf("Processing %d safe script configurations", len(data.SafeOutputs.Scripts)) + for scriptName, scriptConfig := range data.SafeOutputs.Scripts { + normalizedName := stringutil.NormalizeSafeOutputIdentifier(scriptName) + safeOutputsConfigLog.Printf("Generating config for safe script: %s (normalized: %s)", scriptName, normalizedName) + safeScriptConfigMap := map[string]any{} + + // Add description if present + if scriptConfig.Description != "" { + safeScriptConfigMap["description"] = scriptConfig.Description + } + + // Add inputs information + if len(scriptConfig.Inputs) > 0 { + inputsConfig := make(map[string]any) + for inputName, inputDef := range scriptConfig.Inputs { + inputConfig := map[string]any{ + "type": inputDef.Type, + "description": inputDef.Description, + "required": inputDef.Required, + } + if inputDef.Default != "" { + inputConfig["default"] = inputDef.Default + } + if len(inputDef.Options) > 0 { + inputConfig["options"] = inputDef.Options + } + inputsConfig[inputName] = inputConfig + } + safeScriptConfigMap["inputs"] = inputsConfig + } + + safeOutputsConfig[normalizedName] = safeScriptConfigMap + } + } + // Add mentions configuration if data.SafeOutputs.Mentions != nil { mentionsConfig := make(map[string]any) diff --git a/pkg/workflow/safe_outputs_state.go b/pkg/workflow/safe_outputs_state.go index c8a8ce387cf..267c1632512 100644 --- a/pkg/workflow/safe_outputs_state.go +++ b/pkg/workflow/safe_outputs_state.go @@ -78,6 +78,12 @@ func hasAnySafeOutputEnabled(safeOutputs *SafeOutputsConfig) bool { return true } + // Check Scripts separately as it's a map + if len(safeOutputs.Scripts) > 0 { + safeOutputReflectionLog.Printf("Found %d custom scripts enabled", len(safeOutputs.Scripts)) + return true + } + // Use reflection to check all pointer fields val := reflect.ValueOf(safeOutputs).Elem() for fieldName := range safeOutputFieldMapping { @@ -125,6 +131,11 @@ func hasNonBuiltinSafeOutputsEnabled(safeOutputs *SafeOutputsConfig) bool { return true } + // Custom scripts are always non-builtin + if len(safeOutputs.Scripts) > 0 { + return true + } + // Check non-builtin pointer fields using the pre-computed list val := reflect.ValueOf(safeOutputs).Elem() for _, fieldName := range nonBuiltinSafeOutputFieldNames { diff --git a/pkg/workflow/safe_outputs_tools_filtering.go b/pkg/workflow/safe_outputs_tools_filtering.go index 354fc6a2ea2..08f83190dae 100644 --- a/pkg/workflow/safe_outputs_tools_filtering.go +++ b/pkg/workflow/safe_outputs_tools_filtering.go @@ -230,8 +230,27 @@ func generateFilteredToolsJSON(data *WorkflowData, markdownPath string) (string, } } + // Add custom script tools from SafeOutputs.Scripts + if len(data.SafeOutputs.Scripts) > 0 { + safeOutputsConfigLog.Printf("Adding %d custom script tools", len(data.SafeOutputs.Scripts)) + + // Sort script names for deterministic output + scriptNames := make([]string, 0, len(data.SafeOutputs.Scripts)) + for scriptName := range data.SafeOutputs.Scripts { + scriptNames = append(scriptNames, scriptName) + } + sort.Strings(scriptNames) + + for _, scriptName := range scriptNames { + scriptConfig := data.SafeOutputs.Scripts[scriptName] + normalizedScriptName := stringutil.NormalizeSafeOutputIdentifier(scriptName) + customTool := generateCustomScriptToolDefinition(normalizedScriptName, scriptConfig) + filteredTools = append(filteredTools, customTool) + } + } + if safeOutputsConfigLog.Enabled() { - safeOutputsConfigLog.Printf("Filtered %d tools from %d total tools (including %d custom jobs)", len(filteredTools), len(allTools), len(data.SafeOutputs.Jobs)) + safeOutputsConfigLog.Printf("Filtered %d tools from %d total tools (including %d custom jobs, %d custom scripts)", len(filteredTools), len(allTools), len(data.SafeOutputs.Jobs), len(data.SafeOutputs.Scripts)) } // Add dynamic dispatch_workflow tools @@ -726,6 +745,24 @@ func generateDynamicTools(data *WorkflowData, markdownPath string) ([]map[string } } + // Add custom script tools from SafeOutputs.Scripts + if len(data.SafeOutputs.Scripts) > 0 { + safeOutputsConfigLog.Printf("Adding %d custom script tools to dynamic tools", len(data.SafeOutputs.Scripts)) + + scriptNames := make([]string, 0, len(data.SafeOutputs.Scripts)) + for scriptName := range data.SafeOutputs.Scripts { + scriptNames = append(scriptNames, scriptName) + } + sort.Strings(scriptNames) + + for _, scriptName := range scriptNames { + scriptConfig := data.SafeOutputs.Scripts[scriptName] + normalizedScriptName := stringutil.NormalizeSafeOutputIdentifier(scriptName) + customTool := generateCustomScriptToolDefinition(normalizedScriptName, scriptConfig) + dynamicTools = append(dynamicTools, customTool) + } + } + // Add dynamic dispatch_workflow tools if data.SafeOutputs.DispatchWorkflow != nil && len(data.SafeOutputs.DispatchWorkflow.Workflows) > 0 { safeOutputsConfigLog.Printf("Adding %d dispatch_workflow tools", len(data.SafeOutputs.DispatchWorkflow.Workflows)) diff --git a/pkg/workflow/safe_scripts.go b/pkg/workflow/safe_scripts.go new file mode 100644 index 00000000000..9dcca5fab69 --- /dev/null +++ b/pkg/workflow/safe_scripts.go @@ -0,0 +1,141 @@ +package workflow + +import ( + "encoding/json" + "sort" + + "github.com/github/gh-aw/pkg/logger" + "github.com/github/gh-aw/pkg/stringutil" +) + +var safeScriptsLog = logger.New("workflow:safe_scripts") + +// SafeScriptConfig defines a custom safe output handler script that runs in the handler loop. +// Scripts run within the consolidated safe-outputs job as part of the handler manager, +// unlike SafeJobConfig which creates a separate GitHub Actions job. +type SafeScriptConfig struct { + Name string `yaml:"name,omitempty"` + Description string `yaml:"description,omitempty"` + Inputs map[string]*InputDefinition `yaml:"inputs,omitempty"` + Script string `yaml:"script,omitempty"` // Inline JavaScript handler (must export a main factory function) +} + +// parseSafeScriptsConfig parses safe-scripts configuration from a scripts map. +// This function expects a map of script configurations directly (from safe-outputs.scripts). +func parseSafeScriptsConfig(scriptsMap map[string]any) map[string]*SafeScriptConfig { + if scriptsMap == nil { + return nil + } + + safeScriptsLog.Printf("Parsing %d safe-scripts from scripts map", len(scriptsMap)) + result := make(map[string]*SafeScriptConfig) + + for scriptName, scriptValue := range scriptsMap { + scriptConfig, ok := scriptValue.(map[string]any) + if !ok { + continue + } + + safeScript := &SafeScriptConfig{} + + // Parse name + if name, exists := scriptConfig["name"]; exists { + if nameStr, ok := name.(string); ok { + safeScript.Name = nameStr + } + } + + // Parse description + if description, exists := scriptConfig["description"]; exists { + if descStr, ok := description.(string); ok { + safeScript.Description = descStr + } + } + + // Parse inputs using the unified parsing function + if inputs, exists := scriptConfig["inputs"]; exists { + if inputsMap, ok := inputs.(map[string]any); ok { + safeScript.Inputs = ParseInputDefinitions(inputsMap) + } + } + + // Parse script content + if script, exists := scriptConfig["script"]; exists { + if scriptStr, ok := script.(string); ok { + safeScript.Script = scriptStr + } + } + + safeScriptsLog.Printf("Parsed safe-script configuration: name=%s, has_script=%v, has_inputs=%v", + scriptName, safeScript.Script != "", len(safeScript.Inputs) > 0) + result[scriptName] = safeScript + } + + return result +} + +// extractSafeScriptsFromFrontmatter extracts safe-scripts configuration from frontmatter. +func extractSafeScriptsFromFrontmatter(frontmatter map[string]any) map[string]*SafeScriptConfig { + if safeOutputs, exists := frontmatter["safe-outputs"]; exists { + if safeOutputsMap, ok := safeOutputs.(map[string]any); ok { + if scripts, exists := safeOutputsMap["scripts"]; exists { + if scriptsMap, ok := scripts.(map[string]any); ok { + return parseSafeScriptsConfig(scriptsMap) + } + } + } + } + return make(map[string]*SafeScriptConfig) +} + +// buildCustomSafeOutputScriptsJSON builds a JSON mapping of custom safe output script names to their +// .cjs filenames, for use in the GH_AW_SAFE_OUTPUT_SCRIPTS env var of the handler manager step. +// This allows the handler manager to load and dispatch messages to inline script handlers. +func buildCustomSafeOutputScriptsJSON(data *WorkflowData) string { + if data.SafeOutputs == nil || len(data.SafeOutputs.Scripts) == 0 { + return "" + } + + // Build mapping of normalized script names to their .cjs filenames + scriptMapping := make(map[string]string, len(data.SafeOutputs.Scripts)) + for scriptName := range data.SafeOutputs.Scripts { + normalizedName := stringutil.NormalizeSafeOutputIdentifier(scriptName) + scriptMapping[normalizedName] = safeOutputScriptFilename(normalizedName) + } + + // Sort keys for deterministic output + keys := make([]string, 0, len(scriptMapping)) + for k := range scriptMapping { + keys = append(keys, k) + } + sort.Strings(keys) + + ordered := make(map[string]string, len(keys)) + for _, k := range keys { + ordered[k] = scriptMapping[k] + } + + jsonBytes, err := json.Marshal(ordered) + if err != nil { + safeScriptsLog.Printf("Warning: failed to marshal custom safe output scripts: %v", err) + return "" + } + return string(jsonBytes) +} + +// safeOutputScriptFilename returns the .cjs filename for a normalized safe output script name. +func safeOutputScriptFilename(normalizedName string) string { + return "safe_output_script_" + normalizedName + ".cjs" +} + +// generateCustomScriptToolDefinition creates an MCP tool definition for a custom safe-output script. +// Returns a map representing the tool definition in MCP format with name, description, and inputSchema. +// Scripts share the same tool schema generation logic as custom safe-output jobs. +func generateCustomScriptToolDefinition(scriptName string, scriptConfig *SafeScriptConfig) map[string]any { + // Reuse custom job tool definition logic by adapting the script config + jobConfig := &SafeJobConfig{ + Description: scriptConfig.Description, + Inputs: scriptConfig.Inputs, + } + return generateCustomJobToolDefinition(scriptName, jobConfig) +} diff --git a/pkg/workflow/safe_scripts_test.go b/pkg/workflow/safe_scripts_test.go new file mode 100644 index 00000000000..ccd820965da --- /dev/null +++ b/pkg/workflow/safe_scripts_test.go @@ -0,0 +1,377 @@ +//go:build !integration + +package workflow + +import ( + "encoding/json" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestParseSafeScriptsConfig verifies parsing of safe-scripts configuration +func TestParseSafeScriptsConfig(t *testing.T) { + scriptsMap := map[string]any{ + "slack-post-message": map[string]any{ + "name": "Post Slack Message", + "description": "Post a message to a Slack channel", + "inputs": map[string]any{ + "channel": map[string]any{ + "description": "Slack channel name", + "required": true, + "type": "string", + }, + "message": map[string]any{ + "description": "Message content", + "required": true, + "type": "string", + }, + }, + "script": `async function main(config = {}) { + return async function handleSlackPostMessage(message, resolvedTemporaryIds) { + return { success: true }; + }; +} +module.exports = { main };`, + }, + } + + result := parseSafeScriptsConfig(scriptsMap) + + require.NotNil(t, result, "Should return non-nil result") + require.Len(t, result, 1, "Should have one script") + + script, exists := result["slack-post-message"] + require.True(t, exists, "Should have slack-post-message script") + assert.Equal(t, "Post Slack Message", script.Name, "Name should match") + assert.Equal(t, "Post a message to a Slack channel", script.Description, "Description should match") + assert.Contains(t, script.Script, "async function main", "Script should contain main function") + + require.Len(t, script.Inputs, 2, "Should have 2 inputs") + + channelInput, ok := script.Inputs["channel"] + require.True(t, ok, "Should have channel input") + assert.Equal(t, "string", channelInput.Type, "Channel type should be string") + assert.True(t, channelInput.Required, "Channel should be required") + + messageInput, ok := script.Inputs["message"] + require.True(t, ok, "Should have message input") + assert.Equal(t, "string", messageInput.Type, "Message type should be string") + assert.True(t, messageInput.Required, "Message should be required") +} + +// TestParseSafeScriptsConfigNilMap verifies nil input handling +func TestParseSafeScriptsConfigNilMap(t *testing.T) { + result := parseSafeScriptsConfig(nil) + assert.Nil(t, result, "Should return nil for nil map") +} + +// TestExtractSafeScriptsFromFrontmatter verifies extraction from frontmatter +func TestExtractSafeScriptsFromFrontmatter(t *testing.T) { + frontmatter := map[string]any{ + "safe-outputs": map[string]any{ + "scripts": map[string]any{ + "my-handler": map[string]any{ + "description": "A custom handler", + "script": "module.exports = { main: async (c) => async (m) => ({ success: true }) };", + }, + }, + }, + } + + result := extractSafeScriptsFromFrontmatter(frontmatter) + + require.Len(t, result, 1, "Should have one script") + script, exists := result["my-handler"] + require.True(t, exists, "Should have my-handler script") + assert.Equal(t, "A custom handler", script.Description, "Description should match") +} + +// TestExtractSafeScriptsFromFrontmatterEmpty verifies empty result when no scripts +func TestExtractSafeScriptsFromFrontmatterEmpty(t *testing.T) { + frontmatter := map[string]any{ + "safe-outputs": map[string]any{ + "create-issue": map[string]any{}, + }, + } + + result := extractSafeScriptsFromFrontmatter(frontmatter) + assert.Empty(t, result, "Should return empty map when no scripts") +} + +// TestBuildCustomSafeOutputScriptsJSON verifies JSON generation for script env var +func TestBuildCustomSafeOutputScriptsJSON(t *testing.T) { + data := &WorkflowData{ + SafeOutputs: &SafeOutputsConfig{ + Scripts: map[string]*SafeScriptConfig{ + "my-handler": { + Description: "Custom handler", + Script: "module.exports = { main: async (c) => async (m) => ({ success: true }) };", + }, + }, + }, + } + + jsonStr := buildCustomSafeOutputScriptsJSON(data) + require.NotEmpty(t, jsonStr, "Should produce non-empty JSON") + + var mapping map[string]string + err := json.Unmarshal([]byte(jsonStr), &mapping) + require.NoError(t, err, "JSON should be valid") + + filename, exists := mapping["my_handler"] + require.True(t, exists, "Should have my_handler key (normalized with underscore)") + assert.Equal(t, "safe_output_script_my_handler.cjs", filename, "Filename should match expected pattern") +} + +// TestBuildCustomSafeOutputScriptsJSONNormalization verifies name normalization with dashes +func TestBuildCustomSafeOutputScriptsJSONNormalization(t *testing.T) { + data := &WorkflowData{ + SafeOutputs: &SafeOutputsConfig{ + Scripts: map[string]*SafeScriptConfig{ + "slack-post-message": { + Script: "module.exports = { main: async (c) => async (m) => ({ success: true }) };", + }, + "notify-team": { + Script: "module.exports = { main: async (c) => async (m) => ({ success: true }) };", + }, + }, + }, + } + + jsonStr := buildCustomSafeOutputScriptsJSON(data) + require.NotEmpty(t, jsonStr, "Should produce non-empty JSON") + + var mapping map[string]string + err := json.Unmarshal([]byte(jsonStr), &mapping) + require.NoError(t, err, "JSON should be valid") + + // Check names are normalized to underscores + assert.Contains(t, mapping, "slack_post_message", "Should normalize dashes to underscores") + assert.Contains(t, mapping, "notify_team", "Should normalize dashes to underscores") + assert.Equal(t, "safe_output_script_slack_post_message.cjs", mapping["slack_post_message"], "Filename should match") + assert.Equal(t, "safe_output_script_notify_team.cjs", mapping["notify_team"], "Filename should match") +} + +// TestBuildCustomSafeOutputScriptsJSONEmpty verifies empty output when no scripts +func TestBuildCustomSafeOutputScriptsJSONEmpty(t *testing.T) { + data := &WorkflowData{ + SafeOutputs: &SafeOutputsConfig{}, + } + assert.Empty(t, buildCustomSafeOutputScriptsJSON(data), "Should return empty for no scripts") + + dataNil := &WorkflowData{SafeOutputs: nil} + assert.Empty(t, buildCustomSafeOutputScriptsJSON(dataNil), "Should return empty for nil SafeOutputs") +} + +// TestGenerateCustomScriptToolDefinition verifies MCP tool definition for scripts +func TestGenerateCustomScriptToolDefinition(t *testing.T) { + scriptConfig := &SafeScriptConfig{ + Description: "Post a message to Slack", + Inputs: map[string]*InputDefinition{ + "channel": { + Description: "Target Slack channel", + Required: true, + Type: "string", + }, + "message": { + Description: "Message text", + Required: true, + Type: "string", + }, + }, + Script: "module.exports = { main: async (c) => async (m) => ({ success: true }) };", + } + + tool := generateCustomScriptToolDefinition("slack_post_message", scriptConfig) + + assert.Equal(t, "slack_post_message", tool["name"], "Tool name should match") + assert.Equal(t, "Post a message to Slack", tool["description"], "Description should match") + + inputSchema, ok := tool["inputSchema"].(map[string]any) + require.True(t, ok, "Should have inputSchema") + assert.Equal(t, "object", inputSchema["type"], "Schema type should be object") + assert.Equal(t, false, inputSchema["additionalProperties"], "Should not allow additional properties") + + properties, ok := inputSchema["properties"].(map[string]any) + require.True(t, ok, "Should have properties") + assert.Len(t, properties, 2, "Should have 2 properties") + + required, ok := inputSchema["required"].([]string) + require.True(t, ok, "Should have required field") + assert.Len(t, required, 2, "Should have 2 required fields") + assert.Contains(t, required, "channel", "Should require channel") + assert.Contains(t, required, "message", "Should require message") +} + +// TestScriptToolsInFilteredJSON verifies scripts appear in the filtered tools JSON +func TestScriptToolsInFilteredJSON(t *testing.T) { + workflowData := &WorkflowData{ + SafeOutputs: &SafeOutputsConfig{ + Scripts: map[string]*SafeScriptConfig{ + "my-custom-handler": { + Description: "A custom script handler", + Inputs: map[string]*InputDefinition{ + "target": { + Description: "Target to process", + Required: true, + Type: "string", + }, + }, + Script: "module.exports = { main: async (c) => async (m) => ({ success: true }) };", + }, + }, + }, + } + + toolsJSON, err := generateFilteredToolsJSON(workflowData, ".github/workflows/test.md") + require.NoError(t, err, "Should generate tools JSON without error") + + var tools []map[string]any + err = json.Unmarshal([]byte(toolsJSON), &tools) + require.NoError(t, err, "Tools JSON should be parseable") + + var customTool map[string]any + for _, tool := range tools { + if name, ok := tool["name"].(string); ok && name == "my_custom_handler" { + customTool = tool + break + } + } + require.NotNil(t, customTool, "Should find my_custom_handler tool in tools JSON") + assert.Equal(t, "A custom script handler", customTool["description"], "Description should match") + + inputSchema, ok := customTool["inputSchema"].(map[string]any) + require.True(t, ok, "Should have inputSchema") + + properties, ok := inputSchema["properties"].(map[string]any) + require.True(t, ok, "Should have properties") + assert.Contains(t, properties, "target", "Should have target property") +} + +// TestBuildCustomScriptFilesStep verifies the generated step writes scripts to files +func TestBuildCustomScriptFilesStep(t *testing.T) { + scripts := map[string]*SafeScriptConfig{ + "my-handler": { + Script: "async function main(config) {\n return async (msg) => ({ success: true });\n}\nmodule.exports = { main };", + }, + } + + steps := buildCustomScriptFilesStep(scripts) + + require.NotEmpty(t, steps, "Should produce steps") + + fullYAML := strings.Join(steps, "") + + assert.Contains(t, fullYAML, "Setup Safe Output Custom Scripts", "Should have setup step name") + assert.Contains(t, fullYAML, "safe_output_script_my_handler.cjs", "Should reference the output filename") + assert.Contains(t, fullYAML, "GH_AW_SAFE_OUTPUT_SCRIPT_MY_HANDLER_EOF", "Should use correct heredoc delimiter") + assert.Contains(t, fullYAML, "async function main", "Should include script content") +} + +// TestBuildCustomScriptFilesStepEmpty verifies nil return for empty scripts +func TestBuildCustomScriptFilesStepEmpty(t *testing.T) { + steps := buildCustomScriptFilesStep(nil) + assert.Nil(t, steps, "Should return nil for empty scripts") + + stepsEmpty := buildCustomScriptFilesStep(map[string]*SafeScriptConfig{}) + assert.Nil(t, stepsEmpty, "Should return nil for empty map") +} + +// TestHandlerManagerStepIncludesScriptsEnvVar verifies GH_AW_SAFE_OUTPUT_SCRIPTS in handler manager +func TestHandlerManagerStepIncludesScriptsEnvVar(t *testing.T) { + compiler := NewCompiler() + workflowData := &WorkflowData{ + SafeOutputs: &SafeOutputsConfig{ + CreateIssues: &CreateIssuesConfig{}, + Scripts: map[string]*SafeScriptConfig{ + "my-script": { + Description: "Custom script", + Script: "module.exports = { main: async (c) => async (m) => ({ success: true }) };", + }, + }, + }, + } + + steps := compiler.buildHandlerManagerStep(workflowData) + fullYAML := strings.Join(steps, "") + + assert.Contains(t, fullYAML, "GH_AW_SAFE_OUTPUT_SCRIPTS", "Should include GH_AW_SAFE_OUTPUT_SCRIPTS env var") + assert.Contains(t, fullYAML, "my_script", "Should include normalized script name") + assert.Contains(t, fullYAML, "safe_output_script_my_script.cjs", "Should include script filename") +} + +// TestHandlerManagerStepNoScriptsEnvVar verifies GH_AW_SAFE_OUTPUT_SCRIPTS absent when no scripts +func TestHandlerManagerStepNoScriptsEnvVar(t *testing.T) { + compiler := NewCompiler() + workflowData := &WorkflowData{ + SafeOutputs: &SafeOutputsConfig{ + CreateIssues: &CreateIssuesConfig{}, + }, + } + + steps := compiler.buildHandlerManagerStep(workflowData) + fullYAML := strings.Join(steps, "") + + assert.NotContains(t, fullYAML, "GH_AW_SAFE_OUTPUT_SCRIPTS", "Should not include GH_AW_SAFE_OUTPUT_SCRIPTS env var when no scripts") +} + +// TestSafeOutputsConfigIncludesScripts verifies extractSafeOutputsConfig handles scripts +func TestSafeOutputsConfigIncludesScripts(t *testing.T) { + compiler := NewCompiler() + frontmatter := map[string]any{ + "safe-outputs": map[string]any{ + "scripts": map[string]any{ + "post-webhook": map[string]any{ + "name": "Post Webhook", + "description": "Post a webhook notification", + "inputs": map[string]any{ + "url": map[string]any{ + "description": "Webhook URL", + "required": true, + "type": "string", + }, + }, + "script": "module.exports = { main: async (c) => async (m) => ({ success: true }) };", + }, + }, + }, + } + + config := compiler.extractSafeOutputsConfig(frontmatter) + + require.NotNil(t, config, "Should extract config") + require.Len(t, config.Scripts, 1, "Should have 1 script") + + script, exists := config.Scripts["post-webhook"] + require.True(t, exists, "Should have post-webhook script") + assert.Equal(t, "Post Webhook", script.Name, "Name should match") + assert.Equal(t, "Post a webhook notification", script.Description, "Description should match") + require.Len(t, script.Inputs, 1, "Should have 1 input") +} + +// TestHasAnySafeOutputEnabledWithScripts verifies Scripts are detected as enabled +func TestHasAnySafeOutputEnabledWithScripts(t *testing.T) { + config := &SafeOutputsConfig{ + Scripts: map[string]*SafeScriptConfig{ + "my-script": { + Script: "module.exports = { main: async (c) => async (m) => ({ success: true }) };", + }, + }, + } + assert.True(t, hasAnySafeOutputEnabled(config), "Should detect scripts as enabled safe outputs") +} + +// TestHasNonBuiltinSafeOutputsEnabledWithScripts verifies Scripts count as non-builtin +func TestHasNonBuiltinSafeOutputsEnabledWithScripts(t *testing.T) { + config := &SafeOutputsConfig{ + Scripts: map[string]*SafeScriptConfig{ + "my-script": { + Script: "module.exports = { main: async (c) => async (m) => ({ success: true }) };", + }, + }, + } + assert.True(t, hasNonBuiltinSafeOutputsEnabled(config), "Scripts should count as non-builtin safe outputs") +} From bda995e3cf2e0ee939a3b83afc2a6ccdc740783c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 18 Mar 2026 17:03:35 +0000 Subject: [PATCH 3/9] =?UTF-8?q?feat:=20wrap=20script=20body=20in=20main()?= =?UTF-8?q?=20function=20=E2=80=94=20users=20write=20handler=20body=20only?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/compiler_safe_outputs_job.go | 26 +++++++-- pkg/workflow/safe_scripts_test.go | 66 ++++++++++++++++------- 2 files changed, 70 insertions(+), 22 deletions(-) diff --git a/pkg/workflow/compiler_safe_outputs_job.go b/pkg/workflow/compiler_safe_outputs_job.go index a5e9fe4d936..b7b65ac5e59 100644 --- a/pkg/workflow/compiler_safe_outputs_job.go +++ b/pkg/workflow/compiler_safe_outputs_job.go @@ -514,8 +514,27 @@ func buildSafeOutputItemsManifestUploadStep(prefix string) []string { } } +// generateSafeOutputScriptContent wraps the user's script body in a main function template. +// Users write only the body of the main function (the handler setup code and return statement), +// and the compiler wraps it with the function declaration and module.exports — the same pattern +// used by MCP-scripts and actions/github-script. +func generateSafeOutputScriptContent(scriptName string, scriptBody string) string { + var sb strings.Builder + sb.WriteString("// Auto-generated safe-output script handler: " + scriptName + "\n") + sb.WriteString("async function main(config = {}) {\n") + // Indent each line of the user's body by 2 spaces + for line := range strings.SplitSeq(scriptBody, "\n") { + sb.WriteString(" " + line + "\n") + } + sb.WriteString("}\n") + sb.WriteString("module.exports = { main };\n") + return sb.String() +} + // buildCustomScriptFilesStep generates a run step that writes inline safe-output script files // to the setup action destination folder so they can be required by the handler manager. +// The user's script body is automatically wrapped with async function main(config = {}) { ... } +// and module.exports = { main }; — users write only the handler body, not the boilerplate. // Each script is written using a heredoc to avoid shell quoting issues. func buildCustomScriptFilesStep(scripts map[string]*SafeScriptConfig) []string { if len(scripts) == 0 { @@ -537,11 +556,12 @@ func buildCustomScriptFilesStep(scripts map[string]*SafeScriptConfig) []string { scriptConfig := scripts[scriptName] normalizedName := stringutil.NormalizeSafeOutputIdentifier(scriptName) filename := safeOutputScriptFilename(normalizedName) - filepath := SetupActionDestinationShell + "/" + filename + filePath := SetupActionDestinationShell + "/" + filename delimiter := GenerateHeredocDelimiter("SAFE_OUTPUT_SCRIPT_" + strings.ToUpper(normalizedName)) + scriptContent := generateSafeOutputScriptContent(scriptName, scriptConfig.Script) - steps = append(steps, fmt.Sprintf(" cat > %s << '%s'\n", filepath, delimiter)) - for line := range strings.SplitSeq(scriptConfig.Script, "\n") { + steps = append(steps, fmt.Sprintf(" cat > %s << '%s'\n", filePath, delimiter)) + for line := range strings.SplitSeq(scriptContent, "\n") { steps = append(steps, " "+line+"\n") } steps = append(steps, " "+delimiter+"\n") diff --git a/pkg/workflow/safe_scripts_test.go b/pkg/workflow/safe_scripts_test.go index ccd820965da..be5bf8c3b70 100644 --- a/pkg/workflow/safe_scripts_test.go +++ b/pkg/workflow/safe_scripts_test.go @@ -29,12 +29,10 @@ func TestParseSafeScriptsConfig(t *testing.T) { "type": "string", }, }, - "script": `async function main(config = {}) { - return async function handleSlackPostMessage(message, resolvedTemporaryIds) { - return { success: true }; - }; -} -module.exports = { main };`, + // Users write only the body of main — the compiler wraps it + "script": `return async function handleSlackPostMessage(message, resolvedTemporaryIds) { + return { success: true }; +};`, }, } @@ -47,7 +45,7 @@ module.exports = { main };`, require.True(t, exists, "Should have slack-post-message script") assert.Equal(t, "Post Slack Message", script.Name, "Name should match") assert.Equal(t, "Post a message to a Slack channel", script.Description, "Description should match") - assert.Contains(t, script.Script, "async function main", "Script should contain main function") + assert.Contains(t, script.Script, "return async function", "Script body should contain handler return") require.Len(t, script.Inputs, 2, "Should have 2 inputs") @@ -75,7 +73,8 @@ func TestExtractSafeScriptsFromFrontmatter(t *testing.T) { "scripts": map[string]any{ "my-handler": map[string]any{ "description": "A custom handler", - "script": "module.exports = { main: async (c) => async (m) => ({ success: true }) };", + // Users write only the body — no module.exports or main declaration needed + "script": "return async (m) => ({ success: true });", }, }, }, @@ -108,7 +107,7 @@ func TestBuildCustomSafeOutputScriptsJSON(t *testing.T) { Scripts: map[string]*SafeScriptConfig{ "my-handler": { Description: "Custom handler", - Script: "module.exports = { main: async (c) => async (m) => ({ success: true }) };", + Script: "return async (m) => ({ success: true });", }, }, }, @@ -132,10 +131,10 @@ func TestBuildCustomSafeOutputScriptsJSONNormalization(t *testing.T) { SafeOutputs: &SafeOutputsConfig{ Scripts: map[string]*SafeScriptConfig{ "slack-post-message": { - Script: "module.exports = { main: async (c) => async (m) => ({ success: true }) };", + Script: "return async (m) => ({ success: true });", }, "notify-team": { - Script: "module.exports = { main: async (c) => async (m) => ({ success: true }) };", + Script: "return async (m) => ({ success: true });", }, }, }, @@ -182,7 +181,7 @@ func TestGenerateCustomScriptToolDefinition(t *testing.T) { Type: "string", }, }, - Script: "module.exports = { main: async (c) => async (m) => ({ success: true }) };", + Script: "return async (m) => ({ success: true });", } tool := generateCustomScriptToolDefinition("slack_post_message", scriptConfig) @@ -220,7 +219,7 @@ func TestScriptToolsInFilteredJSON(t *testing.T) { Type: "string", }, }, - Script: "module.exports = { main: async (c) => async (m) => ({ success: true }) };", + Script: "return async (m) => ({ success: true });", }, }, }, @@ -251,11 +250,35 @@ func TestScriptToolsInFilteredJSON(t *testing.T) { assert.Contains(t, properties, "target", "Should have target property") } +// TestGenerateSafeOutputScriptContent verifies that the script body is wrapped in a main function +func TestGenerateSafeOutputScriptContent(t *testing.T) { + body := "return async (msg) => ({ success: true });" + content := generateSafeOutputScriptContent("my-handler", body) + + assert.Contains(t, content, "// Auto-generated safe-output script handler: my-handler", "Should have comment header") + assert.Contains(t, content, "async function main(config = {}) {", "Should wrap body with main function") + assert.Contains(t, content, " return async (msg) => ({ success: true });", "Should indent body by 2 spaces") + assert.Contains(t, content, "module.exports = { main };", "Should include module.exports") + + // Verify the overall structure: header → main() { body } → exports + headerIdx := strings.Index(content, "// Auto-generated") + mainIdx := strings.Index(content, "async function main") + bodyIdx := strings.Index(content, " return async") + closingIdx := strings.Index(content, "\n}\n") + exportsIdx := strings.Index(content, "module.exports") + + assert.Less(t, headerIdx, mainIdx, "Header should precede main") + assert.Less(t, mainIdx, bodyIdx, "main() declaration should precede body") + assert.Less(t, bodyIdx, closingIdx, "Body should precede closing brace") + assert.Less(t, closingIdx, exportsIdx, "Closing brace should precede exports") +} + // TestBuildCustomScriptFilesStep verifies the generated step writes scripts to files func TestBuildCustomScriptFilesStep(t *testing.T) { scripts := map[string]*SafeScriptConfig{ "my-handler": { - Script: "async function main(config) {\n return async (msg) => ({ success: true });\n}\nmodule.exports = { main };", + // Users write only the body — no module.exports or main declaration + Script: "return async (msg) => ({ success: true });", }, } @@ -268,7 +291,11 @@ func TestBuildCustomScriptFilesStep(t *testing.T) { assert.Contains(t, fullYAML, "Setup Safe Output Custom Scripts", "Should have setup step name") assert.Contains(t, fullYAML, "safe_output_script_my_handler.cjs", "Should reference the output filename") assert.Contains(t, fullYAML, "GH_AW_SAFE_OUTPUT_SCRIPT_MY_HANDLER_EOF", "Should use correct heredoc delimiter") - assert.Contains(t, fullYAML, "async function main", "Should include script content") + // Verify the compiler wraps the body with async function main + assert.Contains(t, fullYAML, "async function main(config = {}) {", "Should wrap body with main function declaration") + assert.Contains(t, fullYAML, "module.exports = { main };", "Should include module.exports wrapper") + // Original body content should appear indented inside main + assert.Contains(t, fullYAML, "return async (msg) => ({ success: true });", "Should include user's script body") } // TestBuildCustomScriptFilesStepEmpty verifies nil return for empty scripts @@ -289,7 +316,7 @@ func TestHandlerManagerStepIncludesScriptsEnvVar(t *testing.T) { Scripts: map[string]*SafeScriptConfig{ "my-script": { Description: "Custom script", - Script: "module.exports = { main: async (c) => async (m) => ({ success: true }) };", + Script: "return async (m) => ({ success: true });", }, }, }, @@ -334,7 +361,8 @@ func TestSafeOutputsConfigIncludesScripts(t *testing.T) { "type": "string", }, }, - "script": "module.exports = { main: async (c) => async (m) => ({ success: true }) };", + // Users write only the body + "script": "return async (m) => ({ success: true });", }, }, }, @@ -357,7 +385,7 @@ func TestHasAnySafeOutputEnabledWithScripts(t *testing.T) { config := &SafeOutputsConfig{ Scripts: map[string]*SafeScriptConfig{ "my-script": { - Script: "module.exports = { main: async (c) => async (m) => ({ success: true }) };", + Script: "return async (m) => ({ success: true });", }, }, } @@ -369,7 +397,7 @@ func TestHasNonBuiltinSafeOutputsEnabledWithScripts(t *testing.T) { config := &SafeOutputsConfig{ Scripts: map[string]*SafeScriptConfig{ "my-script": { - Script: "module.exports = { main: async (c) => async (m) => ({ success: true }) };", + Script: "return async (m) => ({ success: true });", }, }, } From 9de4b9333cd485e9095d8a199372e37a57418fa8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 18 Mar 2026 18:10:48 +0000 Subject: [PATCH 4/9] feat: add post-slack-message script to smoke-claude workflow and add scripts to schema Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .github/workflows/smoke-claude.lock.yml | 74 ++++++++++++++++---- .github/workflows/smoke-claude.md | 47 ++++++++++--- pkg/parser/schemas/main_workflow_schema.json | 59 ++++++++++++++++ 3 files changed, 155 insertions(+), 25 deletions(-) diff --git a/.github/workflows/smoke-claude.lock.yml b/.github/workflows/smoke-claude.lock.yml index b113788e9ac..17a4b340699 100644 --- a/.github/workflows/smoke-claude.lock.yml +++ b/.github/workflows/smoke-claude.lock.yml @@ -34,7 +34,7 @@ # # inlined-imports: true # -# gh-aw-metadata: {"schema_version":"v2","frontmatter_hash":"3dae8d9e152c763d93627e8fb4f0afa01809030a9e2d33c38c8451da27f250da","strict":true} +# gh-aw-metadata: {"schema_version":"v2","frontmatter_hash":"c1ff801cd188649e2a6dc97d3e68acad8dca5063b03b2486588a2b8cfab9e4dd","strict":true} name: "Smoke Claude" "on": @@ -498,38 +498,42 @@ jobs: - Write a summary of the results to `/tmp/gh-aw/agent/smoke-claude-status-__GH_AW_GITHUB_RUN_ID__.txt` (create directory if needed) - Use bash to verify the file was created and display its contents + 11. **Slack Script Safe Output Testing**: Use the `post_slack_message` safe-output tool to post a fictitious Slack message: + - Use `channel: "#smoke-tests"` and `message: "💥 Smoke test __GH_AW_GITHUB_RUN_ID__ passed — Claude engine nominal!"` + - Verify the tool call succeeds + ## PR Review Safe Outputs Testing **IMPORTANT**: The following tests require an open pull request. First, use the GitHub MCP tool to find an open PR in __GH_AW_GITHUB_REPOSITORY__ (or use the triggering PR if this is a pull_request event). Store the PR number for use in subsequent tests. - 11. **Update PR Testing**: Use the `update_pull_request` tool to update the PR's body by appending a test message: "✨ PR Review Safe Output Test - Run __GH_AW_GITHUB_RUN_ID__" + 12. **Update PR Testing**: Use the `update_pull_request` tool to update the PR's body by appending a test message: "✨ PR Review Safe Output Test - Run __GH_AW_GITHUB_RUN_ID__" - Use `pr_number: ` to target the open PR - Use `operation: "append"` and `body: "\n\n---\n✨ PR Review Safe Output Test - Run __GH_AW_GITHUB_RUN_ID__"` - Verify the tool call succeeds - 12. **PR Review Comment Testing**: Use the `create_pull_request_review_comment` tool to add review comments on the PR + 13. **PR Review Comment Testing**: Use the `create_pull_request_review_comment` tool to add review comments on the PR - Find a file in the PR's diff (use GitHub MCP to get PR files) - Add at least 2 review comments on different lines with constructive feedback - Use `pr_number: `, `path: ""`, `line: `, and `body: ""` - Verify the tool calls succeed - 13. **Submit PR Review Testing**: Use the `submit_pull_request_review` tool to submit a consolidated review + 14. **Submit PR Review Testing**: Use the `submit_pull_request_review` tool to submit a consolidated review - Use `pr_number: `, `event: "COMMENT"`, and `body: "💥 Automated smoke test review - all systems nominal!"` - Verify the review is submitted successfully - - Note: This will bundle all review comments from test #12 + - Note: This will bundle all review comments from test #13 - 14. **Resolve Review Thread Testing**: + 15. **Resolve Review Thread Testing**: - Use the GitHub MCP tool to list review threads on the PR - If any threads exist, use the `resolve_pull_request_review_thread` tool to resolve one thread - Use `thread_id: ""` from an existing thread - If no threads exist, mark this test as ⚠️ (skipped - no threads to resolve) - 15. **Add Reviewer Testing**: Use the `add_reviewer` tool to add a reviewer to the PR + 16. **Add Reviewer Testing**: Use the `add_reviewer` tool to add a reviewer to the PR - Use `pr_number: ` and `reviewers: ["copilot"]` (or another valid reviewer) - Verify the tool call succeeds - Note: May fail if reviewer is already assigned or doesn't have access - 16. **Push to PR Branch Testing**: + 17. **Push to PR Branch Testing**: - Create a test file at `/tmp/test-pr-push-__GH_AW_GITHUB_RUN_ID__.txt` with content "Test file for PR push" - Use git commands to check if we're on the PR branch - Use the `push_to_pull_request_branch` tool to push this change @@ -537,7 +541,7 @@ jobs: - Verify the push succeeds - Note: This test may be skipped if not on a PR branch or if the PR is from a fork - 17. **Close PR Testing** (CONDITIONAL - only if a test PR exists): + 18. **Close PR Testing** (CONDITIONAL - only if a test PR exists): - If you can identify a test/bot PR that can be safely closed, use the `close_pull_request` tool - Use `pr_number: ` and `comment: "Closing as part of smoke test - Run __GH_AW_GITHUB_RUN_ID__"` - If no suitable test PR exists, mark this test as ⚠️ (skipped - no safe PR to close) @@ -550,7 +554,7 @@ jobs: 1. **ALWAYS create an issue** with a summary of the smoke test run: - Title: "Smoke Test: Claude - __GH_AW_GITHUB_RUN_ID__" - Body should include: - - Test results (✅ for pass, ❌ for fail, ⚠️ for skipped) for each test (including PR review tests #11-17) + - Test results (✅ for pass, ❌ for fail, ⚠️ for skipped) for each test (including PR review tests #12-18) - Overall status: PASS (all passed), PARTIAL (some skipped), or FAIL (any failed) - Run URL: __GH_AW_GITHUB_SERVER_URL__/__GH_AW_GITHUB_REPOSITORY__/actions/runs/__GH_AW_GITHUB_RUN_ID__ - Timestamp @@ -559,8 +563,8 @@ jobs: - This issue MUST be created before any other safe output operations 2. **Only if this workflow was triggered by a pull_request event**: Use the `add_comment` tool to add a **very brief** comment (max 5-10 lines) to the triggering pull request (omit the `item_number` parameter to auto-target the triggering PR) with: - - Test results for core tests #1-10 (✅ or ❌) - - Test results for PR review tests #11-17 (✅, ❌, or ⚠️) + - Test results for core tests #1-11 (✅ or ❌) + - Test results for PR review tests #12-18 (✅, ❌, or ⚠️) - Overall status: PASS, PARTIAL, or FAIL 3. Use the `add_comment` tool with `item_number` set to the discussion number you extracted in step 9 to add a **fun comic-book style comment** to that discussion - be playful and use comic-book language like "💥 WHOOSH!" @@ -856,7 +860,7 @@ jobs: mkdir -p /tmp/gh-aw/safeoutputs mkdir -p /tmp/gh-aw/mcp-logs/safeoutputs cat > ${RUNNER_TEMP}/gh-aw/safeoutputs/config.json << 'GH_AW_SAFE_OUTPUTS_CONFIG_EOF' - {"add_comment":{"max":2},"add_labels":{"allowed":["smoke-claude"],"max":3},"add_reviewer":{"max":2},"close_pull_request":{"max":1,"staged":true},"create_issue":{"expires":2,"group":true,"max":1},"create_pull_request_review_comment":{"max":5},"missing_data":{},"missing_tool":{},"noop":{"max":1},"push_to_pull_request_branch":{"max":1,"target":"*"},"resolve_pull_request_review_thread":{"max":5},"submit_pull_request_review":{"max":1},"update_pull_request":{"max":1}} + {"add_comment":{"max":2},"add_labels":{"allowed":["smoke-claude"],"max":3},"add_reviewer":{"max":2},"close_pull_request":{"max":1,"staged":true},"create_issue":{"expires":2,"group":true,"max":1},"create_pull_request_review_comment":{"max":5},"missing_data":{},"missing_tool":{},"noop":{"max":1},"post_slack_message":{"description":"Post a message to a fictitious Slack channel (smoke test only — no real Slack integration)","inputs":{"channel":{"default":null,"description":"Slack channel name to post to","required":true,"type":"string"},"message":{"default":null,"description":"Message text to post","required":true,"type":"string"}}},"push_to_pull_request_branch":{"max":1,"target":"*"},"resolve_pull_request_review_thread":{"max":5},"submit_pull_request_review":{"max":1},"update_pull_request":{"max":1}} GH_AW_SAFE_OUTPUTS_CONFIG_EOF - name: Write Safe Outputs Tools run: | @@ -874,7 +878,30 @@ jobs: "update_pull_request": " CONSTRAINTS: Maximum 1 pull request(s) can be updated. Target: *." }, "repo_params": {}, - "dynamic_tools": [] + "dynamic_tools": [ + { + "description": "Post a message to a fictitious Slack channel (smoke test only — no real Slack integration)", + "inputSchema": { + "additionalProperties": false, + "properties": { + "channel": { + "description": "Slack channel name to post to", + "type": "string" + }, + "message": { + "description": "Message text to post", + "type": "string" + } + }, + "required": [ + "channel", + "message" + ], + "type": "object" + }, + "name": "post_slack_message" + } + ] } GH_AW_SAFE_OUTPUTS_TOOLS_META_EOF cat > ${RUNNER_TEMP}/gh-aw/safeoutputs/validation.json << 'GH_AW_SAFE_OUTPUTS_VALIDATION_EOF' @@ -2591,6 +2618,24 @@ jobs: GH_HOST="${GITHUB_SERVER_URL#https://}" GH_HOST="${GH_HOST#http://}" echo "GH_HOST=${GH_HOST}" >> "$GITHUB_ENV" + - name: Setup Safe Output Custom Scripts + run: | + cat > ${RUNNER_TEMP}/gh-aw/actions/safe_output_script_post_slack_message.cjs << 'GH_AW_SAFE_OUTPUT_SCRIPT_POST_SLACK_MESSAGE_EOF' + // Auto-generated safe-output script handler: post-slack-message + async function main(config = {}) { + const { channel, message } = config; + return async function handlePostSlackMessage(item) { + const { channel: itemChannel, message: itemMessage } = item.data || {}; + const targetChannel = itemChannel || channel || "#general"; + const text = itemMessage || message || "(no message)"; + core.info(`[FICTITIOUS SLACK] → ${targetChannel}: ${text}`); + return { success: true, channel: targetChannel, message: text }; + }; + + } + module.exports = { main }; + + GH_AW_SAFE_OUTPUT_SCRIPT_POST_SLACK_MESSAGE_EOF - name: Process Safe Outputs id: process_safe_outputs uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 @@ -2599,6 +2644,7 @@ jobs: GH_AW_ALLOWED_DOMAINS: "*.githubusercontent.com,127.0.0.1,::1,anthropic.com,api.anthropic.com,api.github.com,api.snapcraft.io,app.renovatebot.com,appveyor.com,archive.ubuntu.com,azure.archive.ubuntu.com,badgen.net,cdn.playwright.dev,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,files.pythonhosted.org,ghcr.io,github-cloud.githubusercontent.com,github-cloud.s3.amazonaws.com,github.blog,github.com,github.githubassets.com,go.dev,golang.org,goproxy.io,host.docker.internal,img.shields.io,json-schema.org,json.schemastore.org,keyserver.ubuntu.com,lfs.github.com,localhost,mcp.tavily.com,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,packagecloud.io,packages.cloud.google.com,packages.microsoft.com,pkg.go.dev,playwright.download.prss.microsoft.com,ppa.launchpad.net,proxy.golang.org,pypi.org,raw.githubusercontent.com,readthedocs.io,readthedocs.org,registry.npmjs.org,renovatebot.com,s.symcb.com,s.symcd.com,security.ubuntu.com,semaphoreci.com,sentry.io,shields.io,snyk.io,sonarcloud.io,sonarqube.com,statsig.anthropic.com,storage.googleapis.com,sum.golang.org,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_OUTPUT_SCRIPTS: "{\"post_slack_message\":\"safe_output_script_post_slack_message.cjs\"}" GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG: "{\"add_comment\":{\"hide_older_comments\":true,\"max\":2},\"add_labels\":{\"allowed\":[\"smoke-claude\"]},\"add_reviewer\":{\"max\":2,\"target\":\"*\"},\"close_pull_request\":{\"max\":1,\"staged\":true},\"create_issue\":{\"close_older_issues\":true,\"expires\":2,\"group\":true,\"labels\":[\"automation\",\"testing\"],\"max\":1},\"create_pull_request_review_comment\":{\"max\":5,\"side\":\"RIGHT\",\"target\":\"*\"},\"missing_data\":{},\"missing_tool\":{},\"push_to_pull_request_branch\":{\"if_no_changes\":\"warn\",\"max_patch_size\":1024,\"protected_files\":[\"package.json\",\"bun.lockb\",\"bunfig.toml\",\"deno.json\",\"deno.jsonc\",\"deno.lock\",\"global.json\",\"NuGet.Config\",\"Directory.Packages.props\",\"mix.exs\",\"mix.lock\",\"go.mod\",\"go.sum\",\"stack.yaml\",\"stack.yaml.lock\",\"pom.xml\",\"build.gradle\",\"build.gradle.kts\",\"settings.gradle\",\"settings.gradle.kts\",\"gradle.properties\",\"package-lock.json\",\"yarn.lock\",\"pnpm-lock.yaml\",\"npm-shrinkwrap.json\",\"requirements.txt\",\"Pipfile\",\"Pipfile.lock\",\"pyproject.toml\",\"setup.py\",\"setup.cfg\",\"Gemfile\",\"Gemfile.lock\",\"uv.lock\",\"CLAUDE.md\"],\"protected_path_prefixes\":[\".github/\",\".agents/\",\".claude/\"],\"staged\":true,\"target\":\"*\"},\"resolve_pull_request_review_thread\":{\"max\":5},\"submit_pull_request_review\":{\"footer\":\"always\",\"max\":1},\"update_pull_request\":{\"allow_body\":true,\"allow_title\":true,\"max\":1,\"target\":\"*\"}}" GH_AW_CI_TRIGGER_TOKEN: ${{ secrets.GH_AW_CI_TRIGGER_TOKEN }} with: diff --git a/.github/workflows/smoke-claude.md b/.github/workflows/smoke-claude.md index 212bef325e8..c184fe5c39c 100644 --- a/.github/workflows/smoke-claude.md +++ b/.github/workflows/smoke-claude.md @@ -96,6 +96,27 @@ safe-outputs: run-started: "💥 **WHOOSH!** [{workflow_name}]({run_url}) springs into action on this {event_type}! *[Panel 1 begins...]*" run-success: "🎬 **THE END** — [{workflow_name}]({run_url}) **MISSION: ACCOMPLISHED!** The hero saves the day! ✨" run-failure: "💫 **TO BE CONTINUED...** [{workflow_name}]({run_url}) {status}! Our hero faces unexpected challenges..." + scripts: + post-slack-message: + description: Post a message to a fictitious Slack channel (smoke test only — no real Slack integration) + inputs: + channel: + description: Slack channel name to post to + required: true + type: string + message: + description: Message text to post + required: true + type: string + script: | + const { channel, message } = config; + return async function handlePostSlackMessage(item) { + const { channel: itemChannel, message: itemMessage } = item.data || {}; + const targetChannel = itemChannel || channel || "#general"; + const text = itemMessage || message || "(no message)"; + core.info(`[FICTITIOUS SLACK] → ${targetChannel}: ${text}`); + return { success: true, channel: targetChannel, message: text }; + }; timeout-minutes: 10 --- @@ -126,38 +147,42 @@ timeout-minutes: 10 - Write a summary of the results to `/tmp/gh-aw/agent/smoke-claude-status-${{ github.run_id }}.txt` (create directory if needed) - Use bash to verify the file was created and display its contents +11. **Slack Script Safe Output Testing**: Use the `post_slack_message` safe-output tool to post a fictitious Slack message: + - Use `channel: "#smoke-tests"` and `message: "💥 Smoke test ${{ github.run_id }} passed — Claude engine nominal!"` + - Verify the tool call succeeds + ## PR Review Safe Outputs Testing **IMPORTANT**: The following tests require an open pull request. First, use the GitHub MCP tool to find an open PR in ${{ github.repository }} (or use the triggering PR if this is a pull_request event). Store the PR number for use in subsequent tests. -11. **Update PR Testing**: Use the `update_pull_request` tool to update the PR's body by appending a test message: "✨ PR Review Safe Output Test - Run ${{ github.run_id }}" +12. **Update PR Testing**: Use the `update_pull_request` tool to update the PR's body by appending a test message: "✨ PR Review Safe Output Test - Run ${{ github.run_id }}" - Use `pr_number: ` to target the open PR - Use `operation: "append"` and `body: "\n\n---\n✨ PR Review Safe Output Test - Run ${{ github.run_id }}"` - Verify the tool call succeeds -12. **PR Review Comment Testing**: Use the `create_pull_request_review_comment` tool to add review comments on the PR +13. **PR Review Comment Testing**: Use the `create_pull_request_review_comment` tool to add review comments on the PR - Find a file in the PR's diff (use GitHub MCP to get PR files) - Add at least 2 review comments on different lines with constructive feedback - Use `pr_number: `, `path: ""`, `line: `, and `body: ""` - Verify the tool calls succeed -13. **Submit PR Review Testing**: Use the `submit_pull_request_review` tool to submit a consolidated review +14. **Submit PR Review Testing**: Use the `submit_pull_request_review` tool to submit a consolidated review - Use `pr_number: `, `event: "COMMENT"`, and `body: "💥 Automated smoke test review - all systems nominal!"` - Verify the review is submitted successfully - - Note: This will bundle all review comments from test #12 + - Note: This will bundle all review comments from test #13 -14. **Resolve Review Thread Testing**: +15. **Resolve Review Thread Testing**: - Use the GitHub MCP tool to list review threads on the PR - If any threads exist, use the `resolve_pull_request_review_thread` tool to resolve one thread - Use `thread_id: ""` from an existing thread - If no threads exist, mark this test as ⚠️ (skipped - no threads to resolve) -15. **Add Reviewer Testing**: Use the `add_reviewer` tool to add a reviewer to the PR +16. **Add Reviewer Testing**: Use the `add_reviewer` tool to add a reviewer to the PR - Use `pr_number: ` and `reviewers: ["copilot"]` (or another valid reviewer) - Verify the tool call succeeds - Note: May fail if reviewer is already assigned or doesn't have access -16. **Push to PR Branch Testing**: +17. **Push to PR Branch Testing**: - Create a test file at `/tmp/test-pr-push-${{ github.run_id }}.txt` with content "Test file for PR push" - Use git commands to check if we're on the PR branch - Use the `push_to_pull_request_branch` tool to push this change @@ -165,7 +190,7 @@ timeout-minutes: 10 - Verify the push succeeds - Note: This test may be skipped if not on a PR branch or if the PR is from a fork -17. **Close PR Testing** (CONDITIONAL - only if a test PR exists): +18. **Close PR Testing** (CONDITIONAL - only if a test PR exists): - If you can identify a test/bot PR that can be safely closed, use the `close_pull_request` tool - Use `pr_number: ` and `comment: "Closing as part of smoke test - Run ${{ github.run_id }}"` - If no suitable test PR exists, mark this test as ⚠️ (skipped - no safe PR to close) @@ -178,7 +203,7 @@ timeout-minutes: 10 1. **ALWAYS create an issue** with a summary of the smoke test run: - Title: "Smoke Test: Claude - ${{ github.run_id }}" - Body should include: - - Test results (✅ for pass, ❌ for fail, ⚠️ for skipped) for each test (including PR review tests #11-17) + - Test results (✅ for pass, ❌ for fail, ⚠️ for skipped) for each test (including PR review tests #12-18) - Overall status: PASS (all passed), PARTIAL (some skipped), or FAIL (any failed) - Run URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} - Timestamp @@ -187,8 +212,8 @@ timeout-minutes: 10 - This issue MUST be created before any other safe output operations 2. **Only if this workflow was triggered by a pull_request event**: Use the `add_comment` tool to add a **very brief** comment (max 5-10 lines) to the triggering pull request (omit the `item_number` parameter to auto-target the triggering PR) with: - - Test results for core tests #1-10 (✅ or ❌) - - Test results for PR review tests #11-17 (✅, ❌, or ⚠️) + - Test results for core tests #1-11 (✅ or ❌) + - Test results for PR review tests #12-18 (✅, ❌, or ⚠️) - Overall status: PASS, PARTIAL, or FAIL 3. Use the `add_comment` tool with `item_number` set to the discussion number you extracted in step 9 to add a **fun comic-book style comment** to that discussion - be playful and use comic-book language like "💥 WHOOSH!" diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index c5713c64db9..fa4777cfc81 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -7529,6 +7529,65 @@ }, "additionalProperties": false }, + "scripts": { + "type": "object", + "description": "Inline JavaScript script handlers that run inside the consolidated safe-outputs job handler loop. Unlike 'jobs' (which create separate GitHub Actions jobs), scripts execute in-process alongside the built-in handlers. Users write only the body of the main function — the compiler wraps it with 'async function main(config = {}) { ... }' and 'module.exports = { main };' automatically. Script names containing dashes will be automatically normalized to underscores (e.g., 'post-slack-message' becomes 'post_slack_message').", + "patternProperties": { + "^[a-zA-Z_][a-zA-Z0-9_-]*$": { + "type": "object", + "description": "Inline script handler configuration. The script body has access to 'config' (the handler config object) and should return an async function that processes individual safe-output messages.", + "properties": { + "name": { + "type": "string", + "description": "Display name for the script handler" + }, + "description": { + "type": "string", + "description": "Description of the script handler (used in MCP tool registration)" + }, + "inputs": { + "type": "object", + "description": "Input parameters for the script handler", + "patternProperties": { + "^[a-zA-Z_][a-zA-Z0-9_-]*$": { + "type": "object", + "properties": { + "description": { + "type": "string", + "description": "Input parameter description" + }, + "required": { + "type": "boolean", + "description": "Whether this input is required", + "default": false + }, + "default": { + "description": "Default value for the input", + "oneOf": [{ "type": "string" }, { "type": "boolean" }, { "type": "number" }] + }, + "type": { + "type": "string", + "enum": ["string", "boolean", "number"], + "description": "Input parameter type", + "default": "string" + } + }, + "additionalProperties": false + } + }, + "additionalProperties": false + }, + "script": { + "type": "string", + "description": "JavaScript body of the main function. Write only the body — the compiler wraps it with 'async function main(config = {}) { ... }' and 'module.exports = { main };'. The body has access to 'config' (the handler config) and should return an async message handler function." + } + }, + "required": ["script"], + "additionalProperties": false + } + }, + "additionalProperties": false + }, "messages": { "type": "object", "description": "Custom message templates for safe-output footer and notification messages. Available placeholders: {workflow_name} (workflow name), {run_url} (GitHub Actions run URL), {triggering_number} (issue/PR/discussion number), {workflow_source} (owner/repo/path@ref), {workflow_source_url} (GitHub URL to source), {operation} (safe-output operation name for staged mode).", From 6ca853fae9af8277bbb6341bd58bc62617318fc8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 18 Mar 2026 19:20:04 +0000 Subject: [PATCH 5/9] feat: add safe-output-script.d.ts TypeScript definitions and fix smoke-claude script body Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .github/workflows/smoke-claude.lock.yml | 8 +- .github/workflows/smoke-claude.md | 6 +- .../setup/js/types/safe-output-script.d.ts | 165 ++++++++++++++++++ 3 files changed, 170 insertions(+), 9 deletions(-) create mode 100644 actions/setup/js/types/safe-output-script.d.ts diff --git a/.github/workflows/smoke-claude.lock.yml b/.github/workflows/smoke-claude.lock.yml index 17a4b340699..b0579fbb8e7 100644 --- a/.github/workflows/smoke-claude.lock.yml +++ b/.github/workflows/smoke-claude.lock.yml @@ -34,7 +34,7 @@ # # inlined-imports: true # -# gh-aw-metadata: {"schema_version":"v2","frontmatter_hash":"c1ff801cd188649e2a6dc97d3e68acad8dca5063b03b2486588a2b8cfab9e4dd","strict":true} +# gh-aw-metadata: {"schema_version":"v2","frontmatter_hash":"df5b2e20728c1a3f7e64335f2fc80cb2f3ce98df51b13ab4929a209e733b639b","strict":true} name: "Smoke Claude" "on": @@ -2623,11 +2623,9 @@ jobs: cat > ${RUNNER_TEMP}/gh-aw/actions/safe_output_script_post_slack_message.cjs << 'GH_AW_SAFE_OUTPUT_SCRIPT_POST_SLACK_MESSAGE_EOF' // Auto-generated safe-output script handler: post-slack-message async function main(config = {}) { - const { channel, message } = config; return async function handlePostSlackMessage(item) { - const { channel: itemChannel, message: itemMessage } = item.data || {}; - const targetChannel = itemChannel || channel || "#general"; - const text = itemMessage || message || "(no message)"; + const targetChannel = item.channel || "#general"; + const text = item.message || "(no message)"; core.info(`[FICTITIOUS SLACK] → ${targetChannel}: ${text}`); return { success: true, channel: targetChannel, message: text }; }; diff --git a/.github/workflows/smoke-claude.md b/.github/workflows/smoke-claude.md index c184fe5c39c..5798f5626f1 100644 --- a/.github/workflows/smoke-claude.md +++ b/.github/workflows/smoke-claude.md @@ -109,11 +109,9 @@ safe-outputs: required: true type: string script: | - const { channel, message } = config; return async function handlePostSlackMessage(item) { - const { channel: itemChannel, message: itemMessage } = item.data || {}; - const targetChannel = itemChannel || channel || "#general"; - const text = itemMessage || message || "(no message)"; + const targetChannel = item.channel || "#general"; + const text = item.message || "(no message)"; core.info(`[FICTITIOUS SLACK] → ${targetChannel}: ${text}`); return { success: true, channel: targetChannel, message: text }; }; diff --git a/actions/setup/js/types/safe-output-script.d.ts b/actions/setup/js/types/safe-output-script.d.ts new file mode 100644 index 00000000000..2198dc05ea1 --- /dev/null +++ b/actions/setup/js/types/safe-output-script.d.ts @@ -0,0 +1,165 @@ +// TypeScript definitions for GitHub Agentic Workflows Safe Output Script Handlers +// This file describes the types available when writing a custom safe-output script +// (defined under safe-outputs.scripts in workflow frontmatter). +// +// Usage in a script body (only the body of main() is written by the user): +// +// const { inputs } = config; +// return async function handleMyScript(item) { +// const channel = item.channel ?? inputs?.channel?.default ?? "#general"; +// core.info(`[SLACK] → ${channel}: ${item.message}`); +// return { success: true }; +// }; + +import type { HandlerResult } from "./handler-factory"; +export type { HandlerResult }; + +// ── Input-definition types ────────────────────────────────────────────────── + +/** + * The definition of a single user-declared input from the YAML `inputs:` section. + * These definitions are available at runtime through `config.inputs`. + */ +export interface SafeOutputScriptInputDefinition { + /** The declared type of this input ("string" | "boolean" | "number"). */ + type?: "string" | "boolean" | "number"; + /** Human-readable description shown in MCP tool registration. */ + description?: string; + /** Whether the caller is required to supply a value for this input. */ + required?: boolean; + /** + * The default value to use when the caller omits the input. + * `null` means no default was specified. + */ + default?: string | boolean | number | null; + /** Available options when `type` is "string" (choice constraint). */ + options?: string[]; +} + +// ── Config type ───────────────────────────────────────────────────────────── + +/** + * The `config` object passed to the `main()` factory function of a + * custom safe-output script. + * + * This contains the **static** YAML configuration for the script — the + * description and the input-definition metadata. The actual per-call input + * values sent by the agent are exposed as direct properties on the `item` + * object inside the handler function (not here). + * + * @example + * ```javascript + * // config.inputs.channel.required === true + * // config.inputs.channel.type === "string" + * const { inputs } = config; + * return async function handleMyScript(item) { + * const ch = item.channel ?? inputs?.channel?.default ?? "#general"; + * return { success: true }; + * }; + * ``` + */ +export interface SafeOutputScriptConfig { + /** + * Human-readable description of this script (from `description:` in YAML). + * Used in MCP tool registration. + */ + description?: string; + /** + * Metadata for each declared input. + * Keys are input names as declared in the YAML `inputs:` section. + * **Note**: This is the *schema* for each input (type, description, required, default), + * not the runtime values. Use `item.` inside the handler to access values. + */ + inputs?: Record; +} + +// ── Per-call message type ─────────────────────────────────────────────────── + +/** + * The per-call message object passed to the handler function returned by `main()`. + * + * For custom safe-output scripts the agent sends a JSONL line like: + * ```json + * { "type": "post_slack_message", "channel": "#general", "message": "Hello" } + * ``` + * All user-declared input values are properties at the **top level** of this + * object (not nested under `.data`). + * + * @typeParam TInputs - The shape of the user-declared inputs. When omitted the + * properties are typed as `unknown` and can be narrowed at runtime. + * + * @example + * ```typescript + * // With explicit input types: + * type SlackInputs = { channel?: string; message?: string }; + * return async function handleSlack(item: SafeOutputScriptItem) { + * core.info(`channel: ${item.channel ?? "#general"}`); + * }; + * ``` + */ +export type SafeOutputScriptItem = Record> = { + /** The safe-output type identifier (normalized script name, e.g. "post_slack_message"). */ + type: string; + /** Optional secrecy level of the message content (e.g. "public", "internal", "private"). */ + secrecy?: string; + /** Optional integrity level of the message source (e.g. "low", "medium", "high"). */ + integrity?: string; +} & TInputs; + +// ── Resolved temporary IDs ────────────────────────────────────────────────── + +/** + * Map of temporary IDs to their resolved GitHub issue/PR/discussion references. + * Passed as the second argument to the handler function. + */ +export interface ResolvedTemporaryIds { + [temporaryId: string]: { + /** Repository in "owner/repo" format. */ + repo: string; + /** Issue, PR, or discussion number. */ + number: number; + }; +} + +// ── Handler and factory function types ───────────────────────────────────── + +/** + * The async message-handler function returned by `main()`. + * Receives a single safe-output message and should return a `HandlerResult`. + * + * @typeParam TInputs - The shape of the user-declared inputs (defaults to + * `Record`). + */ +export type SafeOutputScriptHandler = Record> = (item: SafeOutputScriptItem, resolvedTemporaryIds: ResolvedTemporaryIds) => Promise; + +/** + * The type of the `main()` function generated by the compiler around the user's + * script body. + * + * The compiler wraps the user-written body in: + * ```javascript + * async function main(config = {}) { + * // + * } + * module.exports = { main }; + * ``` + * + * The `main` function receives the static YAML configuration and returns an + * async handler function that processes individual messages. + * + * @typeParam TInputs - The shape of the user-declared inputs. + */ +export type SafeOutputScriptMain = Record> = (config: SafeOutputScriptConfig) => Promise>; + +// ── Globals available in the script body ──────────────────────────────────── +// The globals below are injected by the `actions/github-script` environment +// that hosts the handler manager. They are already declared in +// `github-script.d.ts`; this comment serves as a quick reference. +// +// github — authenticated Octokit instance +// context — GitHub Actions workflow run context +// core — @actions/core (setOutput, info, warning, error, …) +// exec — @actions/exec +// glob — @actions/glob +// io — @actions/io +// require — CommonJS require (supports relative paths and npm packages) From 774eec7f463ff64ade5d5eae3a7e867916d77e8c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 18 Mar 2026 20:52:57 +0000 Subject: [PATCH 6/9] fix: add export declare function main to d.ts and ts-check to generated wrapper Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .github/workflows/smoke-claude.lock.yml | 4 ++++ actions/setup/js/types/safe-output-script.d.ts | 9 +++++++++ pkg/workflow/compiler_safe_outputs_job.go | 5 ++++- pkg/workflow/safe_scripts_test.go | 3 +++ 4 files changed, 20 insertions(+), 1 deletion(-) diff --git a/.github/workflows/smoke-claude.lock.yml b/.github/workflows/smoke-claude.lock.yml index b0579fbb8e7..e878b9ae305 100644 --- a/.github/workflows/smoke-claude.lock.yml +++ b/.github/workflows/smoke-claude.lock.yml @@ -2621,7 +2621,11 @@ jobs: - name: Setup Safe Output Custom Scripts run: | cat > ${RUNNER_TEMP}/gh-aw/actions/safe_output_script_post_slack_message.cjs << 'GH_AW_SAFE_OUTPUT_SCRIPT_POST_SLACK_MESSAGE_EOF' + // @ts-check + /// // Auto-generated safe-output script handler: post-slack-message + + /** @type {import('./types/safe-output-script').SafeOutputScriptMain} */ async function main(config = {}) { return async function handlePostSlackMessage(item) { const targetChannel = item.channel || "#general"; diff --git a/actions/setup/js/types/safe-output-script.d.ts b/actions/setup/js/types/safe-output-script.d.ts index 2198dc05ea1..aa59b33e1a6 100644 --- a/actions/setup/js/types/safe-output-script.d.ts +++ b/actions/setup/js/types/safe-output-script.d.ts @@ -151,6 +151,15 @@ export type SafeOutputScriptHandler = Re */ export type SafeOutputScriptMain = Record> = (config: SafeOutputScriptConfig) => Promise>; +/** + * The `main` factory function exported by every auto-generated safe-output + * script module (`module.exports = { main }`). + * + * This TypeScript declaration provides IDE type-checking support for the + * CommonJS export (`module.exports = { main }`) that the compiler generates. + */ +export declare function main(config: SafeOutputScriptConfig): Promise; + // ── Globals available in the script body ──────────────────────────────────── // The globals below are injected by the `actions/github-script` environment // that hosts the handler manager. They are already declared in diff --git a/pkg/workflow/compiler_safe_outputs_job.go b/pkg/workflow/compiler_safe_outputs_job.go index b7b65ac5e59..cc61be18af2 100644 --- a/pkg/workflow/compiler_safe_outputs_job.go +++ b/pkg/workflow/compiler_safe_outputs_job.go @@ -520,7 +520,10 @@ func buildSafeOutputItemsManifestUploadStep(prefix string) []string { // used by MCP-scripts and actions/github-script. func generateSafeOutputScriptContent(scriptName string, scriptBody string) string { var sb strings.Builder - sb.WriteString("// Auto-generated safe-output script handler: " + scriptName + "\n") + sb.WriteString("// @ts-check\n") + sb.WriteString("/// \n") + sb.WriteString("// Auto-generated safe-output script handler: " + scriptName + "\n\n") + sb.WriteString("/** @type {import('./types/safe-output-script').SafeOutputScriptMain} */\n") sb.WriteString("async function main(config = {}) {\n") // Indent each line of the user's body by 2 spaces for line := range strings.SplitSeq(scriptBody, "\n") { diff --git a/pkg/workflow/safe_scripts_test.go b/pkg/workflow/safe_scripts_test.go index be5bf8c3b70..45c90a61155 100644 --- a/pkg/workflow/safe_scripts_test.go +++ b/pkg/workflow/safe_scripts_test.go @@ -255,6 +255,9 @@ func TestGenerateSafeOutputScriptContent(t *testing.T) { body := "return async (msg) => ({ success: true });" content := generateSafeOutputScriptContent("my-handler", body) + assert.Contains(t, content, "// @ts-check", "Should include ts-check pragma") + assert.Contains(t, content, "/// ", "Should include type reference") + assert.Contains(t, content, "/** @type {import('./types/safe-output-script').SafeOutputScriptMain} */", "Should include type annotation for main") assert.Contains(t, content, "// Auto-generated safe-output script handler: my-handler", "Should have comment header") assert.Contains(t, content, "async function main(config = {}) {", "Should wrap body with main function") assert.Contains(t, content, " return async (msg) => ({ success: true });", "Should indent body by 2 spaces") From a6766d14744df7394a962c5b576cbb134fe14cb0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 18 Mar 2026 22:02:45 +0000 Subject: [PATCH 7/9] feat: generate handler wrapper and config destructuring from script inputs Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .github/workflows/smoke-claude.lock.yml | 7 +- .github/workflows/smoke-claude.md | 10 +-- .../setup/js/types/safe-output-script.d.ts | 16 ++-- pkg/parser/schemas/main_workflow_schema.json | 2 +- pkg/workflow/compiler_safe_outputs_job.go | 69 ++++++++++++--- pkg/workflow/safe_scripts_test.go | 84 +++++++++++++++---- 6 files changed, 145 insertions(+), 43 deletions(-) diff --git a/.github/workflows/smoke-claude.lock.yml b/.github/workflows/smoke-claude.lock.yml index e878b9ae305..df13230dfb5 100644 --- a/.github/workflows/smoke-claude.lock.yml +++ b/.github/workflows/smoke-claude.lock.yml @@ -34,7 +34,7 @@ # # inlined-imports: true # -# gh-aw-metadata: {"schema_version":"v2","frontmatter_hash":"df5b2e20728c1a3f7e64335f2fc80cb2f3ce98df51b13ab4929a209e733b639b","strict":true} +# gh-aw-metadata: {"schema_version":"v2","frontmatter_hash":"9a4d79294c209d0051ffe00014b288b54a2be522a7908fd96f44fab6aa5c9e60","strict":true} name: "Smoke Claude" "on": @@ -2627,13 +2627,14 @@ jobs: /** @type {import('./types/safe-output-script').SafeOutputScriptMain} */ async function main(config = {}) { - return async function handlePostSlackMessage(item) { + const { channel, message } = config; + return async function handlePostSlackMessage(item, resolvedTemporaryIds) { const targetChannel = item.channel || "#general"; const text = item.message || "(no message)"; core.info(`[FICTITIOUS SLACK] → ${targetChannel}: ${text}`); return { success: true, channel: targetChannel, message: text }; + }; - } module.exports = { main }; diff --git a/.github/workflows/smoke-claude.md b/.github/workflows/smoke-claude.md index 5798f5626f1..20c974ecf24 100644 --- a/.github/workflows/smoke-claude.md +++ b/.github/workflows/smoke-claude.md @@ -109,12 +109,10 @@ safe-outputs: required: true type: string script: | - return async function handlePostSlackMessage(item) { - const targetChannel = item.channel || "#general"; - const text = item.message || "(no message)"; - core.info(`[FICTITIOUS SLACK] → ${targetChannel}: ${text}`); - return { success: true, channel: targetChannel, message: text }; - }; + const targetChannel = item.channel || "#general"; + const text = item.message || "(no message)"; + core.info(`[FICTITIOUS SLACK] → ${targetChannel}: ${text}`); + return { success: true, channel: targetChannel, message: text }; timeout-minutes: 10 --- diff --git a/actions/setup/js/types/safe-output-script.d.ts b/actions/setup/js/types/safe-output-script.d.ts index aa59b33e1a6..eb53c5d1a92 100644 --- a/actions/setup/js/types/safe-output-script.d.ts +++ b/actions/setup/js/types/safe-output-script.d.ts @@ -134,18 +134,24 @@ export type SafeOutputScriptHandler = Re /** * The type of the `main()` function generated by the compiler around the user's - * script body. + * handler body. * - * The compiler wraps the user-written body in: + * The compiler generates the full outer wrapper from the user's declared inputs + * and script body: * ```javascript * async function main(config = {}) { - * // + * const { channel, message } = config; // auto-generated from declared inputs + * return async function handleX(item, resolvedTemporaryIds) { + * // + * }; * } * module.exports = { main }; * ``` * - * The `main` function receives the static YAML configuration and returns an - * async handler function that processes individual messages. + * The `main` function receives the static YAML configuration (including input + * defaults) and returns an async handler function that processes individual + * messages. Users write only the handler body — the outer structure is + * generated automatically. * * @typeParam TInputs - The shape of the user-declared inputs. */ diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index fa4777cfc81..bfe80422731 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -7579,7 +7579,7 @@ }, "script": { "type": "string", - "description": "JavaScript body of the main function. Write only the body — the compiler wraps it with 'async function main(config = {}) { ... }' and 'module.exports = { main };'. The body has access to 'config' (the handler config) and should return an async message handler function." + "description": "JavaScript handler body. Write only the code that runs inside the handler for each item — the compiler generates the full outer wrapper including config input destructuring (`const { channel, message } = config;`) and the handler function (`return async function handleX(item, resolvedTemporaryIds) { ... }`). The body has access to `item` (runtime message with input values), `resolvedTemporaryIds` (map of temporary IDs), and config-destructured local variables for each declared input." } }, "required": ["script"], diff --git a/pkg/workflow/compiler_safe_outputs_job.go b/pkg/workflow/compiler_safe_outputs_job.go index cc61be18af2..2fd21977176 100644 --- a/pkg/workflow/compiler_safe_outputs_job.go +++ b/pkg/workflow/compiler_safe_outputs_job.go @@ -514,21 +514,68 @@ func buildSafeOutputItemsManifestUploadStep(prefix string) []string { } } -// generateSafeOutputScriptContent wraps the user's script body in a main function template. -// Users write only the body of the main function (the handler setup code and return statement), -// and the compiler wraps it with the function declaration and module.exports — the same pattern -// used by MCP-scripts and actions/github-script. -func generateSafeOutputScriptContent(scriptName string, scriptBody string) string { +// scriptNameToHandlerName converts a script name like "post-slack-message" to a +// JavaScript function name like "handlePostSlackMessage". +func scriptNameToHandlerName(scriptName string) string { + parts := strings.FieldsFunc(scriptName, func(r rune) bool { + return r == '-' || r == '_' + }) + var sb strings.Builder + sb.WriteString("handle") + for _, part := range parts { + if len(part) > 0 { + sb.WriteString(strings.ToUpper(part[:1]) + part[1:]) + } + } + if sb.Len() == len("handle") { + // Fallback: use the script name as-is when parts are empty + if len(scriptName) == 0 { + sb.WriteString("Unknown") + } else { + sb.WriteString(strings.ToUpper(scriptName[:1]) + scriptName[1:]) + } + } + return sb.String() +} + +// generateSafeOutputScriptContent generates a complete JavaScript module for a custom safe-output +// script handler. Users write only the handler body (the code that runs inside the async handler +// function for each item), and the compiler generates the full outer wrapper including: +// - Config input destructuring: const { channel, message } = config; +// - Handler function: return async function handleX(item, resolvedTemporaryIds) { ... } +// - The module.exports boilerplate +func generateSafeOutputScriptContent(scriptName string, scriptConfig *SafeScriptConfig) string { var sb strings.Builder sb.WriteString("// @ts-check\n") sb.WriteString("/// \n") sb.WriteString("// Auto-generated safe-output script handler: " + scriptName + "\n\n") sb.WriteString("/** @type {import('./types/safe-output-script').SafeOutputScriptMain} */\n") sb.WriteString("async function main(config = {}) {\n") - // Indent each line of the user's body by 2 spaces - for line := range strings.SplitSeq(scriptBody, "\n") { - sb.WriteString(" " + line + "\n") + + // Auto-destructure all declared input names from config (provides access to + // static YAML config values such as defaults). + if len(scriptConfig.Inputs) > 0 { + inputNames := make([]string, 0, len(scriptConfig.Inputs)) + for name := range scriptConfig.Inputs { + safeName := stringutil.SanitizeParameterName(name) + if safeName != name { + inputNames = append(inputNames, name+": "+safeName) + } else { + inputNames = append(inputNames, name) + } + } + sort.Strings(inputNames) + sb.WriteString(" const { " + strings.Join(inputNames, ", ") + " } = config;\n") + } + + // Generate the handler function that receives each item at runtime. + handlerName := scriptNameToHandlerName(scriptName) + sb.WriteString(" return async function " + handlerName + "(item, resolvedTemporaryIds) {\n") + // Indent each line of the user's handler body by 4 spaces + for line := range strings.SplitSeq(scriptConfig.Script, "\n") { + sb.WriteString(" " + line + "\n") } + sb.WriteString(" };\n") sb.WriteString("}\n") sb.WriteString("module.exports = { main };\n") return sb.String() @@ -536,8 +583,8 @@ func generateSafeOutputScriptContent(scriptName string, scriptBody string) strin // buildCustomScriptFilesStep generates a run step that writes inline safe-output script files // to the setup action destination folder so they can be required by the handler manager. -// The user's script body is automatically wrapped with async function main(config = {}) { ... } -// and module.exports = { main }; — users write only the handler body, not the boilerplate. +// Users write only the handler body; the compiler wraps it with config destructuring, +// the handler function, and module.exports boilerplate. // Each script is written using a heredoc to avoid shell quoting issues. func buildCustomScriptFilesStep(scripts map[string]*SafeScriptConfig) []string { if len(scripts) == 0 { @@ -561,7 +608,7 @@ func buildCustomScriptFilesStep(scripts map[string]*SafeScriptConfig) []string { filename := safeOutputScriptFilename(normalizedName) filePath := SetupActionDestinationShell + "/" + filename delimiter := GenerateHeredocDelimiter("SAFE_OUTPUT_SCRIPT_" + strings.ToUpper(normalizedName)) - scriptContent := generateSafeOutputScriptContent(scriptName, scriptConfig.Script) + scriptContent := generateSafeOutputScriptContent(scriptName, scriptConfig) steps = append(steps, fmt.Sprintf(" cat > %s << '%s'\n", filePath, delimiter)) for line := range strings.SplitSeq(scriptContent, "\n") { diff --git a/pkg/workflow/safe_scripts_test.go b/pkg/workflow/safe_scripts_test.go index 45c90a61155..aca51a879e8 100644 --- a/pkg/workflow/safe_scripts_test.go +++ b/pkg/workflow/safe_scripts_test.go @@ -250,38 +250,86 @@ func TestScriptToolsInFilteredJSON(t *testing.T) { assert.Contains(t, properties, "target", "Should have target property") } -// TestGenerateSafeOutputScriptContent verifies that the script body is wrapped in a main function +// TestGenerateSafeOutputScriptContent verifies that the handler body is wrapped with config +// destructuring and a handler function — users write only the handler body. func TestGenerateSafeOutputScriptContent(t *testing.T) { - body := "return async (msg) => ({ success: true });" - content := generateSafeOutputScriptContent("my-handler", body) + scriptConfig := &SafeScriptConfig{ + Script: "core.info(`Channel: ${item.channel}`); return { success: true };", + Inputs: map[string]*InputDefinition{ + "channel": {Type: "string", Description: "Target channel"}, + "message": {Type: "string", Description: "Message text"}, + }, + } + content := generateSafeOutputScriptContent("my-handler", scriptConfig) assert.Contains(t, content, "// @ts-check", "Should include ts-check pragma") assert.Contains(t, content, "/// ", "Should include type reference") assert.Contains(t, content, "/** @type {import('./types/safe-output-script').SafeOutputScriptMain} */", "Should include type annotation for main") assert.Contains(t, content, "// Auto-generated safe-output script handler: my-handler", "Should have comment header") - assert.Contains(t, content, "async function main(config = {}) {", "Should wrap body with main function") - assert.Contains(t, content, " return async (msg) => ({ success: true });", "Should indent body by 2 spaces") + assert.Contains(t, content, "async function main(config = {}) {", "Should wrap with main function") + assert.Contains(t, content, "const { channel, message } = config;", "Should destructure config inputs") + assert.Contains(t, content, "return async function handleMyHandler(item, resolvedTemporaryIds) {", "Should generate handler function") + assert.Contains(t, content, " core.info", "Should indent user body by 4 spaces") assert.Contains(t, content, "module.exports = { main };", "Should include module.exports") - // Verify the overall structure: header → main() { body } → exports + // Verify the overall structure: + // header → main() { destructuring → handler { body } } → exports headerIdx := strings.Index(content, "// Auto-generated") mainIdx := strings.Index(content, "async function main") - bodyIdx := strings.Index(content, " return async") - closingIdx := strings.Index(content, "\n}\n") + destructureIdx := strings.Index(content, "const { channel") + handlerIdx := strings.Index(content, "return async function handle") + bodyIdx := strings.Index(content, " core.info") exportsIdx := strings.Index(content, "module.exports") assert.Less(t, headerIdx, mainIdx, "Header should precede main") - assert.Less(t, mainIdx, bodyIdx, "main() declaration should precede body") - assert.Less(t, bodyIdx, closingIdx, "Body should precede closing brace") - assert.Less(t, closingIdx, exportsIdx, "Closing brace should precede exports") + assert.Less(t, mainIdx, destructureIdx, "main() should precede config destructuring") + assert.Less(t, destructureIdx, handlerIdx, "Config destructuring should precede handler function") + assert.Less(t, handlerIdx, bodyIdx, "Handler function should precede user body") + assert.Less(t, bodyIdx, exportsIdx, "User body should precede exports") +} + +// TestGenerateSafeOutputScriptContentNoInputs verifies output without declared inputs (no destructuring) +func TestGenerateSafeOutputScriptContentNoInputs(t *testing.T) { + scriptConfig := &SafeScriptConfig{ + Script: "return { success: true };", + } + content := generateSafeOutputScriptContent("simple-handler", scriptConfig) + + assert.NotContains(t, content, "const {", "Should not destructure when no inputs declared") + assert.Contains(t, content, "return async function handleSimpleHandler(item, resolvedTemporaryIds) {", "Should still generate handler function") + assert.Contains(t, content, " return { success: true };", "Should indent user body by 4 spaces") +} + +// TestScriptNameToHandlerName verifies handler name generation from script names +func TestScriptNameToHandlerName(t *testing.T) { + tests := []struct { + name string + input string + expected string + }{ + {"hyphen-separated", "post-slack-message", "handlePostSlackMessage"}, + {"underscore-separated", "post_slack_message", "handlePostSlackMessage"}, + {"mixed", "my-handler_name", "handleMyHandlerName"}, + {"single-word", "handler", "handleHandler"}, + {"camelcase-word", "createIssue", "handleCreateIssue"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := scriptNameToHandlerName(tt.input) + assert.Equal(t, tt.expected, result, "Handler name should match expected") + }) + } } // TestBuildCustomScriptFilesStep verifies the generated step writes scripts to files func TestBuildCustomScriptFilesStep(t *testing.T) { scripts := map[string]*SafeScriptConfig{ "my-handler": { - // Users write only the body — no module.exports or main declaration - Script: "return async (msg) => ({ success: true });", + // Users write only the handler body — no function wrapper or boilerplate needed + Script: "return { success: true };", + Inputs: map[string]*InputDefinition{ + "channel": {Type: "string"}, + }, }, } @@ -294,11 +342,13 @@ func TestBuildCustomScriptFilesStep(t *testing.T) { assert.Contains(t, fullYAML, "Setup Safe Output Custom Scripts", "Should have setup step name") assert.Contains(t, fullYAML, "safe_output_script_my_handler.cjs", "Should reference the output filename") assert.Contains(t, fullYAML, "GH_AW_SAFE_OUTPUT_SCRIPT_MY_HANDLER_EOF", "Should use correct heredoc delimiter") - // Verify the compiler wraps the body with async function main - assert.Contains(t, fullYAML, "async function main(config = {}) {", "Should wrap body with main function declaration") + // Verify the compiler generates the full outer wrapper + assert.Contains(t, fullYAML, "async function main(config = {}) {", "Should generate main function declaration") + assert.Contains(t, fullYAML, "const { channel } = config;", "Should generate config input destructuring") + assert.Contains(t, fullYAML, "return async function handleMyHandler(item, resolvedTemporaryIds) {", "Should generate handler function") assert.Contains(t, fullYAML, "module.exports = { main };", "Should include module.exports wrapper") - // Original body content should appear indented inside main - assert.Contains(t, fullYAML, "return async (msg) => ({ success: true });", "Should include user's script body") + // User's handler body content should appear indented inside the handler + assert.Contains(t, fullYAML, "return { success: true };", "Should include user's handler body") } // TestBuildCustomScriptFilesStepEmpty verifies nil return for empty scripts From 2113a5737229c24345e1e35e57899a4999fffd5d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 18 Mar 2026 23:38:55 +0000 Subject: [PATCH 8/9] feat: update safe-output-script.d.ts, add temporaryIdMap to handler, non-fatal script errors Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .github/workflows/smoke-claude.lock.yml | 4 +- .../setup/js/safe_output_handler_manager.cjs | 22 ++--- .../setup/js/types/safe-output-script.d.ts | 83 +++++++++++++++---- pkg/workflow/compiler_safe_outputs_job.go | 2 +- pkg/workflow/safe_scripts_test.go | 6 +- 5 files changed, 83 insertions(+), 34 deletions(-) diff --git a/.github/workflows/smoke-claude.lock.yml b/.github/workflows/smoke-claude.lock.yml index 9d8cfc11c1e..ca12d7a0ae3 100644 --- a/.github/workflows/smoke-claude.lock.yml +++ b/.github/workflows/smoke-claude.lock.yml @@ -2632,7 +2632,7 @@ jobs: /** @type {import('./types/safe-output-script').SafeOutputScriptMain} */ async function main(config = {}) { const { channel, message } = config; - return async function handlePostSlackMessage(item, resolvedTemporaryIds) { + return async function handlePostSlackMessage(item, resolvedTemporaryIds, temporaryIdMap) { const targetChannel = item.channel || "#general"; const text = item.message || "(no message)"; core.info(`[FICTITIOUS SLACK] → ${targetChannel}: ${text}`); @@ -2652,7 +2652,7 @@ jobs: GITHUB_SERVER_URL: ${{ github.server_url }} GITHUB_API_URL: ${{ github.api_url }} GH_AW_SAFE_OUTPUT_SCRIPTS: "{\"post_slack_message\":\"safe_output_script_post_slack_message.cjs\"}" - GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG: "{\"add_comment\":{\"hide_older_comments\":true,\"max\":2},\"add_labels\":{\"allowed\":[\"smoke-claude\"]},\"add_reviewer\":{\"max\":2,\"target\":\"*\"},\"close_pull_request\":{\"max\":1,\"staged\":true},\"create_issue\":{\"close_older_issues\":true,\"expires\":2,\"group\":true,\"labels\":[\"automation\",\"testing\"],\"max\":1},\"create_pull_request_review_comment\":{\"max\":5,\"side\":\"RIGHT\",\"target\":\"*\"},\"missing_data\":{},\"missing_tool\":{},\"push_to_pull_request_branch\":{\"if_no_changes\":\"warn\",\"max_patch_size\":1024,\"protected_files\":[\"package.json\",\"bun.lockb\",\"bunfig.toml\",\"deno.json\",\"deno.jsonc\",\"deno.lock\",\"global.json\",\"NuGet.Config\",\"Directory.Packages.props\",\"mix.exs\",\"mix.lock\",\"go.mod\",\"go.sum\",\"stack.yaml\",\"stack.yaml.lock\",\"pom.xml\",\"build.gradle\",\"build.gradle.kts\",\"settings.gradle\",\"settings.gradle.kts\",\"gradle.properties\",\"package-lock.json\",\"yarn.lock\",\"pnpm-lock.yaml\",\"npm-shrinkwrap.json\",\"requirements.txt\",\"Pipfile\",\"Pipfile.lock\",\"pyproject.toml\",\"setup.py\",\"setup.cfg\",\"Gemfile\",\"Gemfile.lock\",\"uv.lock\",\"CLAUDE.md\"],\"protected_path_prefixes\":[\".github/\",\".agents/\",\".claude/\"],\"staged\":true,\"target\":\"*\"},\"resolve_pull_request_review_thread\":{\"max\":5},\"submit_pull_request_review\":{\"footer\":\"always\",\"max\":1},\"update_pull_request\":{\"allow_body\":true,\"allow_title\":true,\"max\":1,\"target\":\"*\"}}" + GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG: "{\"add_comment\":{\"hide_older_comments\":true,\"max\":2},\"add_labels\":{\"allowed\":[\"smoke-claude\"]},\"add_reviewer\":{\"max\":2,\"target\":\"*\"},\"close_pull_request\":{\"max\":1,\"staged\":true},\"create_issue\":{\"close_older_issues\":true,\"expires\":2,\"group\":true,\"labels\":[\"automation\",\"testing\"],\"max\":1},\"create_pull_request_review_comment\":{\"max\":5,\"side\":\"RIGHT\",\"target\":\"*\"},\"missing_data\":{},\"missing_tool\":{},\"noop\":{\"max\":1,\"report-as-issue\":\"true\"},\"push_to_pull_request_branch\":{\"if_no_changes\":\"warn\",\"max_patch_size\":1024,\"protected_files\":[\"package.json\",\"bun.lockb\",\"bunfig.toml\",\"deno.json\",\"deno.jsonc\",\"deno.lock\",\"global.json\",\"NuGet.Config\",\"Directory.Packages.props\",\"mix.exs\",\"mix.lock\",\"go.mod\",\"go.sum\",\"stack.yaml\",\"stack.yaml.lock\",\"pom.xml\",\"build.gradle\",\"build.gradle.kts\",\"settings.gradle\",\"settings.gradle.kts\",\"gradle.properties\",\"package-lock.json\",\"yarn.lock\",\"pnpm-lock.yaml\",\"npm-shrinkwrap.json\",\"requirements.txt\",\"Pipfile\",\"Pipfile.lock\",\"pyproject.toml\",\"setup.py\",\"setup.cfg\",\"Gemfile\",\"Gemfile.lock\",\"uv.lock\",\"CLAUDE.md\"],\"protected_path_prefixes\":[\".github/\",\".agents/\",\".claude/\"],\"staged\":true,\"target\":\"*\"},\"resolve_pull_request_review_thread\":{\"max\":5},\"submit_pull_request_review\":{\"footer\":\"always\",\"max\":1},\"update_pull_request\":{\"allow_body\":true,\"allow_title\":true,\"max\":1,\"target\":\"*\"}}" GH_AW_CI_TRIGGER_TOKEN: ${{ secrets.GH_AW_CI_TRIGGER_TOKEN }} with: github-token: ${{ secrets.GH_AW_GITHUB_TOKEN || secrets.GITHUB_TOKEN }} diff --git a/actions/setup/js/safe_output_handler_manager.cjs b/actions/setup/js/safe_output_handler_manager.cjs index 2d6dc14d566..b7a4fea5758 100644 --- a/actions/setup/js/safe_output_handler_manager.cjs +++ b/actions/setup/js/safe_output_handler_manager.cjs @@ -175,21 +175,21 @@ async function loadHandlers(config, prReviewBuffer) { const handlerConfig = config[scriptType] || {}; const messageHandler = await scriptModule.main(handlerConfig); if (typeof messageHandler !== "function") { - const error = new Error(`Script handler ${scriptType} main() did not return a function - expected a message handler function but got ${typeof messageHandler}`); - core.error(`✗ Fatal error loading script handler ${scriptType}: ${error.message}`); - throw error; + // Non-fatal: warn and skip this custom script handler rather than crashing the + // entire safe-output loop. A misconfigured user script should not block all + // other safe-output operations. + core.warning(`✗ Custom script handler ${scriptType} main() did not return a function (got ${typeof messageHandler}) — this handler will be skipped`); + } else { + messageHandlers.set(scriptType, messageHandler); + core.info(`✓ Loaded and initialized custom script handler for: ${scriptType}`); } - messageHandlers.set(scriptType, messageHandler); - core.info(`✓ Loaded and initialized custom script handler for: ${scriptType}`); } else { - core.warning(`Custom script handler module ${scriptType} does not export a main function`); + core.warning(`Custom script handler module ${scriptType} does not export a main function — skipping`); } } catch (error) { - const errorMessage = getErrorMessage(error); - if (errorMessage.includes("did not return a function")) { - throw error; - } - core.warning(`Failed to load custom script handler for ${scriptType}: ${errorMessage}`); + // Non-fatal: log a warning and continue loading the remaining handlers. A broken + // custom script should not prevent built-in or other custom handlers from running. + core.warning(`Failed to load custom script handler for ${scriptType}: ${getErrorMessage(error)} — this handler will be skipped`); } } } diff --git a/actions/setup/js/types/safe-output-script.d.ts b/actions/setup/js/types/safe-output-script.d.ts index eb53c5d1a92..7d6d57e658e 100644 --- a/actions/setup/js/types/safe-output-script.d.ts +++ b/actions/setup/js/types/safe-output-script.d.ts @@ -2,14 +2,21 @@ // This file describes the types available when writing a custom safe-output script // (defined under safe-outputs.scripts in workflow frontmatter). // -// Usage in a script body (only the body of main() is written by the user): +// Usage — write only the handler body (compiler generates the outer wrapper): // -// const { inputs } = config; -// return async function handleMyScript(item) { -// const channel = item.channel ?? inputs?.channel?.default ?? "#general"; -// core.info(`[SLACK] → ${channel}: ${item.message}`); -// return { success: true }; -// }; +// const targetChannel = item.channel ?? channel ?? "#general"; +// core.info(`[SLACK] → ${targetChannel}: ${item.message}`); +// return { success: true }; +// +// The compiler generates: +// +// async function main(config = {}) { +// const { channel } = config; // from declared inputs +// return async function handleMyScript(item, resolvedTemporaryIds, temporaryIdMap) { +// // ← your handler body here +// }; +// } +// module.exports = { main }; import type { HandlerResult } from "./handler-factory"; export type { HandlerResult }; @@ -109,28 +116,70 @@ export type SafeOutputScriptItem = Recor // ── Resolved temporary IDs ────────────────────────────────────────────────── /** - * Map of temporary IDs to their resolved GitHub issue/PR/discussion references. - * Passed as the second argument to the handler function. + * A single entry in the resolved temporary IDs map. + * Represents a GitHub issue, PR, or discussion that was created during this run. + */ +export interface ResolvedTemporaryIdEntry { + /** Repository in "owner/repo" format. */ + repo: string; + /** Issue, PR, or discussion number. */ + number: number; +} + +/** + * Plain-object snapshot of the temporary ID map at the time the handler is + * invoked. Passed as the **second** argument to the handler function. + * Temporary IDs are string keys like `"#tmp-1"` or `"#issue-123"`. + * + * @example + * ```javascript + * const resolved = resolvedTemporaryIds["#tmp-1"]; + * if (resolved) { + * core.info(`Issue was created: ${resolved.repo}#${resolved.number}`); + * } + * ``` */ export interface ResolvedTemporaryIds { - [temporaryId: string]: { - /** Repository in "owner/repo" format. */ - repo: string; - /** Issue, PR, or discussion number. */ - number: number; - }; + [temporaryId: string]: ResolvedTemporaryIdEntry; } +/** + * Live Map of temporary IDs to their resolved references. + * Passed as the **third** argument to the handler function. + * + * Unlike `resolvedTemporaryIds` (a plain object snapshot), this is the live + * `Map` that the handler loop updates as new + * issues/PRs are created. Use it when you need up-to-date values. + * + * @example + * ```javascript + * const entry = temporaryIdMap.get("tmp-1"); + * if (entry) { + * core.info(`Resolved: ${entry.repo}#${entry.number}`); + * } + * ``` + */ +export type TemporaryIdMap = Map; + // ── Handler and factory function types ───────────────────────────────────── /** * The async message-handler function returned by `main()`. * Receives a single safe-output message and should return a `HandlerResult`. * + * The handler receives three arguments: + * - `item` — the per-call message with runtime input values as top-level properties + * - `resolvedTemporaryIds` — plain-object snapshot of resolved temporary IDs + * - `temporaryIdMap` — live `Map` of resolved temporary IDs (updated as the loop runs) + * * @typeParam TInputs - The shape of the user-declared inputs (defaults to * `Record`). */ -export type SafeOutputScriptHandler = Record> = (item: SafeOutputScriptItem, resolvedTemporaryIds: ResolvedTemporaryIds) => Promise; +export type SafeOutputScriptHandler = Record> = ( + item: SafeOutputScriptItem, + resolvedTemporaryIds: ResolvedTemporaryIds, + temporaryIdMap: TemporaryIdMap +) => Promise; /** * The type of the `main()` function generated by the compiler around the user's @@ -141,7 +190,7 @@ export type SafeOutputScriptHandler = Re * ```javascript * async function main(config = {}) { * const { channel, message } = config; // auto-generated from declared inputs - * return async function handleX(item, resolvedTemporaryIds) { + * return async function handleX(item, resolvedTemporaryIds, temporaryIdMap) { * // * }; * } diff --git a/pkg/workflow/compiler_safe_outputs_job.go b/pkg/workflow/compiler_safe_outputs_job.go index 2fd21977176..fa84c71518b 100644 --- a/pkg/workflow/compiler_safe_outputs_job.go +++ b/pkg/workflow/compiler_safe_outputs_job.go @@ -570,7 +570,7 @@ func generateSafeOutputScriptContent(scriptName string, scriptConfig *SafeScript // Generate the handler function that receives each item at runtime. handlerName := scriptNameToHandlerName(scriptName) - sb.WriteString(" return async function " + handlerName + "(item, resolvedTemporaryIds) {\n") + sb.WriteString(" return async function " + handlerName + "(item, resolvedTemporaryIds, temporaryIdMap) {\n") // Indent each line of the user's handler body by 4 spaces for line := range strings.SplitSeq(scriptConfig.Script, "\n") { sb.WriteString(" " + line + "\n") diff --git a/pkg/workflow/safe_scripts_test.go b/pkg/workflow/safe_scripts_test.go index aca51a879e8..f2aa082e06e 100644 --- a/pkg/workflow/safe_scripts_test.go +++ b/pkg/workflow/safe_scripts_test.go @@ -268,7 +268,7 @@ func TestGenerateSafeOutputScriptContent(t *testing.T) { assert.Contains(t, content, "// Auto-generated safe-output script handler: my-handler", "Should have comment header") assert.Contains(t, content, "async function main(config = {}) {", "Should wrap with main function") assert.Contains(t, content, "const { channel, message } = config;", "Should destructure config inputs") - assert.Contains(t, content, "return async function handleMyHandler(item, resolvedTemporaryIds) {", "Should generate handler function") + assert.Contains(t, content, "return async function handleMyHandler(item, resolvedTemporaryIds, temporaryIdMap) {", "Should generate handler function") assert.Contains(t, content, " core.info", "Should indent user body by 4 spaces") assert.Contains(t, content, "module.exports = { main };", "Should include module.exports") @@ -296,7 +296,7 @@ func TestGenerateSafeOutputScriptContentNoInputs(t *testing.T) { content := generateSafeOutputScriptContent("simple-handler", scriptConfig) assert.NotContains(t, content, "const {", "Should not destructure when no inputs declared") - assert.Contains(t, content, "return async function handleSimpleHandler(item, resolvedTemporaryIds) {", "Should still generate handler function") + assert.Contains(t, content, "return async function handleSimpleHandler(item, resolvedTemporaryIds, temporaryIdMap) {", "Should still generate handler function") assert.Contains(t, content, " return { success: true };", "Should indent user body by 4 spaces") } @@ -345,7 +345,7 @@ func TestBuildCustomScriptFilesStep(t *testing.T) { // Verify the compiler generates the full outer wrapper assert.Contains(t, fullYAML, "async function main(config = {}) {", "Should generate main function declaration") assert.Contains(t, fullYAML, "const { channel } = config;", "Should generate config input destructuring") - assert.Contains(t, fullYAML, "return async function handleMyHandler(item, resolvedTemporaryIds) {", "Should generate handler function") + assert.Contains(t, fullYAML, "return async function handleMyHandler(item, resolvedTemporaryIds, temporaryIdMap) {", "Should generate handler function") assert.Contains(t, fullYAML, "module.exports = { main };", "Should include module.exports wrapper") // User's handler body content should appear indented inside the handler assert.Contains(t, fullYAML, "return { success: true };", "Should include user's handler body") From 878ebf61cbe6c256d2010817d08ac1d474cd7153 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 18 Mar 2026 23:47:21 +0000 Subject: [PATCH 9/9] Add changeset [skip-ci] --- .changeset/patch-add-safe-output-scripts.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/patch-add-safe-output-scripts.md diff --git a/.changeset/patch-add-safe-output-scripts.md b/.changeset/patch-add-safe-output-scripts.md new file mode 100644 index 00000000000..f79226f8764 --- /dev/null +++ b/.changeset/patch-add-safe-output-scripts.md @@ -0,0 +1,5 @@ +--- +"gh-aw": patch +--- + +Add support for custom `safe-outputs.scripts` handlers that run in the consolidated safe-outputs loop, including schema support, generated tool definitions, runtime loading/dispatch, and temporary ID context.