fix: a composite action must not change the result of the calling step before it completes (#1019)

Resolves forgejo/runner#1014

---

Manual testing can also be done using the [reproducer from the issue](https://code.forgejo.org/forgejo/runner/issues/1014#issuecomment-60694).

## Before

The first step of the local composite action sets the step result of the job to success which confuses Forgejo display.

![image](/attachments/4f5c9477-47b8-4450-a858-c86312ced946)

## After

Forgejo displays the progress of the composite action in the step calling it.

![image](/attachments/eed08b5d-b590-472f-9140-eced739dd47b)

<!--start release-notes-assistant-->
<!--URL:https://code.forgejo.org/forgejo/runner-->
- bug fixes
  - [PR](https://code.forgejo.org/forgejo/runner/pulls/1019): <!--number 1019 --><!--line 0 --><!--description Zml4OiBhIGNvbXBvc2l0ZSBhY3Rpb24gbXVzdCBub3QgY2hhbmdlIHRoZSByZXN1bHQgb2YgdGhlIGNhbGxpbmcgc3RlcCBiZWZvcmUgaXQgY29tcGxldGVz-->fix: a composite action must not change the result of the calling step before it completes<!--description-->
<!--end release-notes-assistant-->

Reviewed-on: https://code.forgejo.org/forgejo/runner/pulls/1019
Reviewed-by: Michael Kriese <michael.kriese@gmx.de>
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.code.forgejo.org>
Co-authored-by: Earl Warren <contact@earl-warren.org>
Co-committed-by: Earl Warren <contact@earl-warren.org>

act/runner/logger.go

@@ -146,6 +146,26 @@ func WithCompositeStepLogger(ctx context.Context, stepID string) context.Context
 	}).WithContext(ctx))
 }
 
+func GetOuterStepResult(entry *logrus.Entry) any {
+	r, ok := entry.Data["stepResult"]
+	if !ok {
+		return nil
+	}
+
+	// composite actions steps log with a list of stepID
+	if s, ok := entry.Data["stepID"]; ok {
+		if stepIDs, ok := s.([]string); ok {
+			if len(stepIDs) > 1 {
+				return nil
+			}
+		}
+	} else {
+		return nil
+	}
+
+	return r
+}
+
 func withStepLogger(ctx context.Context, stepNumber int, stepID, stepName, stageName string) context.Context {
 	rtn := common.Logger(ctx).WithFields(logrus.Fields{
 		"stepNumber": stepNumber,

act/runner/logger_test.go

@@ -0,0 +1,63 @@
+package runner
+
+import (
+	"testing"
+
+	"code.forgejo.org/forgejo/runner/v11/act/common"
+
+	"github.com/sirupsen/logrus/hooks/test"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestRunner_GetOuterStepResult(t *testing.T) {
+	nullLogger, hook := test.NewNullLogger()
+	ctx := common.WithLogger(t.Context(), nullLogger)
+
+	t.Run("no stepResult", func(t *testing.T) {
+		hook.Reset()
+		common.Logger(ctx).Info("✅ Success")
+		entry := hook.LastEntry()
+		require.NotNil(t, entry)
+		assert.Nil(t, GetOuterStepResult(entry))
+	})
+
+	t.Run("stepResult and no stepID", func(t *testing.T) {
+		hook.Reset()
+		common.Logger(ctx).WithField("stepResult", "success").Info("✅ Success")
+		entry := hook.LastEntry()
+		require.NotNil(t, entry)
+		assert.Nil(t, GetOuterStepResult(entry))
+	})
+
+	stepNumber := 123
+	stepID := "step id"
+	stepName := "readable name"
+	stageName := "Main"
+	ctx = withStepLogger(ctx, stepNumber, stepID, stepName, stageName)
+
+	t.Run("stepResult and stepID", func(t *testing.T) {
+		hook.Reset()
+		common.Logger(ctx).WithField("stepResult", "success").Info("✅ Success")
+		entry := hook.LastEntry()
+		actualStepIDs, ok := entry.Data["stepID"]
+		require.True(t, ok)
+		require.Equal(t, []string{stepID}, actualStepIDs)
+		require.NotNil(t, entry)
+		assert.Equal(t, "success", GetOuterStepResult(entry))
+	})
+
+	compositeStepID := "composite step id"
+	ctx = WithCompositeStepLogger(ctx, compositeStepID)
+
+	t.Run("stepResult and composite stepID", func(t *testing.T) {
+		hook.Reset()
+		common.Logger(ctx).WithField("stepResult", "success").Info("✅ Success")
+		entry := hook.LastEntry()
+		actualStepIDs, ok := entry.Data["stepID"]
+		require.True(t, ok)
+		require.Equal(t, []string{stepID, compositeStepID}, actualStepIDs)
+		require.NotNil(t, entry)
+		assert.Nil(t, GetOuterStepResult(entry))
+	})
+}

internal/pkg/report/reporter.go

@@ -13,6 +13,7 @@ import (
 	"time"
 
 	runnerv1 "code.forgejo.org/forgejo/actions-proto/runner/v1"
+	"code.forgejo.org/forgejo/runner/v11/act/runner"
 	"connectrpc.com/connect"
 	retry "github.com/avast/retry-go/v4"
 	log "github.com/sirupsen/logrus"
@@ -173,7 +174,7 @@ func (r *Reporter) Fire(entry *log.Entry) error {
 	} else if !r.duringSteps() {
 		r.logRows = appendIfNotNil(r.logRows, r.parseLogRow(entry))
 	}
-	if v, ok := entry.Data["stepResult"]; ok {
+	if v := runner.GetOuterStepResult(entry); v != nil {
 		if stepResult, ok := r.parseResult(v); ok {
 			if step.LogLength == 0 {
 				step.LogIndex = int64(r.logOffset + len(r.logRows))