fix: loop node reports iteration failures as partial success

[chrisli30]

Summary

• Loop node now reports success: false when any iterations fail, enabling partial_success at the execution level
• Previously, per-iteration runner errors (e.g. ERC20 revert) left the loop step as success: true with nil output entries, causing the overall execution to incorrectly report success
• Output data is still preserved with nil entries for failed iterations so clients can inspect partial results
• Error message is clean: 2 of 2 iterations failed: ERC20: transfer amount exceeds balance (no redundant prefixes)

Test plan

• TestLoopNode_ContractWrite_InvalidAddress_PartialFailure updated and passing
• All existing loop, contract write, and Tenderly tests pass
• Manually verified on local aggregator-sepolia — Telegram notification shows clean error, execution status is partial_success

[Copilot]

Pull request overview

Updates loop-node execution semantics so iteration-level runner failures cause the loop step to report success: false (while still preserving per-iteration output with nil placeholders), enabling execution-level partial_success detection.

Changes:

• Mark loop step as failed when any iteration fails, while retaining OutputData for partial inspection.
• Build a cleaner aggregated error message summarizing iteration failures.
• Update the partial-failure loop test to assert step.Success == false and validate the new error text.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
core/taskengine/vm.go	Changes loop step success/error reporting based on iteration outcomes and formats aggregated error messaging.
core/taskengine/manual_trigger_loop_test.go	Updates regression test expectations for loop partial-failure behavior and error message content.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

iterationSuccessCount is declared but never used, which will fail compilation in Go. Remove it, or use it (e.g., for logging or to build the error message).

Suggested change

	// 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.
	iterationSuccessCount := 0
	iterationFailCount := 0
	for _, result := range results {
	if result != nil {
	iterationSuccessCount++
	} else {
	// Count 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 {

[Copilot]

Iteration failures are currently inferred by counting nil entries in results. This can over-count failures when an iteration succeeds but legitimately produces a nil output (e.g., extractResultData can return nil even when step.Success is true). Consider tracking failures using the per-iteration result.Error / step.Success while collecting iteration results, rather than using result == nil as the failure signal.

Suggested change

	// 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.
	iterationSuccessCount := 0
	iterationFailCount := 0
	for _, result := range results {
	if result != nil {
	iterationSuccessCount++
	} else {
	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: ")
	loopErr := fmt.Errorf("%d of %d iterations failed: %s", iterationFailCount, len(results), innerMsg)
	// Do not infer iteration failures from nil result entries: a successful
	// iteration can legitimately produce a nil output. Infrastructure failures
	// are handled above; any remaining firstError here represents one or more
	// iteration-level failures while still preserving OutputData for the client
	// to inspect partial results.
	if 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: ")
	loopErr := fmt.Errorf("one or more iterations failed: %s", innerMsg)

[Copilot]

loopErr is created with fmt.Errorf, so finalizeStep will set step.ErrorCode to ERROR_CODE_UNSPECIFIED (since GetErrorCode only recognizes StructuredError). If clients rely on error codes, consider using NewStructuredError(avsproto.ErrorCode_INVALID_REQUEST, <message>) (or another appropriate code) to keep a clean message without the invalid request: prefix while preserving the error code.

Suggested change

	loopErr := fmt.Errorf("%d of %d iterations failed: %s", iterationFailCount, len(results), innerMsg)
	loopErr := NewStructuredError(
	avsproto.ErrorCode_INVALID_REQUEST,
	"%d of %d iterations failed: %s",
	iterationFailCount,
	len(results),
	innerMsg,
	)

[Copilot]

err is assigned but never used (step, err := vm.RunNodeWithInputs(...)), which will fail compilation. Either assert on err (likely require.NoError(t, err) if infra failures should not occur here) or explicitly ignore it with _.