fix: loop node reports iteration failures as partial success (#518)

Co-authored-by: Wei Lin <wei@avaprotocol.org>

Author: chrisli30

core/taskengine/manual_trigger_loop_test.go

@@ -426,14 +426,15 @@ func TestLoopNode_ContractWrite_InvalidAddress_PartialFailure(t *testing.T) {
 	node.Name = "loopTransfer"
 
 	step, err := vm.RunNodeWithInputs(node, inputVariables)
+	require.NoError(t, err, "loop infrastructure should not fail")
 
-	// Per AvaProtocol/EigenLayer-AVS#511, per-iteration runner failures do not
-	// fail the loop step. The loop ran to completion, so step.Success is true
-	// and the per-iteration outcomes are reflected in the data array (nil for
-	// failed iterations).
+	// When some iterations fail, the loop step reports success=false with a
+	// descriptive error so that AnalyzeExecutionResult detects partial_success.
+	// The loop still ran to completion and preserves OutputData with nil entries
+	// for failed iterations.
 	require.NotNil(t, step, "Execution step should not be nil")
-	assert.True(t, step.Success, "Loop step should succeed when it ran to completion, even if an iteration failed")
-	assert.Empty(t, step.Error, "Loop step error should be empty when loop ran to completion")
+	assert.False(t, step.Success, "Loop step should report failure when iterations failed")
+	assert.Contains(t, step.Error, "1 of 2 iterations failed", "Error should describe the partial failure")
 
 	// The output should still contain results (first iteration data, second nil)
 	loopOutput := step.GetLoop()

core/taskengine/vm.go

@@ -4840,16 +4840,44 @@ func (v *VM) executeLoopWithQueue(stepID string, taskNode *avsproto.TaskNode, no
 	}
 
 	// Only treat infrastructure failures (queue submit errors, iteration timeouts)
-	// as a hard step failure. Per-iteration runner errors (e.g. a contract call
-	// reverting in one iteration of a Loop > ContractRead) are reflected as nil
-	// entries in the results array — the loop ran to completion, so we preserve
-	// OutputData and return success so the client can inspect partial results.
-	// See AvaProtocol/EigenLayer-AVS#511.
+	// as a hard step failure.
 	if infraFailure && firstError != nil {
 		finalizeStep(s, false, nil, firstError.Error(), log.String())
 		return s, firstError
 	}
 
+	// Count successful vs failed iterations to determine step status.
+	// Per-iteration runner errors (e.g. a contract call reverting) are reflected
+	// as nil entries in the results array. The loop ran to completion, so we
+	// always preserve OutputData for the client to inspect partial results.
+	// See AvaProtocol/EigenLayer-AVS#511.
+	iterationFailCount := 0
+	for _, result := range results {
+		if result == nil {
+			iterationFailCount++
+		}
+	}
+
+	if iterationFailCount > 0 && firstError != nil {
+		// Some or all iterations failed — mark the loop step as failed so
+		// AnalyzeExecutionResult can detect partial_success at the execution level.
+		// Pass the error via the `err` parameter (not `errorMessage`) so that
+		// finalizeStep uses err.Error() directly without wrapping it in
+		// NewInvalidRequestError which adds an "invalid request: " prefix.
+		innerMsg := strings.TrimPrefix(firstError.Error(), "invalid request: ")
+		errorMsg := fmt.Sprintf("%d of %d iterations failed: %s", iterationFailCount, len(results), innerMsg)
+		loopErr := NewStructuredError(
+			avsproto.ErrorCode_INVALID_REQUEST,
+			errorMsg,
+			map[string]interface{}{
+				"failed_iterations": iterationFailCount,
+				"total_iterations":  len(results),
+			},
+		)
+		finalizeStep(s, false, loopErr, "", log.String())
+		return s, nil // return nil error: the loop itself ran to completion
+	}
+
 	finalizeStep(s, success, nil, "", log.String())
 	return s, nil
 }