From 475421457f6d9c1cb8f1a657f0bbefb8e88c35b3 Mon Sep 17 00:00:00 2001
From: shankar0123 <skreddy040@gmail.com>
Date: Sat, 2 May 2026 14:34:48 +0000
Subject: [PATCH] fix(test): TestBoundedFanOut_SkipsAgentRoutedDeployments race
 on seenIDs slice
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

CI race detector flagged TestBoundedFanOut_SkipsAgentRoutedDeployments
on commit 35e18bf (audit fix #9). The test's `work` closure was
appending to a plain []string slice from worker goroutines without
synchronisation:

    var seenIDs []string
    work := func(ctx context.Context, job *domain.Job) error {
        seen.Add(1)
        seenIDs = append(seenIDs, job.ID)  // race
        return nil
    }

atomic.Int64 covered the count assertion but the slice header itself
is the racing memory — race detector caught both the read+write race
on the slice header and the runtime.growslice path on append.

Fix: protect seenIDs with a sync.Mutex. The slice is only used in
the failure-message branch (`t.Errorf` ids=%v formatting), so the
contention is irrelevant to performance — correctness only.

Also locked around the read in the t.Errorf format-args evaluation,
since that read happens AFTER boundedFanOut returns (and Wait()
inside boundedFanOut synchronizes the worker goroutines), but the
explicit Lock/Unlock makes the synchronisation visible without
depending on the implicit happens-before from Wait.

The other five tests in the file (TestBoundedFanOut_CapHolds,
_AllJobsRun, _CtxCancelInterrupts, _FailedJobsCounted,
TestSetRenewalConcurrency_NormalizesNonPositive) only mutate
atomic.Int64 counters from worker goroutines, so they were
already race-clean.

Verified locally: go test -race -count=1 -run
'TestBoundedFanOut|TestSetRenewalConcurrency' ./internal/service/...
green.
---
 internal/service/job_concurrency_test.go | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/internal/service/job_concurrency_test.go b/internal/service/job_concurrency_test.go
index a710a06..e702afe 100644
--- a/internal/service/job_concurrency_test.go
+++ b/internal/service/job_concurrency_test.go
@@ -16,6 +16,7 @@ import (
 	"io"
 	"log/slog"
 	"strconv"
+	"sync"
 	"sync/atomic"
 	"testing"
 	"time"
@@ -135,10 +136,19 @@ func TestBoundedFanOut_SkipsAgentRoutedDeployments(t *testing.T) {
 	}
 
 	var seen atomic.Int64
-	var seenIDs []string
+	// seenIDs is appended from multiple worker goroutines; the
+	// surrounding mutex serialises the slice mutation so the
+	// race detector stays clean. (atomic.Int64 alone isn't
+	// sufficient: the slice header is the racing memory.)
+	var (
+		mu      sync.Mutex
+		seenIDs []string
+	)
 	work := func(ctx context.Context, job *domain.Job) error {
 		seen.Add(1)
+		mu.Lock()
 		seenIDs = append(seenIDs, job.ID)
+		mu.Unlock()
 		return nil
 	}
 
@@ -147,7 +157,9 @@ func TestBoundedFanOut_SkipsAgentRoutedDeployments(t *testing.T) {
 	}
 
 	if got := seen.Load(); got != 2 {
+		mu.Lock()
 		t.Errorf("expected 2 jobs to run (renewal + issuance, deployment-with-agent skipped), got %d (ids=%v)", got, seenIDs)
+		mu.Unlock()
 	}
 }