From a4a6e291d5b38b1f1649e744bd611827e02bcc13 Mon Sep 17 00:00:00 2001
From: silverwind <me@silverwind.io>
Date: Thu, 19 Feb 2026 02:54:45 +0100
Subject: [PATCH] fix: protect all r.closed accesses, add race test, enable
 -race in CI
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The initial `if r.closed` guard in RunDaemon() was still reading
r.closed without holding closedM, causing a data race with Close().
Wrap it with closedM to match the other access sites.

Add TestReporter_RunDaemonClose_Race which exercises RunDaemon() and
Close() concurrently — the exact scenario from #793. Without the fix,
`go test -race` reliably detects the data race.

Enable the -race flag in `make test` so CI catches data races.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---
 Makefile                             |  2 +-
 internal/pkg/report/reporter.go      |  4 +++
 internal/pkg/report/reporter_test.go | 43 ++++++++++++++++++++++++++++
 3 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index ec22160..c6ecd8d 100644
--- a/Makefile
+++ b/Makefile
@@ -112,7 +112,7 @@ security-check: deps-tools
 	GOEXPERIMENT= $(GO) run $(GOVULNCHECK_PACKAGE) -show color ./...
 
 test: fmt-check security-check
-	@$(GO) test -v -cover -coverprofile coverage.txt ./... && echo "\n==>\033[32m Ok\033[m\n" || exit 1
+	@$(GO) test -race -v -cover -coverprofile coverage.txt ./... && echo "\n==>\033[32m Ok\033[m\n" || exit 1
 
 .PHONY: vet
 vet:
diff --git a/internal/pkg/report/reporter.go b/internal/pkg/report/reporter.go
index 6be40ae..3602eb0 100644
--- a/internal/pkg/report/reporter.go
+++ b/internal/pkg/report/reporter.go
@@ -178,9 +178,13 @@ func (r *Reporter) Fire(entry *log.Entry) error {
 }
 
 func (r *Reporter) RunDaemon() {
+	r.closedM.Lock()
 	if r.closed {
+		r.closedM.Unlock()
 		return
 	}
+	r.closedM.Unlock()
+
 	if r.ctx.Err() != nil {
 		return
 	}
diff --git a/internal/pkg/report/reporter_test.go b/internal/pkg/report/reporter_test.go
index c5a5b61..a7f0714 100644
--- a/internal/pkg/report/reporter_test.go
+++ b/internal/pkg/report/reporter_test.go
@@ -6,7 +6,9 @@ package report
 import (
 	"context"
 	"strings"
+	"sync"
 	"testing"
+	"time"
 
 	runnerv1 "code.gitea.io/actions-proto-go/runner/v1"
 	connect_go "connectrpc.com/connect"
@@ -195,3 +197,44 @@ func TestReporter_Fire(t *testing.T) {
 		assert.Equal(t, int64(3), reporter.state.Steps[0].LogLength)
 	})
 }
+
+func TestReporter_RunDaemonClose_Race(t *testing.T) {
+	client := mocks.NewClient(t)
+	client.On("UpdateLog", mock.Anything, mock.Anything).Return(
+		func(_ context.Context, req *connect_go.Request[runnerv1.UpdateLogRequest]) (*connect_go.Response[runnerv1.UpdateLogResponse], error) {
+			return connect_go.NewResponse(&runnerv1.UpdateLogResponse{
+				AckIndex: req.Msg.Index + int64(len(req.Msg.Rows)),
+			}), nil
+		},
+	)
+	client.On("UpdateTask", mock.Anything, mock.Anything).Return(
+		func(_ context.Context, req *connect_go.Request[runnerv1.UpdateTaskRequest]) (*connect_go.Response[runnerv1.UpdateTaskResponse], error) {
+			return connect_go.NewResponse(&runnerv1.UpdateTaskResponse{}), nil
+		},
+	)
+
+	ctx, cancel := context.WithCancel(context.Background())
+	taskCtx, err := structpb.NewStruct(map[string]interface{}{})
+	require.NoError(t, err)
+	reporter := NewReporter(ctx, cancel, client, &runnerv1.Task{
+		Context: taskCtx,
+	})
+	reporter.ResetSteps(1)
+
+	// Start the daemon loop in a separate goroutine.
+	// RunDaemon reads r.closed and reschedules itself via time.AfterFunc.
+	var wg sync.WaitGroup
+	wg.Add(1)
+	go func() {
+		defer wg.Done()
+		reporter.RunDaemon()
+	}()
+
+	// Close concurrently — this races with RunDaemon on r.closed.
+	assert.NoError(t, reporter.Close(""))
+
+	// Cancel context so pending AfterFunc callbacks exit quickly.
+	cancel()
+	wg.Wait()
+	time.Sleep(2 * time.Second)
+}