fix(validate): reject condition edges routed on the wrong port field#8
Merged
Merged
Conversation
B24: routing is keyed exclusively on `from_port` (see
`engine::outgoing_by_port` / `handler_routing`), but nothing in
`validate::validate` checked that a `condition` node's outgoing edges
actually emit on `from_port` "true"/"false". An edge authored with the
branch label on `to_port` instead (e.g. `{from_port:"main",
to_port:"true"}` and `{from_port:"main", to_port:"false"}`) puts both
edges in the same `from_port` group, which `handler_routing`
classifies as a parallel `FanOut` — silently driving BOTH branches
unconditionally, with no validation error or routing-divergence
warning to flag the mistake.
Add a hard structural check: every outgoing edge from a `condition`
node must have `from_port` in {"true", "false"}, or `validate` now
rejects the graph with a new `ValidationError::InvalidConditionRouting`
naming the offending node and port. A condition with only one branch
wired (a common intentional "gate/filter" shape) is still accepted.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds structural validation for ChangesCondition routing validation
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
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.
Summary
Bug B24 (engine hardening): a
conditionnode's outgoing edges are routed exclusively onfrom_port(seeengine::outgoing_by_port/handler_routing/collect_input) —to_portis never consulted to decide which successor fires. Nothing invalidate::validateenforced that a condition node's edges actually declare their branch ("true"/"false") onfrom_port.An edge authored with the branch label on
to_portinstead — e.g.puts both edges into the same
from_portgroup.handler_routingsees one port group with two targets and classifies it as a parallelHandlerRouting::FanOut— it drives both successors unconditionally, with no validation error and no routing-divergence warning. The condition gate silently becomes a no-op.This surfaced live in OpenHuman: the
workflow_builderagent produced exactly this malformed shape, and a flow sent an email even though its condition field resolved tofalse. See tinyhumansai/openhuman's companion fix (B23: builder prompt clarification) and B18 (null-arg hard gate) in the same investigation — this PR is B24, the engine-side hardening that gives B23 teeth regardless of how a graph was authored (agent, UI, or import).Changes
src/error.rs: newValidationError::InvalidConditionRouting { node, from_port }variant.src/validate.rs:validate()now rejects any edge whosefrom_nodeis aconditionnode and whosefrom_portis not"true"or"false". A condition with only one branch wired (the common intentional gate/filter shape) is still accepted — only the value offrom_portis checked, not that both are present.Test plan
GGML_NATIVE=OFF cargo test --lib— all 315 tests pass (24 invalidate::tests, including 4 new: accepts correct from_port shape, accepts single-branch shape, rejects the exact to_port-mislabeled bug shape, rejects an arbitrary unrecognized from_port).engine.rs,model/mod.rs,n8n_importvia the OpenHuman host) already uses the correctfrom_portconvention — no behavior change for correctly-authored graphs.Summary by CodeRabbit
trueorfalsebranch labels.