From 990d4519bb68e794a6c529efa67a7d49b7905157 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Sun, 8 Mar 2026 18:58:34 +0000 Subject: [PATCH] refactor: simplify generateCustomJobToolDefinition and extractDispatchWorkflowNames - Remove redundant `if len(jobConfig.Inputs) > 0` guard (ranging over nil map is safe) - Merge duplicate `case "string", ""` and `default:` switch branches into single `default:` - Move `additionalProperties: false` into the inputSchema map literal - Build tool return map at the point of return instead of mutating across the function - Remove obvious inline comments that repeat what the code already says - Inline the filter loop in extractDispatchWorkflowNames into the collection loop - Combine two-step map existence check + type assertion into a single comma-ok assertion Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/cli/remote_workflow.go | 29 ++---- .../safe_outputs_config_generation.go | 99 ++++++++----------- 2 files changed, 46 insertions(+), 82 deletions(-) diff --git a/pkg/cli/remote_workflow.go b/pkg/cli/remote_workflow.go index 13e9b1a5f37..89aacbcc7fc 100644 --- a/pkg/cli/remote_workflow.go +++ b/pkg/cli/remote_workflow.go @@ -631,12 +631,7 @@ func extractDispatchWorkflowNames(content string) []string { return nil } - safeOutputs, exists := result.Frontmatter["safe-outputs"] - if !exists { - return nil - } - - safeOutputsMap, ok := safeOutputs.(map[string]any) + safeOutputsMap, ok := result.Frontmatter["safe-outputs"].(map[string]any) if !ok { return nil } @@ -652,32 +647,22 @@ func extractDispatchWorkflowNames(content string) []string { case []any: // Array format: dispatch-workflow: [name1, name2] for _, item := range v { - if name, ok := item.(string); ok { + if name, ok := item.(string); ok && !strings.Contains(name, "${{") { workflowNames = append(workflowNames, name) } } case map[string]any: // Map format: dispatch-workflow: {workflows: [name1, name2]} - if workflows, exists := v["workflows"]; exists { - if workflowsArray, ok := workflows.([]any); ok { - for _, item := range workflowsArray { - if name, ok := item.(string); ok { - workflowNames = append(workflowNames, name) - } + if workflowsArray, ok := v["workflows"].([]any); ok { + for _, item := range workflowsArray { + if name, ok := item.(string); ok && !strings.Contains(name, "${{") { + workflowNames = append(workflowNames, name) } } } } - // Filter out GitHub Actions expression syntax (e.g. "${{ vars.WORKFLOW }}") - filtered := make([]string, 0, len(workflowNames)) - for _, name := range workflowNames { - if !strings.Contains(name, "${{") { - filtered = append(filtered, name) - } - } - - return filtered + return workflowNames } // fetchAndSaveRemoteDispatchWorkflows fetches and saves the workflow files referenced in the diff --git a/pkg/workflow/safe_outputs_config_generation.go b/pkg/workflow/safe_outputs_config_generation.go index 309ce97ccc2..da7dab5abb3 100644 --- a/pkg/workflow/safe_outputs_config_generation.go +++ b/pkg/workflow/safe_outputs_config_generation.go @@ -10,87 +10,66 @@ import ( func generateCustomJobToolDefinition(jobName string, jobConfig *SafeJobConfig) map[string]any { safeOutputsConfigLog.Printf("Generating tool definition for custom job: %s", jobName) - // Build the tool definition - tool := map[string]any{ - "name": jobName, + description := jobConfig.Description + if description == "" { + description = fmt.Sprintf("Execute the %s custom job", jobName) } - // Add description if present - if jobConfig.Description != "" { - tool["description"] = jobConfig.Description - } else { - // Provide a default description if none is specified - tool["description"] = fmt.Sprintf("Execute the %s custom job", jobName) - } - - // Build the input schema inputSchema := map[string]any{ - "type": "object", - "properties": make(map[string]any), + "type": "object", + "properties": make(map[string]any), + "additionalProperties": false, } - // Track required fields var requiredFields []string + properties := inputSchema["properties"].(map[string]any) - // Add each input to the schema - if len(jobConfig.Inputs) > 0 { - properties := inputSchema["properties"].(map[string]any) + for inputName, inputDef := range jobConfig.Inputs { + property := map[string]any{} - for inputName, inputDef := range jobConfig.Inputs { - property := map[string]any{} - - // Add description - if inputDef.Description != "" { - property["description"] = inputDef.Description - } - - // Convert type to JSON Schema type - switch inputDef.Type { - case "choice": - // Choice inputs are strings with enum constraints - property["type"] = "string" - if len(inputDef.Options) > 0 { - property["enum"] = inputDef.Options - } - case "boolean": - property["type"] = "boolean" - case "number": - property["type"] = "number" - case "string", "": - // Default to string if type is not specified - property["type"] = "string" - default: - // For any unknown type, default to string - property["type"] = "string" - } + if inputDef.Description != "" { + property["description"] = inputDef.Description + } - // Add default value if present - if inputDef.Default != nil { - property["default"] = inputDef.Default + // Convert type to JSON Schema type + switch inputDef.Type { + case "choice": + // Choice inputs are strings with enum constraints + property["type"] = "string" + if len(inputDef.Options) > 0 { + property["enum"] = inputDef.Options } + case "boolean": + property["type"] = "boolean" + case "number": + property["type"] = "number" + default: + // "string", empty string, or any unknown type defaults to string + property["type"] = "string" + } - // Track required fields - if inputDef.Required { - requiredFields = append(requiredFields, inputName) - } + if inputDef.Default != nil { + property["default"] = inputDef.Default + } - properties[inputName] = property + if inputDef.Required { + requiredFields = append(requiredFields, inputName) } + + properties[inputName] = property } - // Add required fields array if any inputs are required if len(requiredFields) > 0 { sort.Strings(requiredFields) inputSchema["required"] = requiredFields } - // Prevent additional properties to maintain schema strictness - inputSchema["additionalProperties"] = false - - tool["inputSchema"] = inputSchema - safeOutputsConfigLog.Printf("Generated tool definition for %s with %d inputs, %d required", jobName, len(jobConfig.Inputs), len(requiredFields)) - return tool + return map[string]any{ + "name": jobName, + "description": description, + "inputSchema": inputSchema, + } }