refactor: replace PARTIAL_SUCCESS with clear SUCCESS/FAILED/ERROR execution status (#521)

Co-authored-by: Wei Lin <wei@avaprotocol.org>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Author: 3 people

core/taskengine/engine.go

@@ -3051,7 +3051,7 @@ func (n *Engine) SimulateTask(user *model.User, trigger *avsproto.TaskTrigger, n
 	}
 
 	// Step 10: Analyze execution results from all steps
-	_, executionError, failedStepCount, resultStatus := vm.AnalyzeExecutionResult()
+	executionError, failedStepCount, resultStatus := vm.AnalyzeExecutionResult()
 
 	// Step 11: Calculate total gas cost for the workflow
 
@@ -3073,20 +3073,7 @@ func (n *Engine) SimulateTask(user *model.User, trigger *avsproto.TaskTrigger, n
 	switch resultStatus {
 	case ExecutionSuccess:
 		n.logger.Info("workflow simulation completed successfully", "task_id", task.Id, "simulation_id", simulationID, "steps", len(execution.Steps))
-	case ExecutionPartialSuccess:
-		// Clean up error message to avoid stack traces in logs
-		cleanErrorMsg := executionError
-		stackTraceRegex := regexp.MustCompile(`(?m)^\s*at .*$`)
-		cleanErrorMsg = stackTraceRegex.ReplaceAllString(cleanErrorMsg, "")
-		cleanErrorMsg = strings.TrimSpace(cleanErrorMsg)
-
-		n.logger.Warn("workflow simulation completed with partial success",
-			"error", cleanErrorMsg,
-			"task_id", task.Id,
-			"simulation_id", simulationID,
-			"failed_steps", failedStepCount,
-			"total_steps", len(vm.ExecutionLogs))
-	case ExecutionFailure:
+	case ExecutionFailed:
 		// Clean up error message to avoid stack traces in logs
 		cleanErrorMsg := executionError
 		stackTraceRegex := regexp.MustCompile(`(?m)^\s*at .*$`)
@@ -3103,12 +3090,10 @@ func (n *Engine) SimulateTask(user *model.User, trigger *avsproto.TaskTrigger, n
 
 	// Handle VM-level errors if they occurred
 	if runErr != nil {
-		// This should not happen if AnalyzeExecutionResult is working correctly,
-		// but handle it as a fallback for VM-level errors
 		n.logger.Error("workflow simulation had VM-level error", "vm_error", runErr, "task_id", task.Id, "simulation_id", simulationID)
 		if execution.Error == "" {
 			execution.Error = fmt.Sprintf("VM execution error: %s", runErr.Error())
-			execution.Status = avsproto.ExecutionStatus_EXECUTION_STATUS_FAILED
+			execution.Status = avsproto.ExecutionStatus_EXECUTION_STATUS_ERROR
 		}
 	}
 

core/taskengine/executor.go

@@ -618,7 +618,7 @@ func (x *TaskExecutor) RunTask(task *model.Task, queueData *QueueExecutionData)
 	}
 
 	// Analyze execution results from all steps (including failed ones)
-	_, executionError, failedStepCount, resultStatus := vm.AnalyzeExecutionResult()
+	executionError, failedStepCount, resultStatus := vm.AnalyzeExecutionResult()
 
 	// Calculate total gas cost for the workflow
 
@@ -652,14 +652,7 @@ func (x *TaskExecutor) RunTask(task *model.Task, queueData *QueueExecutionData)
 	switch resultStatus {
 	case ExecutionSuccess:
 		x.logger.Info("task execution completed successfully", "task_id", task.Id, "execution_id", queueData.ExecutionID, "total_steps", len(vm.ExecutionLogs))
-	case ExecutionPartialSuccess:
-		x.logger.Warn("task execution completed with partial success",
-			"error", executionError,
-			"task_id", task.Id,
-			"execution_id", queueData.ExecutionID,
-			"failed_steps", failedStepCount,
-			"total_steps", len(vm.ExecutionLogs))
-	case ExecutionFailure:
+	case ExecutionFailed:
 		x.logger.Error("task execution completed with failures",
 			"error", executionError,
 			"task_id", task.Id,
@@ -669,12 +662,10 @@ func (x *TaskExecutor) RunTask(task *model.Task, queueData *QueueExecutionData)
 	}
 
 	if runTaskErr != nil {
-		// This should not happen if AnalyzeExecutionResult is working correctly,
-		// but handle it as a fallback for VM-level errors
 		x.logger.Error("task execution had VM-level error", "vm_error", runTaskErr, "task_id", task.Id, "execution_id", queueData.ExecutionID)
 		if execution.Error == "" {
 			execution.Error = fmt.Sprintf("VM execution error: %s", runTaskErr.Error())
-			execution.Status = avsproto.ExecutionStatus_EXECUTION_STATUS_FAILED
+			execution.Status = avsproto.ExecutionStatus_EXECUTION_STATUS_ERROR
 		}
 	}
 
@@ -732,10 +723,8 @@ func (x *TaskExecutor) RunTask(task *model.Task, queueData *QueueExecutionData)
 	switch resultStatus {
 	case ExecutionSuccess:
 		x.logger.Info("successfully executing task", "task_id", task.Id, "triggermark", queueData)
-	case ExecutionPartialSuccess:
-		x.logger.Info("task execution completed with partial success", "task_id", task.Id, "failed_steps", failedStepCount, "triggermark", queueData)
-	default: // ExecutionFailure or other
-		x.logger.Warn("task execution completed with step failures", "task_id", task.Id, "failed_steps", failedStepCount)
+	case ExecutionFailed:
+		x.logger.Warn("task execution completed with step failures", "task_id", task.Id, "failed_steps", failedStepCount, "triggermark", queueData)
 	}
 
 	return execution, nil

core/taskengine/executor_test.go

@@ -3,7 +3,6 @@ package taskengine
 import (
 	"net/http"
 	"net/http/httptest"
-	"strings"
 	"testing"
 	"time"
 
@@ -225,14 +224,15 @@ func TestExecutorRunTaskWithBranchSilentFailureBehavior(t *testing.T) {
 		t.Errorf("Expected no error with silent failure behavior, but got: %v", err)
 	}
 
-	// Branch workflows with skipped nodes should report PARTIAL_SUCCESS
-	if execution.Status != avsproto.ExecutionStatus_EXECUTION_STATUS_PARTIAL_SUCCESS {
-		t.Errorf("Expected partial success status (branch path with skipped nodes), but got: %v with error: %s", execution.Status, execution.Error)
+	// Branch workflows with skipped nodes are SUCCESS — the workflow executed its
+	// chosen path correctly; skipping nodes due to branching is expected behavior.
+	if execution.Status != avsproto.ExecutionStatus_EXECUTION_STATUS_SUCCESS {
+		t.Errorf("Expected success status (branch path with skipped nodes is normal), but got: %v with error: %s", execution.Status, execution.Error)
 	}
 
-	// Should have a partial execution message explaining the branching
-	if execution.Error == "" || !strings.Contains(execution.Error, "Partial execution") {
-		t.Errorf("Expected partial execution message, but got: %s", execution.Error)
+	// No error when all executed steps succeeded
+	if execution.Error != "" {
+		t.Errorf("Expected empty error for successful branch execution, but got: %s", execution.Error)
 	}
 
 	// Find the branch step regardless of ordering

core/taskengine/partial_success_test.go

@@ -39,12 +39,9 @@ func TestAnalyzeExecutionResult_AllSuccess(t *testing.T) {
 		},
 	}
 
-	success, errorMessage, failedCount, resultStatus := vm.AnalyzeExecutionResult()
+	errorMessage, failedCount, resultStatus := vm.AnalyzeExecutionResult()
 
 	// Verify results
-	if !success {
-		t.Errorf("Expected success=true, got success=%v", success)
-	}
 	if errorMessage != "" {
 		t.Errorf("Expected empty error message, got: %s", errorMessage)
 	}
@@ -56,8 +53,8 @@ func TestAnalyzeExecutionResult_AllSuccess(t *testing.T) {
 	}
 }
 
-// TestAnalyzeExecutionResult_PartialSuccess tests the case where some steps succeed and some fail
-func TestAnalyzeExecutionResult_PartialSuccess(t *testing.T) {
+// TestAnalyzeExecutionResult_SomeStepsFailed tests the case where some steps succeed and some fail
+func TestAnalyzeExecutionResult_SomeStepsFailed(t *testing.T) {
 	vm := NewVM()
 	vm.logger = testutil.GetLogger()
 
@@ -89,20 +86,17 @@ func TestAnalyzeExecutionResult_PartialSuccess(t *testing.T) {
 		},
 	}
 
-	success, errorMessage, failedCount, resultStatus := vm.AnalyzeExecutionResult()
+	errorMessage, failedCount, resultStatus := vm.AnalyzeExecutionResult()
 
 	// Verify results
-	if success {
-		t.Errorf("Expected success=false for partial success, got success=%v", success)
-	}
 	if errorMessage == "" {
-		t.Errorf("Expected non-empty error message for partial success")
+		t.Errorf("Expected non-empty error message when some steps failed")
 	}
 	if failedCount != 1 {
 		t.Errorf("Expected failedCount=1, got failedCount=%d", failedCount)
 	}
-	if resultStatus != ExecutionPartialSuccess {
-		t.Errorf("Expected resultStatus=ExecutionPartialSuccess, got resultStatus=%v", resultStatus)
+	if resultStatus != ExecutionFailed {
+		t.Errorf("Expected resultStatus=ExecutionFailed, got resultStatus=%v", resultStatus)
 	}
 
 	// Check that error message contains failure information
@@ -139,20 +133,17 @@ func TestAnalyzeExecutionResult_AllFailure(t *testing.T) {
 		},
 	}
 
-	success, errorMessage, failedCount, resultStatus := vm.AnalyzeExecutionResult()
+	errorMessage, failedCount, resultStatus := vm.AnalyzeExecutionResult()
 
 	// Verify results
-	if success {
-		t.Errorf("Expected success=false for all failures, got success=%v", success)
-	}
 	if errorMessage == "" {
 		t.Errorf("Expected non-empty error message for all failures")
 	}
 	if failedCount != 3 {
 		t.Errorf("Expected failedCount=3, got failedCount=%d", failedCount)
 	}
-	if resultStatus != ExecutionFailure {
-		t.Errorf("Expected resultStatus=ExecutionFailure, got resultStatus=%v", resultStatus)
+	if resultStatus != ExecutionFailed {
+		t.Errorf("Expected resultStatus=ExecutionFailed, got resultStatus=%v", resultStatus)
 	}
 
 	// Check that error message contains failure information
@@ -170,25 +161,22 @@ func TestAnalyzeExecutionResult_NoSteps(t *testing.T) {
 	// No execution logs
 	vm.ExecutionLogs = []*avsproto.Execution_Step{}
 
-	success, errorMessage, failedCount, resultStatus := vm.AnalyzeExecutionResult()
+	errorMessage, failedCount, resultStatus := vm.AnalyzeExecutionResult()
 
 	// Verify results
-	if success {
-		t.Errorf("Expected success=false for no steps, got success=%v", success)
-	}
 	if errorMessage != "no execution steps found" {
 		t.Errorf("Expected specific error message for no steps, got: %s", errorMessage)
 	}
 	if failedCount != 0 {
 		t.Errorf("Expected failedCount=0 for no steps, got failedCount=%d", failedCount)
 	}
-	if resultStatus != ExecutionFailure {
-		t.Errorf("Expected resultStatus=ExecutionFailure for no steps, got resultStatus=%v", resultStatus)
+	if resultStatus != ExecutionFailed {
+		t.Errorf("Expected resultStatus=ExecutionFailed for no steps, got resultStatus=%v", resultStatus)
 	}
 }
 
-// TestGetExecutionStatus_PartialSuccess tests the GetExecutionStatus method for partial success
-func TestGetExecutionStatus_PartialSuccess(t *testing.T) {
+// TestGetExecutionStatus_StepFailures tests the GetExecutionStatus method when some steps fail
+func TestGetExecutionStatus_StepFailures(t *testing.T) {
 	// Set up test database and engine
 	db := testutil.TestMustDB()
 	defer storage.Destroy(db.(*storage.BadgerStorage))
@@ -209,13 +197,13 @@ func TestGetExecutionStatus_PartialSuccess(t *testing.T) {
 		},
 	}
 
-	// Create execution with partial success (some steps succeed, some fail)
+	// Create execution where some steps succeed and some fail
 	execution := &avsproto.Execution{
 		Id:      "test-execution-id",
 		StartAt: time.Now().UnixMilli(),
 		EndAt:   time.Now().UnixMilli(),
-		Status:  avsproto.ExecutionStatus_EXECUTION_STATUS_PARTIAL_SUCCESS, // Overall status is partial success
-		Error:   "Partial success: 1 of 3 steps failed: Database Query",
+		Status:  avsproto.ExecutionStatus_EXECUTION_STATUS_FAILED,
+		Error:   "1 of 3 steps failed: Database Query",
 		Index:   0, // First execution
 		Steps: []*avsproto.Execution_Step{
 			{
@@ -269,9 +257,9 @@ func TestGetExecutionStatus_PartialSuccess(t *testing.T) {
 		t.Fatalf("GetExecutionStatus failed: %v", err)
 	}
 
-	// Verify that it returns PARTIAL_SUCCESS status
-	if statusResp.Status != avsproto.ExecutionStatus_EXECUTION_STATUS_PARTIAL_SUCCESS {
-		t.Errorf("Expected EXECUTION_STATUS_PARTIAL_SUCCESS, got %v", statusResp.Status)
+	// Verify that it returns FAILED status (some steps failed)
+	if statusResp.Status != avsproto.ExecutionStatus_EXECUTION_STATUS_FAILED {
+		t.Errorf("Expected EXECUTION_STATUS_FAILED, got %v", statusResp.Status)
 	}
 }
 

core/taskengine/vm.go

@@ -3208,12 +3208,12 @@ func (v *VM) createExecutionStep(nodeId string, success bool, errorMsg string, l
 type ExecutionResultStatus int
 
 const (
-	// ExecutionSuccess indicates all steps completed successfully
+	// ExecutionSuccess indicates all executed steps succeeded (includes branch/conditional skips)
 	ExecutionSuccess ExecutionResultStatus = iota
-	// ExecutionPartialSuccess indicates some steps succeeded but at least one failed
-	ExecutionPartialSuccess
-	// ExecutionFailure indicates execution failed (all steps failed or critical failure)
-	ExecutionFailure
+	// ExecutionFailed indicates one or more node-level steps failed during execution
+	ExecutionFailed
+	// ExecutionError indicates a system-level failure (VM could not run the workflow)
+	ExecutionError
 )
 
 // getStepDisplayName extracts the display name for a step, preferring the name over ID
@@ -3225,77 +3225,36 @@ func getStepDisplayName(step *avsproto.Execution_Step) string {
 	return stepName
 }
 
-// AnalyzeExecutionResult examines all execution steps and determines overall success/failure/partial status
-// Returns (success, errorMessage, failedStepCount, resultStatus)
-func (v *VM) AnalyzeExecutionResult() (bool, string, int, ExecutionResultStatus) {
+// AnalyzeExecutionResult examines all execution steps and determines overall success/failure status.
+// Returns (errorMessage, failedStepCount, resultStatus)
+func (v *VM) AnalyzeExecutionResult() (string, int, ExecutionResultStatus) {
 	v.mu.Lock()
 	defer v.mu.Unlock()
 
 	if len(v.ExecutionLogs) == 0 {
-		return false, "no execution steps found", 0, ExecutionFailure
+		return "no execution steps found", 0, ExecutionFailed
 	}
 
 	var failedStepNames []string
-	var successfulStepNames []string
-	var firstErrorMessage string
 
 	for _, step := range v.ExecutionLogs {
-		stepName := getStepDisplayName(step)
-
 		if !step.Success && step.Error != "" {
-			if firstErrorMessage == "" {
-				firstErrorMessage = step.Error
-			}
-			failedStepNames = append(failedStepNames, stepName)
-		} else if step.Success {
-			successfulStepNames = append(successfulStepNames, stepName)
+			failedStepNames = append(failedStepNames, getStepDisplayName(step))
 		}
 	}
 
 	failedCount := len(failedStepNames)
-	totalSteps := len(v.ExecutionLogs)
-
-	// Determine execution status and success flag
-	var resultStatus ExecutionResultStatus
-	var success bool
-	var errorMessage string
 
 	if failedCount == 0 {
-		// All executed steps succeeded. However, if not all workflow steps executed
-		// (e.g., due to branch selections or conditional skips), report PARTIAL_SUCCESS
-		// to reflect that the workflow did not traverse all configured nodes.
-		executedCount := len(v.ExecutionLogs)
-		totalWorkflowSteps := 1 + len(v.TaskNodes) // 1 trigger + all nodes
-		if v.GetTaskId() == "" && len(v.TaskNodes) == 1 {
-			// single-node immediate execution
-			totalWorkflowSteps = 1
-		}
-
-		if executedCount < totalWorkflowSteps {
-			resultStatus = ExecutionPartialSuccess
-			success = false // do not mark full success when nodes were skipped
-			errorMessage = fmt.Sprintf("Partial execution: %d out of %d steps executed (branch/conditional path)", executedCount, totalWorkflowSteps)
-		} else {
-			// All steps that exist in the workflow executed and succeeded
-			resultStatus = ExecutionSuccess
-			success = true
-			errorMessage = ""
-		}
-	} else if failedCount > 0 {
-		// Distinguish between all failed vs some failed (for internal status tracking)
-		if failedCount == totalSteps {
-			// All steps failed
-			resultStatus = ExecutionFailure
-		} else {
-			// Some steps succeeded, some failed - partial success for internal tracking
-			resultStatus = ExecutionPartialSuccess
-		}
-		success = false
-		// Use simple error message format (no prefix) for both cases
-		errorMessage = formatExecutionErrorMessage("", failedCount, totalSteps, failedStepNames)
+		// All executed steps succeeded. Branch/conditional skips are normal
+		// workflow behavior and count as SUCCESS — the workflow did what it
+		// was configured to do.
+		return "", 0, ExecutionSuccess
 	}
 
-	return success, errorMessage, failedCount, resultStatus
+	// One or more steps failed (covers both partial and total failure).
+	errorMessage := formatExecutionErrorMessage("", failedCount, len(v.ExecutionLogs), failedStepNames)
+	return errorMessage, failedCount, ExecutionFailed
 }
 
 // CalculateTotalGasCost sums up gas costs from all execution steps that involve blockchain operations
@@ -3366,10 +3325,10 @@ func convertToExecutionStatus(resultStatus ExecutionResultStatus) avsproto.Execu
 	switch resultStatus {
 	case ExecutionSuccess:
 		return avsproto.ExecutionStatus_EXECUTION_STATUS_SUCCESS
-	case ExecutionPartialSuccess:
-		return avsproto.ExecutionStatus_EXECUTION_STATUS_PARTIAL_SUCCESS
-	case ExecutionFailure:
+	case ExecutionFailed:
 		return avsproto.ExecutionStatus_EXECUTION_STATUS_FAILED
+	case ExecutionError:
+		return avsproto.ExecutionStatus_EXECUTION_STATUS_ERROR
 	default:
 		return avsproto.ExecutionStatus_EXECUTION_STATUS_UNSPECIFIED
 	}