Skip to content

Commit eedb6ee

Browse files
committed
refactor(workflow): move output validation off inline node paths
Make the scheduler the single authoritative point for output validation. FunctionNode.wrappedFn and ToolNode.runTool no longer validate the output against their schema inline; the schema still flows through BaseNode so the scheduler enforces it on every yielded event with non-nil output. This removes duplicate validation and the risk of drift as the scheduler logic evolves (symmetric to the input validation cleanup). The FunctionTool {"result": X} unwrap fallback is not lost: it moves to a ToolNode.ValidateOutput override, the correct home for that tool-specific convention. The override tries standard validation first, then unwraps a "result" key and validates the unwrapped value, and finally re-runs standard validation so callers see the original schema-mismatch error rather than a fallback artifact. nil schema is a passthrough. Tests that previously asserted on inline behavior are updated: - FunctionNode: the inline ValidationError table case is replaced by TestFunctionNode_ValidateOutput, which asserts Run passes the output through unchanged and ValidateOutput surfaces the schema mismatch. - ToolNode: TestToolNode_Run now asserts the raw FunctionTool map output from Run (no inline unwrap); the new TestToolNode_ValidateOutput covers the four override cases (direct-valid passthrough, {"result": X} unwrap, both-fail original error, nil-schema passthrough). BUG=516382303 BUG=516382092
1 parent 3f4aa49 commit eedb6ee

4 files changed

Lines changed: 148 additions & 65 deletions

File tree

workflow/function_node.go

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -230,11 +230,6 @@ func NewFunctionNodeFromState[Params, OUT any](
230230
if err != nil {
231231
return output, err
232232
}
233-
if oschema != nil {
234-
if err := typeutil.ValidateWithJSONSchema(output, oschema); err != nil {
235-
return nil, fmt.Errorf("function node %s: validation failed for output %T: %w", name, new(OUT), err)
236-
}
237-
}
238233
return output, nil
239234
}
240235

@@ -276,13 +271,6 @@ func newFunctionNodeWithResolvedSchemas[IN, OUT any](name string, fn func(ctx ag
276271
return output, err
277272
}
278273

279-
if outputSchema != nil {
280-
validateErr := typeutil.ValidateWithJSONSchema(output, outputSchema)
281-
if validateErr != nil {
282-
return nil, fmt.Errorf("function node %s: validation failed for output %T: %w", name, new(OUT), validateErr)
283-
}
284-
}
285-
286274
return output, nil
287275
}
288276

@@ -321,11 +309,6 @@ func newEmittingFunctionNodeWithResolvedSchemas[IN, OUT any](name string, fn Emi
321309
if err != nil {
322310
return nil, err
323311
}
324-
if outputSchema != nil {
325-
if validateErr := typeutil.ValidateWithJSONSchema(output, outputSchema); validateErr != nil {
326-
return nil, fmt.Errorf("function node %s: validation failed for output %T: %w", name, new(OUT), validateErr)
327-
}
328-
}
329312
return output, nil
330313
}
331314

workflow/function_node_test.go

Lines changed: 43 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,6 @@ func TestNewFunctionNodeWithSchema(t *testing.T) {
3737
type Output struct {
3838
Result string `json:"result"`
3939
}
40-
type TargetOutput struct {
41-
Result int `json:"result"`
42-
}
4340

4441
tests := []struct {
4542
name string
@@ -79,18 +76,6 @@ func TestNewFunctionNodeWithSchema(t *testing.T) {
7976
wantOutput: map[string]any{"result": "zero"},
8077
wantErr: false,
8178
},
82-
{
83-
name: "ValidationError",
84-
nodeName: "test",
85-
fn: func(ctx agent.Context, input Input) (map[string]any, error) {
86-
return map[string]any{"result": "not-an-int"}, nil
87-
},
88-
inputSchema: mustSchema[Input](t),
89-
outputSchema: mustSchema[TargetOutput](t),
90-
input: Input{Value: "hello"},
91-
wantErr: true,
92-
errSubstr: "validation failed for output",
93-
},
9479
}
9580

9681
for _, tc := range tests {
@@ -132,6 +117,49 @@ func TestNewFunctionNodeWithSchema(t *testing.T) {
132117
}
133118
}
134119

120+
// TestFunctionNode_ValidateOutput verifies that output schema validation
121+
// is enforced through the node-level ValidateOutput contract (invoked
122+
// scheduler-side), not inline inside Run. Run itself passes the output
123+
// through unchanged; ValidateOutput surfaces the schema mismatch.
124+
func TestFunctionNode_ValidateOutput(t *testing.T) {
125+
type Input struct {
126+
Value string `json:"value"`
127+
}
128+
type TargetOutput struct {
129+
Result int `json:"result"`
130+
}
131+
132+
fn := func(ctx agent.Context, input Input) (map[string]any, error) {
133+
return map[string]any{"result": "not-an-int"}, nil
134+
}
135+
node, err := NewFunctionNodeWithSchema[Input, map[string]any](
136+
"test", fn, mustSchema[Input](t), mustSchema[TargetOutput](t), defaultNodeConfig)
137+
if err != nil {
138+
t.Fatalf("NewFunctionNodeWithSchema failed: %v", err)
139+
}
140+
141+
// Run no longer validates: it yields the raw output without error.
142+
mockCtx := &MockInvocationContext{sess: nil}
143+
exCtx := agent.NewNodeContext(mockCtx, nil)
144+
var got any
145+
count := 0
146+
for ev, err := range node.Run(exCtx, Input{Value: "hello"}) {
147+
if err != nil {
148+
t.Fatalf("Run returned unexpected error: %v", err)
149+
}
150+
got = ev.Output
151+
count++
152+
}
153+
if count != 1 {
154+
t.Fatalf("expected 1 event from Run, got %d", count)
155+
}
156+
157+
// ValidateOutput (the scheduler-side contract) rejects the mismatch.
158+
if _, err := node.ValidateOutput(got); err == nil {
159+
t.Fatalf("ValidateOutput: expected validation error, got nil")
160+
}
161+
}
162+
135163
func mustSchema[T any](t *testing.T) *jsonschema.Schema {
136164
t.Helper()
137165
s, err := jsonschema.For[T](nil)

workflow/tool_node.go

Lines changed: 29 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -112,23 +112,38 @@ func (n *ToolNode) runTool(toolCtx agent.Context, input any) (any, error) {
112112
return nil, fmt.Errorf("tool %q execution failed: %w", n.tool.Name(), err)
113113
}
114114

115-
var toolOutput any = output
116-
117-
// Validate
118-
if schema := n.OutputSchema(); schema != nil {
119-
if err := schema.Validate(output); err != nil {
120-
if val, ok := output["result"]; ok {
121-
if err := schema.Validate(val); err != nil {
122-
return nil, fmt.Errorf("converting tool %q output: validation failed for result key: %w", n.tool.Name(), err)
123-
}
124-
toolOutput = val
125-
} else {
126-
return nil, fmt.Errorf("converting tool %q output: validation failed: %w", n.tool.Name(), err)
115+
return output, nil
116+
}
117+
118+
// ValidateOutput validates the tool output against the node's output
119+
// schema, adding a FunctionTool-specific fallback on top of the default
120+
// behavior: when the output is a map of shape {"result": X} that fails
121+
// direct schema validation, it retries against the unwrapped "result"
122+
// value and, on success, returns that unwrapped value.
123+
//
124+
// This override is the home for the {"result": X} convention because it
125+
// is tool-specific; making it a general default could mask genuine
126+
// validation errors in other node types.
127+
func (n *ToolNode) ValidateOutput(out any) (any, error) {
128+
schema := n.OutputSchema()
129+
if schema == nil {
130+
return out, nil
131+
}
132+
// Try standard validation first.
133+
if validated, err := defaultValidateOutput(out, schema); err == nil {
134+
return validated, nil
135+
}
136+
// Fallback: unwrap {"result": X} (FunctionTool convention).
137+
if m, ok := out.(map[string]any); ok {
138+
if val, ok := m["result"]; ok {
139+
if validated, err := defaultValidateOutput(val, schema); err == nil {
140+
return validated, nil
127141
}
128142
}
129143
}
130-
131-
return toolOutput, nil
144+
// Re-run standard validation so the caller sees the original
145+
// schema-mismatch error rather than a fallback artifact.
146+
return defaultValidateOutput(out, schema)
132147
}
133148

134149
// Run implements the Node interface and executes the tool.

workflow/tool_node_test.go

Lines changed: 76 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -110,9 +110,6 @@ func TestToolNode_Run(t *testing.T) {
110110
type Output struct {
111111
Greeting string `json:"greeting"`
112112
}
113-
type ErrorOutput struct {
114-
Result int `json:"result"`
115-
}
116113

117114
tests := []struct {
118115
name string
@@ -162,26 +159,14 @@ func TestToolNode_Run(t *testing.T) {
162159
node: func(t tool.Tool) (Node, error) {
163160
return NewToolNodeTyped[Input, string](t, defaultNodeConfig)
164161
},
162+
// Run no longer unwraps {"result": X}; that happens
163+
// scheduler-side in ToolNode.ValidateOutput. Run yields the
164+
// raw FunctionTool map output.
165165
extract: func(t *testing.T, out any) string {
166-
return out.(string)
166+
return out.(map[string]any)["result"].(string)
167167
},
168168
want: "HELLO WORLD",
169169
},
170-
{
171-
name: "schema_validation_error",
172-
tool: func() (tool.Tool, error) {
173-
return functiontool.New(functiontool.Config{
174-
Name: "test_tool",
175-
}, func(ctx agent.Context, in map[string]any) (map[string]any, error) {
176-
return map[string]any{"result": "not-an-int"}, nil
177-
})
178-
},
179-
nodeInput: map[string]any{},
180-
node: func(t tool.Tool) (Node, error) {
181-
return NewToolNodeTyped[map[string]any, ErrorOutput](t, defaultNodeConfig)
182-
},
183-
wantErr: "converting tool \"test_tool\" output",
184-
},
185170
{
186171
name: "tool_execution_error",
187172
tool: func() (tool.Tool, error) {
@@ -276,6 +261,78 @@ func TestToolNode_Run(t *testing.T) {
276261
}
277262
}
278263

264+
// TestToolNode_ValidateOutput exercises the FunctionTool-specific
265+
// {"result": X} unwrap fallback that ToolNode layers on top of the
266+
// default schema validation.
267+
func TestToolNode_ValidateOutput(t *testing.T) {
268+
type Result struct {
269+
Greeting string `json:"greeting"`
270+
}
271+
272+
// Node carrying a Result output schema.
273+
schemaNode := &ToolNode{
274+
BaseNode: NewBaseNodeWithSchemas(
275+
"greet", "", defaultNodeConfig, nil, resolveTestSchema[Result](t)),
276+
}
277+
// Node with no output schema.
278+
nilSchemaNode := &ToolNode{
279+
BaseNode: NewBaseNode("greet", "", defaultNodeConfig),
280+
}
281+
282+
valid := map[string]any{"greeting": "Hello World"}
283+
284+
tests := []struct {
285+
name string
286+
node *ToolNode
287+
output any
288+
want any
289+
wantErr bool
290+
}{
291+
{
292+
name: "direct_valid_passes_through",
293+
node: schemaNode,
294+
output: valid,
295+
want: valid,
296+
},
297+
{
298+
name: "result_wrapped_is_unwrapped",
299+
node: schemaNode,
300+
output: map[string]any{"result": valid},
301+
want: valid,
302+
},
303+
{
304+
name: "fails_direct_and_fallback",
305+
node: schemaNode,
306+
output: map[string]any{"result": map[string]any{"unexpected": 1}},
307+
wantErr: true,
308+
},
309+
{
310+
name: "nil_schema_passes_through",
311+
node: nilSchemaNode,
312+
output: map[string]any{"anything": 1},
313+
want: map[string]any{"anything": 1},
314+
},
315+
}
316+
317+
for _, tc := range tests {
318+
t.Run(tc.name, func(t *testing.T) {
319+
got, err := tc.node.ValidateOutput(tc.output)
320+
if tc.wantErr {
321+
if err == nil {
322+
t.Fatalf("ValidateOutput: expected error, got nil (out=%v)", got)
323+
}
324+
return
325+
}
326+
if err != nil {
327+
t.Fatalf("ValidateOutput: unexpected error: %v", err)
328+
}
329+
if diff := cmp.Diff(tc.want, got); diff != "" {
330+
t.Errorf("ValidateOutput mismatch (-want +got):\n%s", diff)
331+
}
332+
})
333+
}
334+
}
335+
279336
func TestToolNode_WorkflowIntegration(t *testing.T) {
280337
type Input struct {
281338
Val int `json:"val"`

0 commit comments

Comments
 (0)