From cd26c4a52325676ba3c01bd9ad33ab650ad48a65 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 22 Feb 2026 18:54:24 +0000 Subject: [PATCH 1/2] Initial plan From b4013d42eb82b4e9485ec5d6f5b2e4b58df00e68 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 22 Feb 2026 19:05:08 +0000 Subject: [PATCH 2/2] fix(IMP-003): move generateCustomJobToolDefinition to safe_outputs_config_generation.go Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../safe_outputs_config_generation.go | 94 +++++++++- .../safe_outputs_config_generation_test.go | 163 ++++++++++++++++++ pkg/workflow/safe_outputs_tools_generation.go | 90 ---------- 3 files changed, 255 insertions(+), 92 deletions(-) diff --git a/pkg/workflow/safe_outputs_config_generation.go b/pkg/workflow/safe_outputs_config_generation.go index ab19ba28248..0a10f77cfd6 100644 --- a/pkg/workflow/safe_outputs_config_generation.go +++ b/pkg/workflow/safe_outputs_config_generation.go @@ -2,6 +2,8 @@ package workflow import ( "encoding/json" + "fmt" + "sort" "strconv" "strings" ) @@ -10,8 +12,96 @@ import ( // Safe Output Configuration Generation // ======================================== -// populateDispatchWorkflowFiles populates the WorkflowFiles map for dispatch-workflow configuration. -// This must be called before generateSafeOutputsConfig to ensure workflow file extensions are available. +// generateCustomJobToolDefinition creates an MCP tool definition for a custom safe-output job +// Returns a map representing the tool definition in MCP format with name, description, and inputSchema +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, + } + + // 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), + } + + // Track required fields + var requiredFields []string + + // 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{} + + // 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" + } + + // Add default value if present + if inputDef.Default != nil { + property["default"] = inputDef.Default + } + + // Track required fields + 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 +} + func populateDispatchWorkflowFiles(data *WorkflowData, markdownPath string) { if data.SafeOutputs == nil || data.SafeOutputs.DispatchWorkflow == nil { return diff --git a/pkg/workflow/safe_outputs_config_generation_test.go b/pkg/workflow/safe_outputs_config_generation_test.go index 6925d1c52d9..0ce0fed7de6 100644 --- a/pkg/workflow/safe_outputs_config_generation_test.go +++ b/pkg/workflow/safe_outputs_config_generation_test.go @@ -165,3 +165,166 @@ func TestPopulateDispatchWorkflowFilesFindsLockFile(t *testing.T) { assert.Equal(t, ".lock.yml", data.SafeOutputs.DispatchWorkflow.WorkflowFiles["deploy"], "Should prefer .lock.yml over .yml") } + +// TestGenerateCustomJobToolDefinition tests that generateCustomJobToolDefinition produces +// valid MCP tool definitions from SafeJobConfig input definitions. +func TestGenerateCustomJobToolDefinition(t *testing.T) { + tests := []struct { + name string + jobName string + jobConfig *SafeJobConfig + check func(t *testing.T, result map[string]any) + }{ + { + name: "basic string input", + jobName: "my_job", + jobConfig: &SafeJobConfig{ + Description: "A test job", + Inputs: map[string]*InputDefinition{ + "title": { + Type: "string", + Description: "The title", + Required: true, + }, + }, + }, + check: func(t *testing.T, result map[string]any) { + assert.Equal(t, "my_job", result["name"], "name should match job name") + assert.Equal(t, "A test job", result["description"], "description should be included") + schema, ok := result["inputSchema"].(map[string]any) + require.True(t, ok, "inputSchema should be a map") + assert.Equal(t, "object", schema["type"], "schema type should be object") + assert.Equal(t, false, schema["additionalProperties"], "additionalProperties should be false") + props, ok := schema["properties"].(map[string]any) + require.True(t, ok, "properties should be a map") + titleProp, ok := props["title"].(map[string]any) + require.True(t, ok, "title property should exist") + assert.Equal(t, "string", titleProp["type"], "title type should be string") + assert.Equal(t, "The title", titleProp["description"], "title description should be set") + required, ok := schema["required"].([]string) + require.True(t, ok, "required should be a []string") + assert.Contains(t, required, "title", "title should be required") + }, + }, + { + name: "boolean input", + jobName: "bool_job", + jobConfig: &SafeJobConfig{ + Inputs: map[string]*InputDefinition{ + "flag": { + Type: "boolean", + Required: false, + }, + }, + }, + check: func(t *testing.T, result map[string]any) { + schema := result["inputSchema"].(map[string]any) + props := schema["properties"].(map[string]any) + flagProp := props["flag"].(map[string]any) + assert.Equal(t, "boolean", flagProp["type"], "flag type should be boolean") + assert.Nil(t, schema["required"], "required should be absent when no required fields") + }, + }, + { + name: "number input", + jobName: "num_job", + jobConfig: &SafeJobConfig{ + Inputs: map[string]*InputDefinition{ + "count": { + Type: "number", + Required: true, + }, + }, + }, + check: func(t *testing.T, result map[string]any) { + schema := result["inputSchema"].(map[string]any) + props := schema["properties"].(map[string]any) + countProp := props["count"].(map[string]any) + assert.Equal(t, "number", countProp["type"], "count type should be number") + }, + }, + { + name: "choice input with enum", + jobName: "choice_job", + jobConfig: &SafeJobConfig{ + Inputs: map[string]*InputDefinition{ + "color": { + Type: "choice", + Options: []string{"red", "green", "blue"}, + }, + }, + }, + check: func(t *testing.T, result map[string]any) { + schema := result["inputSchema"].(map[string]any) + props := schema["properties"].(map[string]any) + colorProp := props["color"].(map[string]any) + assert.Equal(t, "string", colorProp["type"], "choice type should map to string") + assert.Equal(t, []string{"red", "green", "blue"}, colorProp["enum"], "enum options should be set") + }, + }, + { + name: "no inputs", + jobName: "empty_job", + jobConfig: &SafeJobConfig{ + Description: "No inputs", + }, + check: func(t *testing.T, result map[string]any) { + assert.Equal(t, "empty_job", result["name"], "name should match") + schema := result["inputSchema"].(map[string]any) + props := schema["properties"].(map[string]any) + assert.Empty(t, props, "properties should be empty") + assert.Nil(t, schema["required"], "required should be absent") + }, + }, + { + name: "no description uses default", + jobName: "nodesc_job", + jobConfig: &SafeJobConfig{ + Inputs: map[string]*InputDefinition{ + "x": {Type: "string"}, + }, + }, + check: func(t *testing.T, result map[string]any) { + desc, hasDesc := result["description"] + assert.True(t, hasDesc, "description should be present (default is added)") + assert.Contains(t, desc.(string), "nodesc_job", "default description should include job name") + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := generateCustomJobToolDefinition(tt.jobName, tt.jobConfig) + require.NotNil(t, result, "result should not be nil") + tt.check(t, result) + }) + } +} + +// TestGenerateCustomJobToolDefinitionJSONSerializable verifies that the output of +// generateCustomJobToolDefinition can be marshaled to valid JSON. +func TestGenerateCustomJobToolDefinitionJSONSerializable(t *testing.T) { + jobConfig := &SafeJobConfig{ + Description: "Run deployment", + Inputs: map[string]*InputDefinition{ + "env": { + Type: "choice", + Description: "Target environment", + Required: true, + Options: []string{"staging", "production"}, + }, + "dry_run": { + Type: "boolean", + Required: false, + }, + }, + } + + result := generateCustomJobToolDefinition("deploy", jobConfig) + data, err := json.Marshal(result) + require.NoError(t, err, "result should be JSON serializable") + + var parsed map[string]any + require.NoError(t, json.Unmarshal(data, &parsed), "JSON should be parseable back") + assert.Equal(t, "deploy", parsed["name"], "name should round-trip through JSON") +} diff --git a/pkg/workflow/safe_outputs_tools_generation.go b/pkg/workflow/safe_outputs_tools_generation.go index c655909a8d4..95ccf6e05fe 100644 --- a/pkg/workflow/safe_outputs_tools_generation.go +++ b/pkg/workflow/safe_outputs_tools_generation.go @@ -13,96 +13,6 @@ import ( // Safe Output Tools Generation // ======================================== -// generateCustomJobToolDefinition creates an MCP tool definition for a custom safe-output job -// Returns a map representing the tool definition in MCP format with name, description, and inputSchema -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, - } - - // 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), - } - - // Track required fields - var requiredFields []string - - // 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{} - - // 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" - } - - // Add default value if present - if inputDef.Default != nil { - property["default"] = inputDef.Default - } - - // Track required fields - 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 -} - // checkAllEnabledToolsPresent verifies that every tool in enabledTools has a matching entry // in filteredTools. This is a compiler error check: if a safe-output type is registered in // Go code but its definition is missing from safe-output-tools.json, it will not appear in