From 11f157c2cff50a82832ff1b19f3d404f1cea78b6 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Wed, 14 May 2025 00:27:51 +0000 Subject: [PATCH 1/4] Fixes #140 Co-Authored-By: Chris Li --- core/taskengine/vm_runner_branch.go | 28 +++++++ core/taskengine/vm_runner_branch_test.go | 98 ++++++++++++++++++++++++ 2 files changed, 126 insertions(+) diff --git a/core/taskengine/vm_runner_branch.go b/core/taskengine/vm_runner_branch.go index 51b78f66..8d9e2a34 100644 --- a/core/taskengine/vm_runner_branch.go +++ b/core/taskengine/vm_runner_branch.go @@ -32,6 +32,34 @@ func (r *BranchProcessor) Validate(node *avsproto.BranchNode) error { return fmt.Errorf("the first condition need to be an if but got :%s", node.Conditions[0].Type) } + for i, condition := range node.Conditions { + if condition == nil { + return fmt.Errorf("condition at index %d is nil", i) + } + + if condition.Id == "" { + return fmt.Errorf("condition at index %d has empty ID", i) + } + + if condition.Type == "" { + return fmt.Errorf("condition at index %d has empty type", i) + } + + if condition.Type != "if" && condition.Type != "else" { + return fmt.Errorf("condition at index %d has invalid type: %s (must be 'if' or 'else')", i, condition.Type) + } + + if condition.Type == "if" && strings.TrimSpace(condition.Expression) == "" { + return fmt.Errorf("condition at index %d has empty expression", i) + } + + if condition.Type == "else" && i < len(node.Conditions)-1 { + if r.vm.logger != nil { + r.vm.logger.Warn("'else' condition is not the last one, subsequent conditions will be ignored") + } + } + } + return nil } diff --git a/core/taskengine/vm_runner_branch_test.go b/core/taskengine/vm_runner_branch_test.go index d9404334..01571d05 100644 --- a/core/taskengine/vm_runner_branch_test.go +++ b/core/taskengine/vm_runner_branch_test.go @@ -549,3 +549,101 @@ func TestBranchNodeNoElseSkip(t *testing.T) { }) } } + +func TestBranchNodeMalformedData(t *testing.T) { + testCases := []struct { + name string + conditions []*avsproto.Condition + expectError bool + errorMsg string + }{ + { + name: "nil condition", + conditions: []*avsproto.Condition{ + {Id: "condition1", Type: "if", Expression: "a > 10"}, + nil, + }, + expectError: true, + errorMsg: "condition at index 1 is nil", + }, + { + name: "empty condition ID", + conditions: []*avsproto.Condition{ + {Id: "condition1", Type: "if", Expression: "a > 10"}, + {Id: "", Type: "if", Expression: "a > 5"}, + }, + expectError: true, + errorMsg: "condition at index 1 has empty ID", + }, + { + name: "empty condition type", + conditions: []*avsproto.Condition{ + {Id: "condition1", Type: "if", Expression: "a > 10"}, + {Id: "condition2", Type: "", Expression: "a > 5"}, + }, + expectError: true, + errorMsg: "condition at index 1 has empty type", + }, + { + name: "invalid condition type", + conditions: []*avsproto.Condition{ + {Id: "condition1", Type: "if", Expression: "a > 10"}, + {Id: "condition2", Type: "invalid", Expression: "a > 5"}, + }, + expectError: true, + errorMsg: "condition at index 1 has invalid type", + }, + { + name: "empty if expression", + conditions: []*avsproto.Condition{ + {Id: "condition1", Type: "if", Expression: "a > 10"}, + {Id: "condition2", Type: "if", Expression: ""}, + }, + expectError: true, + errorMsg: "condition at index 1 has empty expression", + }, + { + name: "whitespace if expression", + conditions: []*avsproto.Condition{ + {Id: "condition1", Type: "if", Expression: "a > 10"}, + {Id: "condition2", Type: "if", Expression: " \t\n "}, + }, + expectError: true, + errorMsg: "condition at index 1 has empty expression", + }, + { + name: "valid conditions", + conditions: []*avsproto.Condition{ + {Id: "condition1", Type: "if", Expression: "a > 10"}, + {Id: "condition2", Type: "if", Expression: "a > 5"}, + {Id: "condition3", Type: "else"}, + }, + expectError: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + vm := NewVM() + processor := NewBranchProcessor(vm) + + err := processor.Validate(&avsproto.BranchNode{ + Conditions: tc.conditions, + }) + + if tc.expectError { + if err == nil { + t.Errorf("expected error but got none") + return + } + if !strings.Contains(err.Error(), tc.errorMsg) { + t.Errorf("expected error message to contain '%s', but got: '%s'", tc.errorMsg, err.Error()) + } + } else { + if err != nil { + t.Errorf("unexpected error: %v", err) + } + } + }) + } +} From 38d95e540355d9cfbd3e7dd614b15b3ac9e3e632 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Wed, 14 May 2025 00:32:07 +0000 Subject: [PATCH 2/4] Fix validation to allow empty expressions in if conditions Co-Authored-By: Chris Li --- core/taskengine/vm_runner_branch.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/core/taskengine/vm_runner_branch.go b/core/taskengine/vm_runner_branch.go index 8d9e2a34..7407fbb4 100644 --- a/core/taskengine/vm_runner_branch.go +++ b/core/taskengine/vm_runner_branch.go @@ -49,9 +49,6 @@ func (r *BranchProcessor) Validate(node *avsproto.BranchNode) error { return fmt.Errorf("condition at index %d has invalid type: %s (must be 'if' or 'else')", i, condition.Type) } - if condition.Type == "if" && strings.TrimSpace(condition.Expression) == "" { - return fmt.Errorf("condition at index %d has empty expression", i) - } if condition.Type == "else" && i < len(node.Conditions)-1 { if r.vm.logger != nil { From a530645b801a373d46e5440a3bfcaafb54a7038a Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Wed, 14 May 2025 00:32:25 +0000 Subject: [PATCH 3/4] style: Automated gofmt Formatted Go code using gofmt. --- core/taskengine/vm_runner_branch.go | 1 - 1 file changed, 1 deletion(-) diff --git a/core/taskengine/vm_runner_branch.go b/core/taskengine/vm_runner_branch.go index 7407fbb4..b6645198 100644 --- a/core/taskengine/vm_runner_branch.go +++ b/core/taskengine/vm_runner_branch.go @@ -49,7 +49,6 @@ func (r *BranchProcessor) Validate(node *avsproto.BranchNode) error { return fmt.Errorf("condition at index %d has invalid type: %s (must be 'if' or 'else')", i, condition.Type) } - if condition.Type == "else" && i < len(node.Conditions)-1 { if r.vm.logger != nil { r.vm.logger.Warn("'else' condition is not the last one, subsequent conditions will be ignored") From 4a5d2098d73dd8c4506059c4386f2f8fb9f99695 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Wed, 14 May 2025 00:34:27 +0000 Subject: [PATCH 4/4] Update tests to allow empty expressions in if conditions Co-Authored-By: Chris Li --- core/taskengine/vm_runner_branch_test.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/core/taskengine/vm_runner_branch_test.go b/core/taskengine/vm_runner_branch_test.go index 01571d05..5ae313be 100644 --- a/core/taskengine/vm_runner_branch_test.go +++ b/core/taskengine/vm_runner_branch_test.go @@ -599,8 +599,7 @@ func TestBranchNodeMalformedData(t *testing.T) { {Id: "condition1", Type: "if", Expression: "a > 10"}, {Id: "condition2", Type: "if", Expression: ""}, }, - expectError: true, - errorMsg: "condition at index 1 has empty expression", + expectError: false, }, { name: "whitespace if expression", @@ -608,8 +607,7 @@ func TestBranchNodeMalformedData(t *testing.T) { {Id: "condition1", Type: "if", Expression: "a > 10"}, {Id: "condition2", Type: "if", Expression: " \t\n "}, }, - expectError: true, - errorMsg: "condition at index 1 has empty expression", + expectError: false, }, { name: "valid conditions",