Fix Docker agent test JSON-marshal race by making the hook per-Agent

Same class of race fixed for newTimerFn in 8e5ef365d: tests swapped the
package-level jsonMarshalFn hook while async goroutines leaked from
earlier tests (sendCommandAck ack retries via runAsync) could still be
reading it, tripping the race detector. Replace the global with a
per-Agent jsonMarshalFn seam (nil defaults to json.Marshal), make the
decode payload helpers Agent methods so they use it, and inject the
failing marshaller into the tests that previously swapped the global.
Verified with go test ./internal/dockeragent/ -race -count=20.

Contract-Neutral: test seam refactor to fix data race, no public contract delta

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

internal/dockeragent/agent.go

@@ -135,6 +135,7 @@ type Agent struct {
 	manualCheckResults  map[string]manualUpdateCheckResult
 	manualCheckCollect  func(context.Context) (agentsdocker.Report, error) // test seam for bounded manual checks
 	newTimerFn          func(time.Duration) *time.Timer                    // test seam; per-Agent so async goroutines never read a shared global (nil = time.NewTimer)
+	jsonMarshalFn       func(any) ([]byte, error)                          // test seam; per-Agent so async goroutines never read a shared global (nil = json.Marshal)
 	backgroundMu        sync.Mutex                                         // protects updateCheckRunning, cleanupTaskRunning
 	updateCheckRunning  bool
 	cleanupTaskRunning  bool
@@ -819,6 +820,13 @@ func (a *Agent) newTimer(delay time.Duration) *time.Timer {
 	return time.NewTimer(delay)
 }
 
+func (a *Agent) jsonMarshal(v any) ([]byte, error) {
+	if a.jsonMarshalFn != nil {
+		return a.jsonMarshalFn(v)
+	}
+	return json.Marshal(v)
+}
+
 func (a *Agent) waitForAsyncDelay(delay time.Duration) bool {
 	if delay <= 0 {
 		return true
@@ -1524,7 +1532,7 @@ func (a *Agent) sendCommandAckWithPayload(ctx context.Context, target TargetConf
 		Payload: payload,
 	}
 
-	body, err := jsonMarshalFn(ackPayload)
+	body, err := a.jsonMarshal(ackPayload)
 	if err != nil {
 		return fmt.Errorf("marshal command acknowledgement: %w", err)
 	}

internal/dockeragent/agent_http_test.go

@@ -364,11 +364,12 @@ func TestSendCommandAck(t *testing.T) {
 	})
 
 	t.Run("marshal error", func(t *testing.T) {
-		swap(t, &jsonMarshalFn, func(any) ([]byte, error) {
-			return nil, errors.New("marshal failed")
-		})
-
-		agent := &Agent{hostID: "host1"}
+		agent := &Agent{
+			hostID: "host1",
+			jsonMarshalFn: func(any) ([]byte, error) {
+				return nil, errors.New("marshal failed")
+			},
+		}
 		if err := agent.sendCommandAck(context.Background(), TargetConfig{URL: "http://example"}, "cmd", "status", "msg"); err == nil {
 			t.Fatal("expected error")
 		}

internal/dockeragent/container_update.go

@@ -38,10 +38,10 @@ type updateContainerCommandPayload struct {
 	ContainerID string `json:"containerId"`
 }
 
-func decodeUpdateContainerPayload(payload map[string]any) (updateContainerCommandPayload, error) {
+func (a *Agent) decodeUpdateContainerPayload(payload map[string]any) (updateContainerCommandPayload, error) {
 	var commandPayload updateContainerCommandPayload
 
-	body, err := jsonMarshalFn(payload)
+	body, err := a.jsonMarshal(payload)
 	if err != nil {
 		return commandPayload, fmt.Errorf("marshal update command payload: %w", err)
 	}
@@ -189,7 +189,7 @@ func daemonGeneratedHostname(hostname, containerID string) bool {
 
 // handleUpdateContainerCommand handles the update_container command from Pulse.
 func (a *Agent) handleUpdateContainerCommand(ctx context.Context, target TargetConfig, command agentsdocker.Command) error {
-	commandPayload, err := decodeUpdateContainerPayload(command.Payload)
+	commandPayload, err := a.decodeUpdateContainerPayload(command.Payload)
 	if err != nil {
 		a.logger.Error().Err(err).Msg("Update command missing or invalid containerId in payload")
 		if err := a.sendCommandAck(ctx, target, command.ID, agentsdocker.CommandStatusFailed, "Missing containerId in payload"); err != nil {

internal/dockeragent/deps.go

@@ -3,7 +3,6 @@ package dockeragent
 import (
 	"context"
 	"crypto/rand"
-	"encoding/json"
 	"io"
 	"os"
 	"os/exec"
@@ -22,7 +21,6 @@ var (
 	randomDurationFn         = randomDuration
 	nowFn                    = time.Now
 	sleepFn                  = time.Sleep
-	jsonMarshalFn            = json.Marshal
 	normalizeTargetsFn       = normalizeTargets
 	buildRuntimeCandidatesFn = buildRuntimeCandidates
 	tryRuntimeCandidateFn    = tryRuntimeCandidate

internal/dockeragent/update_all.go

@@ -14,10 +14,10 @@ type updateAllCommandPayload struct {
 	ContainerIDs []string `json:"containerIds"`
 }
 
-func decodeUpdateAllPayload(payload map[string]any) (updateAllCommandPayload, error) {
+func (a *Agent) decodeUpdateAllPayload(payload map[string]any) (updateAllCommandPayload, error) {
 	var commandPayload updateAllCommandPayload
 
-	body, err := jsonMarshalFn(payload)
+	body, err := a.jsonMarshal(payload)
 	if err != nil {
 		return commandPayload, fmt.Errorf("marshal update_all command payload: %w", err)
 	}
@@ -54,7 +54,7 @@ func decodeUpdateAllPayload(payload map[string]any) (updateAllCommandPayload, er
 // handleUpdateAllCommand handles the update_all command from Pulse.
 // It updates each container sequentially to avoid overloading the runtime and registry.
 func (a *Agent) handleUpdateAllCommand(ctx context.Context, target TargetConfig, command agentsdocker.Command) error {
-	commandPayload, err := decodeUpdateAllPayload(command.Payload)
+	commandPayload, err := a.decodeUpdateAllPayload(command.Payload)
 	if err != nil {
 		a.logger.Error().Err(err).Msg("Update-all command missing or invalid containerIds in payload")
 		if err := a.sendCommandAck(ctx, target, command.ID, agentsdocker.CommandStatusFailed, "Missing containerIds in payload"); err != nil {

internal/dockeragent/update_all_decode_branchcov0723am_test.go

@@ -11,7 +11,7 @@ import (
 
 func TestBranchcov0723AmDecodeUpdateAllPayload(t *testing.T) {
 	t.Run("nil map yields missing-containerIds error and zero-value struct", func(t *testing.T) {
-		got, err := decodeUpdateAllPayload(nil)
+		got, err := (&Agent{}).decodeUpdateAllPayload(nil)
 		if err == nil {
 			t.Fatal("expected error for nil payload")
 		}
@@ -24,7 +24,7 @@ func TestBranchcov0723AmDecodeUpdateAllPayload(t *testing.T) {
 	})
 
 	t.Run("empty map yields missing-containerIds error", func(t *testing.T) {
-		got, err := decodeUpdateAllPayload(map[string]any{})
+		got, err := (&Agent{}).decodeUpdateAllPayload(map[string]any{})
 		if err == nil {
 			t.Fatal("expected error for empty payload")
 		}
@@ -42,7 +42,7 @@ func TestBranchcov0723AmDecodeUpdateAllPayload(t *testing.T) {
 		// defaulting and reports "missing containerIds" for both. The
 		// returned structs do differ (nil vs a non-nil empty slice), which
 		// simply reflects what encoding/json left on the field.
-		absent, errAbsent := decodeUpdateAllPayload(map[string]any{"other": "x"})
+		absent, errAbsent := (&Agent{}).decodeUpdateAllPayload(map[string]any{"other": "x"})
 		if errAbsent == nil {
 			t.Fatal("expected error when containerIds is absent")
 		}
@@ -53,7 +53,7 @@ func TestBranchcov0723AmDecodeUpdateAllPayload(t *testing.T) {
 			t.Fatalf("absent field should leave ContainerIDs nil, got %+v", absent.ContainerIDs)
 		}
 
-		emptySlice, errEmpty := decodeUpdateAllPayload(map[string]any{"containerIds": []any{}})
+		emptySlice, errEmpty := (&Agent{}).decodeUpdateAllPayload(map[string]any{"containerIds": []any{}})
 		if errEmpty == nil {
 			t.Fatal("expected error for empty containerIds slice")
 		}
@@ -72,7 +72,7 @@ func TestBranchcov0723AmDecodeUpdateAllPayload(t *testing.T) {
 		// struct still carries the RAW decoded slice (the `= normalized`
 		// assignment only runs on success).
 		raw := []any{"", "  ", "\t"}
-		got, err := decodeUpdateAllPayload(map[string]any{"containerIds": raw})
+		got, err := (&Agent{}).decodeUpdateAllPayload(map[string]any{"containerIds": raw})
 		if err == nil {
 			t.Fatal("expected error")
 		}
@@ -86,11 +86,11 @@ func TestBranchcov0723AmDecodeUpdateAllPayload(t *testing.T) {
 
 	t.Run("marshal error is wrapped", func(t *testing.T) {
 		marshalErr := errors.New("boom")
-		swap(t, &jsonMarshalFn, func(any) ([]byte, error) {
+		agent := &Agent{jsonMarshalFn: func(any) ([]byte, error) {
 			return nil, marshalErr
-		})
+		}}
 
-		got, err := decodeUpdateAllPayload(map[string]any{"containerIds": []any{"a"}})
+		got, err := agent.decodeUpdateAllPayload(map[string]any{"containerIds": []any{"a"}})
 		if err == nil {
 			t.Fatal("expected error")
 		}
@@ -106,7 +106,7 @@ func TestBranchcov0723AmDecodeUpdateAllPayload(t *testing.T) {
 	})
 
 	t.Run("wrong field type yields decode error", func(t *testing.T) {
-		got, err := decodeUpdateAllPayload(map[string]any{"containerIds": 123})
+		got, err := (&Agent{}).decodeUpdateAllPayload(map[string]any{"containerIds": 123})
 		if err == nil {
 			t.Fatal("expected error")
 		}
@@ -122,7 +122,7 @@ func TestBranchcov0723AmDecodeUpdateAllPayload(t *testing.T) {
 	})
 
 	t.Run("valid single id", func(t *testing.T) {
-		got, err := decodeUpdateAllPayload(map[string]any{"containerIds": []any{"a"}})
+		got, err := (&Agent{}).decodeUpdateAllPayload(map[string]any{"containerIds": []any{"a"}})
 		if err != nil {
 			t.Fatalf("unexpected error: %v", err)
 		}
@@ -132,7 +132,7 @@ func TestBranchcov0723AmDecodeUpdateAllPayload(t *testing.T) {
 	})
 
 	t.Run("multiple ids preserve input order", func(t *testing.T) {
-		got, err := decodeUpdateAllPayload(map[string]any{"containerIds": []any{"b", "a"}})
+		got, err := (&Agent{}).decodeUpdateAllPayload(map[string]any{"containerIds": []any{"b", "a"}})
 		if err != nil {
 			t.Fatalf("unexpected error: %v", err)
 		}
@@ -142,7 +142,7 @@ func TestBranchcov0723AmDecodeUpdateAllPayload(t *testing.T) {
 	})
 
 	t.Run("surrounding whitespace is trimmed", func(t *testing.T) {
-		got, err := decodeUpdateAllPayload(map[string]any{"containerIds": []any{"  a  "}})
+		got, err := (&Agent{}).decodeUpdateAllPayload(map[string]any{"containerIds": []any{"  a  "}})
 		if err != nil {
 			t.Fatalf("unexpected error: %v", err)
 		}
@@ -152,7 +152,7 @@ func TestBranchcov0723AmDecodeUpdateAllPayload(t *testing.T) {
 	})
 
 	t.Run("exact duplicates are deduped", func(t *testing.T) {
-		got, err := decodeUpdateAllPayload(map[string]any{"containerIds": []any{"a", "a"}})
+		got, err := (&Agent{}).decodeUpdateAllPayload(map[string]any{"containerIds": []any{"a", "a"}})
 		if err != nil {
 			t.Fatalf("unexpected error: %v", err)
 		}
@@ -164,7 +164,7 @@ func TestBranchcov0723AmDecodeUpdateAllPayload(t *testing.T) {
 	t.Run("duplicates after trim are deduped", func(t *testing.T) {
 		// Verifies the source orders trim-before-dedupe: " a " collapses to
 		// "a" and is then deduped against the literal "a".
-		got, err := decodeUpdateAllPayload(map[string]any{"containerIds": []any{" a ", "a"}})
+		got, err := (&Agent{}).decodeUpdateAllPayload(map[string]any{"containerIds": []any{" a ", "a"}})
 		if err != nil {
 			t.Fatalf("unexpected error: %v", err)
 		}