Skip to content

Commit 0e6154d

Browse files
authored
Sort required fields in safe output custom job schemas for stable output (#8266)
1 parent 5dec658 commit 0e6154d

File tree

2 files changed

+75
-0
lines changed

2 files changed

+75
-0
lines changed

pkg/workflow/safe_outputs_config_generation.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package workflow
33
import (
44
"encoding/json"
55
"fmt"
6+
"sort"
67
)
78

89
// ========================================
@@ -473,6 +474,7 @@ func generateCustomJobToolDefinition(jobName string, jobConfig *SafeJobConfig) m
473474

474475
// Add required fields array if any inputs are required
475476
if len(requiredFields) > 0 {
477+
sort.Strings(requiredFields)
476478
inputSchema["required"] = requiredFields
477479
}
478480

pkg/workflow/safe_outputs_custom_job_tools_test.go

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,3 +207,76 @@ func TestCustomJobToolsWithDifferentInputTypes(t *testing.T) {
207207
assert.NotContains(t, required, "count", "Count should not be required")
208208
assert.NotContains(t, required, "mode", "Mode should not be required")
209209
}
210+
211+
// TestCustomJobToolsRequiredFieldsSorted verifies that the required array
212+
// is sorted alphabetically for stable output
213+
func TestCustomJobToolsRequiredFieldsSorted(t *testing.T) {
214+
workflowData := &WorkflowData{
215+
SafeOutputs: &SafeOutputsConfig{
216+
Jobs: map[string]*SafeJobConfig{
217+
"sorted_test": {
218+
Description: "Job to test sorted required fields",
219+
Inputs: map[string]*InputDefinition{
220+
"zebra": {
221+
Description: "Last alphabetically",
222+
Required: true,
223+
Type: "string",
224+
},
225+
"apple": {
226+
Description: "First alphabetically",
227+
Required: true,
228+
Type: "string",
229+
},
230+
"middle": {
231+
Description: "Middle alphabetically",
232+
Required: true,
233+
Type: "string",
234+
},
235+
"banana": {
236+
Description: "Second alphabetically",
237+
Required: true,
238+
Type: "string",
239+
},
240+
},
241+
},
242+
},
243+
},
244+
}
245+
246+
// Generate the tools JSON
247+
toolsJSON, err := generateFilteredToolsJSON(workflowData)
248+
require.NoError(t, err, "Should generate tools JSON")
249+
250+
// Parse the JSON
251+
var tools []map[string]any
252+
err = json.Unmarshal([]byte(toolsJSON), &tools)
253+
require.NoError(t, err, "Should parse tools JSON")
254+
255+
// Find the sorted_test tool
256+
var sortedTestTool map[string]any
257+
for _, tool := range tools {
258+
if name, ok := tool["name"].(string); ok && name == "sorted_test" {
259+
sortedTestTool = tool
260+
break
261+
}
262+
}
263+
264+
require.NotNil(t, sortedTestTool, "Should find sorted_test tool in tools.json")
265+
266+
// Verify the input schema
267+
inputSchema, ok := sortedTestTool["inputSchema"].(map[string]any)
268+
require.True(t, ok, "Should have inputSchema")
269+
270+
// Verify required fields are sorted
271+
required, ok := inputSchema["required"].([]any)
272+
require.True(t, ok, "Should have required array")
273+
assert.Len(t, required, 4, "Should have 4 required fields")
274+
275+
// Check that the required array is sorted alphabetically
276+
expectedOrder := []string{"apple", "banana", "middle", "zebra"}
277+
for i, expectedField := range expectedOrder {
278+
actualField, ok := required[i].(string)
279+
require.True(t, ok, "Required field should be a string")
280+
assert.Equal(t, expectedField, actualField, "Required field at index %d should be %s", i, expectedField)
281+
}
282+
}

0 commit comments

Comments
 (0)