From 331979b887df761c14be6d59d4784727b1627a70 Mon Sep 17 00:00:00 2001
From: Earl Warren <contact@earl-warren.org>
Date: Thu, 18 Sep 2025 16:17:56 +0000
Subject: [PATCH] fix: outputs are only set when the job is successful (#1017)

Resolves forgejo/runner#1016

<!--start release-notes-assistant-->
<!--URL:https://code.forgejo.org/forgejo/runner-->
- bug fixes
  - [PR](https://code.forgejo.org/forgejo/runner/pulls/1017): <!--number 1017 --><!--line 0 --><!--description Zml4OiBvdXRwdXRzIGFyZSBvbmx5IHNldCB3aGVuIHRoZSBqb2IgaXMgc3VjY2Vzc2Z1bA==-->fix: outputs are only set when the job is successful<!--description-->
<!--end release-notes-assistant-->

Reviewed-on: https://code.forgejo.org/forgejo/runner/pulls/1017
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>
---
 internal/pkg/report/reporter.go      | 10 ++++++----
 internal/pkg/report/reporter_test.go | 14 ++++++++++++++
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/internal/pkg/report/reporter.go b/internal/pkg/report/reporter.go
index 0d1ebadb..e6322128 100644
--- a/internal/pkg/report/reporter.go
+++ b/internal/pkg/report/reporter.go
@@ -130,10 +130,12 @@ func (r *Reporter) Fire(entry *log.Entry) error {
 					}
 				}
 			}
-			if v, ok := entry.Data["jobOutputs"]; ok {
-				_ = r.setOutputs(v.(map[string]string))
-			} else {
-				log.Panicf("received log entry with jobResult, but without jobOutputs -- outputs will be corrupted for this job")
+			if r.state.Result == runnerv1.Result_RESULT_SUCCESS {
+				if v, ok := entry.Data["jobOutputs"]; ok {
+					_ = r.setOutputs(v.(map[string]string))
+				} else {
+					log.Panicf("received log entry with successful jobResult, but without jobOutputs -- outputs will be corrupted for this job")
+				}
 			}
 		}
 		if !r.duringSteps() {
diff --git a/internal/pkg/report/reporter_test.go b/internal/pkg/report/reporter_test.go
index a949c01b..f41282cb 100644
--- a/internal/pkg/report/reporter_test.go
+++ b/internal/pkg/report/reporter_test.go
@@ -301,6 +301,20 @@ func TestReporter_Fire(t *testing.T) {
 		value, _ := reporter.outputs.Load("key1")
 		assert.EqualValues(t, "value1", value)
 	})
+
+	t.Run("jobResult jobOutputs is absent if not success", func(t *testing.T) {
+		reporter, _, _ := mockReporter(t)
+
+		dataStep0 := map[string]any{
+			"stage":      "Post",
+			"stepNumber": 0,
+			"raw_output": true,
+			"jobResult":  "skipped",
+		}
+		assert.NoError(t, reporter.Fire(&log.Entry{Message: "skipped!", Data: dataStep0}))
+
+		assert.EqualValues(t, runnerv1.Result_RESULT_SKIPPED, reporter.state.Result)
+	})
 }
 
 func TestReporterReportState(t *testing.T) {