refactor(workflow): move output validation off inline node paths#1047
Merged
Conversation
wolo-lab
force-pushed
the
wolo/output-validation-cleanup
branch
2 times, most recently
from
June 16, 2026 18:15
eedb6ee to
3df7c29
Compare
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
wolo-lab
force-pushed
the
wolo/output-validation-cleanup
branch
from
June 16, 2026 18:17
3df7c29 to
ca8a6f6
Compare
wolo-lab
marked this pull request as ready for review
June 16, 2026 18:19
hanorik
reviewed
Jun 17, 2026
| // is enforced through the node-level ValidateOutput contract (invoked | ||
| // scheduler-side), not inline inside Run. Run itself passes the output | ||
| // through unchanged; ValidateOutput surfaces the schema mismatch. | ||
| func TestFunctionNode_ValidateOutput(t *testing.T) { |
Contributor
There was a problem hiding this comment.
Maybe to add a success test case as well?
Contributor
Author
There was a problem hiding this comment.
Good idea. Adding.
hanorik
approved these changes
Jun 17, 2026
Cover the positive path: a schema-conforming output passes ValidateOutput and is returned unchanged. Brings the test to parity with TestToolNode_ValidateOutput, which already asserts a direct-valid case.
…put table-driven Separate the two concerns the test conflated: Run-passthrough now lives in TestFunctionNode_RunDoesNotValidate, while TestFunctionNode_ValidateOutput becomes table-driven (direct-valid, schema-mismatch, nil-schema). Mirrors the structure of TestToolNode_Run + TestToolNode_ValidateOutput.
wolo-lab
added a commit
that referenced
this pull request
Jun 18, 2026
* 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
* test(workflow): add success case to TestFunctionNode_ValidateOutput
Cover the positive path: a schema-conforming output passes
ValidateOutput and is returned unchanged. Brings the test to parity
with TestToolNode_ValidateOutput, which already asserts a direct-valid
case.
* test(workflow): split FunctionNode validation tests, make ValidateOutput table-driven
Separate the two concerns the test conflated: Run-passthrough now lives
in TestFunctionNode_RunDoesNotValidate, while TestFunctionNode_ValidateOutput
becomes table-driven (direct-valid, schema-mismatch, nil-schema). Mirrors
the structure of TestToolNode_Run + TestToolNode_ValidateOutput.
wolo-lab
added a commit
that referenced
this pull request
Jun 22, 2026
* 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
* test(workflow): add success case to TestFunctionNode_ValidateOutput
Cover the positive path: a schema-conforming output passes
ValidateOutput and is returned unchanged. Brings the test to parity
with TestToolNode_ValidateOutput, which already asserts a direct-valid
case.
* test(workflow): split FunctionNode validation tests, make ValidateOutput table-driven
Separate the two concerns the test conflated: Run-passthrough now lives
in TestFunctionNode_RunDoesNotValidate, while TestFunctionNode_ValidateOutput
becomes table-driven (direct-valid, schema-mismatch, nil-schema). Mirrors
the structure of TestToolNode_Run + TestToolNode_ValidateOutput.
wolo-lab
added a commit
that referenced
this pull request
Jun 30, 2026
* 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
* test(workflow): add success case to TestFunctionNode_ValidateOutput
Cover the positive path: a schema-conforming output passes
ValidateOutput and is returned unchanged. Brings the test to parity
with TestToolNode_ValidateOutput, which already asserts a direct-valid
case.
* test(workflow): split FunctionNode validation tests, make ValidateOutput table-driven
Separate the two concerns the test conflated: Run-passthrough now lives
in TestFunctionNode_RunDoesNotValidate, while TestFunctionNode_ValidateOutput
becomes table-driven (direct-valid, schema-mismatch, nil-schema). Mirrors
the structure of TestToolNode_Run + TestToolNode_ValidateOutput.
wolo-lab
added a commit
that referenced
this pull request
Jun 30, 2026
* 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
* test(workflow): add success case to TestFunctionNode_ValidateOutput
Cover the positive path: a schema-conforming output passes
ValidateOutput and is returned unchanged. Brings the test to parity
with TestToolNode_ValidateOutput, which already asserts a direct-valid
case.
* test(workflow): split FunctionNode validation tests, make ValidateOutput table-driven
Separate the two concerns the test conflated: Run-passthrough now lives
in TestFunctionNode_RunDoesNotValidate, while TestFunctionNode_ValidateOutput
becomes table-driven (direct-valid, schema-mismatch, nil-schema). Mirrors
the structure of TestToolNode_Run + TestToolNode_ValidateOutput.
wolo-lab
added a commit
that referenced
this pull request
Jun 30, 2026
* 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
* test(workflow): add success case to TestFunctionNode_ValidateOutput
Cover the positive path: a schema-conforming output passes
ValidateOutput and is returned unchanged. Brings the test to parity
with TestToolNode_ValidateOutput, which already asserts a direct-valid
case.
* test(workflow): split FunctionNode validation tests, make ValidateOutput table-driven
Separate the two concerns the test conflated: Run-passthrough now lives
in TestFunctionNode_RunDoesNotValidate, while TestFunctionNode_ValidateOutput
becomes table-driven (direct-valid, schema-mismatch, nil-schema). Mirrors
the structure of TestToolNode_Run + TestToolNode_ValidateOutput.
wolo-lab
added a commit
that referenced
this pull request
Jun 30, 2026
* 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
* test(workflow): add success case to TestFunctionNode_ValidateOutput
Cover the positive path: a schema-conforming output passes
ValidateOutput and is returned unchanged. Brings the test to parity
with TestToolNode_ValidateOutput, which already asserts a direct-valid
case.
* test(workflow): split FunctionNode validation tests, make ValidateOutput table-driven
Separate the two concerns the test conflated: Run-passthrough now lives
in TestFunctionNode_RunDoesNotValidate, while TestFunctionNode_ValidateOutput
becomes table-driven (direct-valid, schema-mismatch, nil-schema). Mirrors
the structure of TestToolNode_Run + TestToolNode_ValidateOutput.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Output validation for
FunctionNodeandToolNodewas duplicated. Each nodevalidated its output inline (inside
wrappedFn/runTool) and thescheduler validated it again scheduler-side via
BaseNode.ValidateOutput(introduced in #970). The same
OutputSchemawas therefore checked twice onevery activation.
Beyond the redundancy, the two copies are a drift hazard: as the
scheduler-side helper evolves (e.g. #970 added the
*genai.Contenttextfallback and passthrough for framework control values), the inline copies fall
behind and the two paths diverge. The validation behaviour also stopped being
uniform across node types — custom
Nodeimplementations only ever went throughthe scheduler, while
FunctionNode/ToolNodecarried their own logic.The FunctionTool
{"result": X}unwrap convention was buried inline insideToolNode.runTool, mixing a tool-specific quirk into the execution path insteadof expressing it as part of the validation contract.
Solution
Make the scheduler the single, authoritative output-validation point and remove
the inline validation from the node run paths:
FunctionNode: drop the inlineOutputSchemacheck from all threeconstructors (
newFunctionNodeWithResolvedSchemas,NewFunctionNodeFromState,and the emitting variant). The schema still flows through
BaseNode, so thescheduler enforces it via
BaseNode.ValidateOutput.wrappedFnis back todoing one thing: materialise the typed parameter and call the user function.
ToolNode: remove the inline validation fromrunTooland move theFunctionTool
{"result": X}unwrap into a dedicatedToolNode.ValidateOutputoverride. The override tries standard validation first, falls back to
unwrapping
{"result": X}, and re-runs standard validation on total failureso the caller still sees the original schema-mismatch error. This is the right
home for the convention because it is tool-specific — making it a general
default could mask genuine validation errors in other node types.
Behaviour note
Validation errors now surface scheduler-side rather than inside
Run. For realworkflows this is unobservable (every activation goes through the scheduler).
Callers that invoke
node.Run()directly (mostly tests) now receive the rawoutput and must call
ValidateOutputthemselves. Tests are split accordingly:Run-level tests assert raw passthrough; validation is asserted viaValidateOutput(TestFunctionNode_ValidateOutput,TestToolNode_ValidateOutputcovering direct-valid,{"result": X}unwrap,fail-both-paths, and nil-schema cases).