Backfill agent-lifecycle governance for the marshal-hook race fix

68e557e9f moved the Docker agent's JSON-marshal hook from a package global
to a per-Agent field, which touches internal/dockeragent/agent.go and
internal/dockeragent/container_update.go. It meant to take the
Contract-Neutral bypass, but a blank line separated the trailer from
Co-Authored-By, so git's trailer parser dropped it, CI ran the completion
guard without a reason, and Canonical Governance went red on three counts:
missing contract docs/release-control/v6/internal/subsystems/agent-lifecycle.md,
missing verification artifact for "agent runtime transport trust proof", and
missing verification artifact for "Docker container recreate, rollback,
durable result, and live network-mode proof". The sibling timer-hook commit
8e5ef365d has the same defect, so this backfill covers both seams.

Document the seams in the agent-lifecycle contract's Current State: both
newTimerFn and jsonMarshalFn are unexported Agent fields reached through the
newTimer and jsonMarshal methods, nil falls back to time.NewTimer and
json.Marshal, no package-level hook global remains in internal/dockeragent,
and injection happens at construction only so the fields need no mutex. The
seams stay internal — no enrollment, transport trust, command admission,
acknowledgement, or recreate/rollback semantics move, and they must not be
promoted into NewAgent options or any server-facing surface.

Pin that shape in the two registered verification artifacts.
agent_internal_test.go asserts the field names, types, and unexported-ness,
parses every non-test source in the package to fail if either hook returns as
a package-level var, proves the nil defaults run the standard library, drives
two Agents with different hooks concurrently so the isolation is checked under
-race, and proves sendCommandAck marshals through the receiver's own seam.
container_update_test.go proves decodeUpdateContainerPayload routes through
a.jsonMarshal, that an injected failure never reaches a sibling Agent, and
pins the method form at compile time.

Guard dry-run over the staged set passes with no Contract-Neutral bypass, and
replaying 68e557e9f's file list plus these three files is green, so the
original commit would have passed had it carried them.

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

docs/release-control/v6/internal/subsystems/agent-lifecycle.md

@@ -2322,6 +2322,41 @@ manual host-local update command only for a timestamped update attempt inside
 the bounded in-flight window; a missing timestamp or a stale attempt restores
 the manual recovery handoff.
 
+### Docker agent test seams are per-Agent fields, never package globals
+
+The Docker / Podman module owns two indirection seams that exist only so tests
+can drive timing and encoding failures: the timer constructor used by
+`waitForAsyncDelay` and the JSON marshaller used by command acknowledgement
+delivery and by update-command payload decoding. Both are unexported fields on
+the `Agent` struct — `newTimerFn func(time.Duration) *time.Timer` and
+`jsonMarshalFn func(any) ([]byte, error)` — reached through the `newTimer` and
+`jsonMarshal` methods, and both fall back to the standard library
+(`time.NewTimer`, `json.Marshal`) when the field is nil. No package-level
+`newTimerFn` or `jsonMarshalFn` variable exists in `internal/dockeragent`.
+
+Ownership is per-Agent because the seams are read from goroutines the agent
+itself started. `runAsync` work such as command-acknowledgement retry outlives
+the test that scheduled it, so a package global swapped by a later test was a
+genuine data race between that swap and the leaked goroutine's read, not merely
+a test-isolation smell. Making the seam a field the goroutine's own `Agent`
+already carries removes the shared location: an injected hook can only ever be
+observed by the Agent it was constructed with, and two Agents running
+concurrently with different hooks stay independent. The injection point is
+construction only — nothing reassigns either field after an Agent is in use, so
+the fields need no mutex and take part in no locking order.
+
+This is an internal test-seam shape with no public contract delta. Agent
+enrollment, transport trust, command admission, acknowledgement semantics, and
+the Docker recreate/rollback result contract are all unchanged by it; a
+production Agent leaves both fields nil and runs the standard-library
+implementations. The seams remain unexported and must not be promoted into
+`NewAgent` options, agent profiles, or any server-facing configuration surface.
+`internal/dockeragent/agent_internal_test.go` proves the field shape, the nil
+defaults, the absence of package-level hook globals, and per-Agent isolation
+under the race detector;
+`internal/dockeragent/container_update_test.go` proves the update-command
+decode path routes through the receiver's own marshaller.
+
 ### vSphere system members carry no agent semantics
 
 ESXi host rows that now compose under their owning vCenter connection in the

internal/dockeragent/agent_internal_test.go

@@ -1,11 +1,19 @@
 package dockeragent
 
 import (
+	"context"
 	"errors"
+	"fmt"
+	"go/ast"
+	"go/parser"
+	"go/token"
 	"io"
 	"math"
 	"math/big"
+	"path/filepath"
 	"reflect"
+	"strings"
+	"sync"
 	"testing"
 	"time"
 
@@ -1331,3 +1339,209 @@ func TestDetectHostRemovedError(t *testing.T) {
 		})
 	}
 }
+
+// TestAgentHookSeamsArePerAgentFields pins the shape of the two indirection
+// seams the Docker agent exposes for tests: the timer constructor read by
+// waitForAsyncDelay and the JSON marshaller read by acknowledgement delivery
+// and update-payload decoding. Both used to be package-level vars in deps.go,
+// which raced with async goroutines leaked from earlier tests. They are now
+// fields on Agent, injected at construction and never reassigned afterwards.
+func TestAgentHookSeamsArePerAgentFields(t *testing.T) {
+	agentType := reflect.TypeOf(Agent{})
+
+	for _, tc := range []struct {
+		field string
+		want  reflect.Type
+	}{
+		{field: "newTimerFn", want: reflect.TypeOf((func(time.Duration) *time.Timer)(nil))},
+		{field: "jsonMarshalFn", want: reflect.TypeOf((func(any) ([]byte, error))(nil))},
+	} {
+		field, ok := agentType.FieldByName(tc.field)
+		if !ok {
+			t.Fatalf("Agent is missing the %s test seam field; the hook must stay per-Agent, not a package global", tc.field)
+		}
+		if field.Type != tc.want {
+			t.Fatalf("Agent.%s has type %s, want %s", tc.field, field.Type, tc.want)
+		}
+		if field.PkgPath == "" {
+			t.Fatalf("Agent.%s must stay unexported; it is an internal test seam, not a configuration surface", tc.field)
+		}
+	}
+}
+
+// TestAgentHookSeamsHaveNoPackageGlobals fails if either seam is reintroduced
+// as a package-level var in the non-test sources of internal/dockeragent. A
+// package global is reachable from every leaked goroutine in the package, which
+// is exactly the race the per-Agent fields removed.
+func TestAgentHookSeamsHaveNoPackageGlobals(t *testing.T) {
+	forbidden := map[string]struct{}{
+		"newTimerFn":    {},
+		"jsonMarshalFn": {},
+	}
+
+	sources, err := filepath.Glob("*.go")
+	if err != nil {
+		t.Fatalf("glob package sources: %v", err)
+	}
+	if len(sources) == 0 {
+		t.Fatal("no package sources found; the global scan would vacuously pass")
+	}
+
+	scanned := 0
+	for _, source := range sources {
+		if strings.HasSuffix(source, "_test.go") {
+			continue
+		}
+		scanned++
+
+		fset := token.NewFileSet()
+		file, err := parser.ParseFile(fset, source, nil, 0)
+		if err != nil {
+			t.Fatalf("parse %s: %v", source, err)
+		}
+
+		for _, decl := range file.Decls {
+			genDecl, ok := decl.(*ast.GenDecl)
+			if !ok || genDecl.Tok != token.VAR {
+				continue
+			}
+			for _, spec := range genDecl.Specs {
+				valueSpec, ok := spec.(*ast.ValueSpec)
+				if !ok {
+					continue
+				}
+				for _, name := range valueSpec.Names {
+					if _, bad := forbidden[name.Name]; bad {
+						t.Errorf(
+							"%s declares package-level var %s; the timer and JSON-marshal seams must stay Agent fields so async goroutines never read shared state",
+							source, name.Name,
+						)
+					}
+				}
+			}
+		}
+	}
+
+	if scanned == 0 {
+		t.Fatal("scanned no non-test sources; the global scan would vacuously pass")
+	}
+}
+
+// TestAgentHookSeamsDefaultToStdlib proves a zero-value Agent — the production
+// shape, where nothing injects a hook — runs the standard library behind both
+// seams.
+func TestAgentHookSeamsDefaultToStdlib(t *testing.T) {
+	agent := &Agent{}
+
+	body, err := agent.jsonMarshal(map[string]string{"k": "v"})
+	if err != nil {
+		t.Fatalf("nil jsonMarshalFn should fall back to json.Marshal: %v", err)
+	}
+	if string(body) != `{"k":"v"}` {
+		t.Fatalf("nil jsonMarshalFn produced %q, want json.Marshal output", body)
+	}
+
+	timer := agent.newTimer(time.Millisecond)
+	if timer == nil {
+		t.Fatal("nil newTimerFn should fall back to time.NewTimer")
+	}
+	defer stopTimer(timer)
+	select {
+	case <-timer.C:
+	case <-time.After(5 * time.Second):
+		t.Fatal("fallback timer never fired")
+	}
+}
+
+// TestAgentHookSeamsAreIsolatedPerAgent is the regression proof for the race
+// the per-Agent seams fixed. Two Agents constructed with different hooks are
+// exercised concurrently; with a package global, one Agent's injection would be
+// visible to the other and the swap would race the reads. Run under -race.
+func TestAgentHookSeamsAreIsolatedPerAgent(t *testing.T) {
+	marshalErr := errors.New("injected marshal failure")
+
+	failing := &Agent{
+		hostID: "failing",
+		jsonMarshalFn: func(any) ([]byte, error) {
+			return nil, marshalErr
+		},
+		newTimerFn: func(time.Duration) *time.Timer {
+			return time.NewTimer(0)
+		},
+	}
+	plain := &Agent{hostID: "plain"}
+
+	var wg sync.WaitGroup
+	errs := make(chan error, 64)
+
+	for range 16 {
+		wg.Add(2)
+
+		go func() {
+			defer wg.Done()
+			if _, err := failing.jsonMarshal(struct{}{}); !errors.Is(err, marshalErr) {
+				errs <- fmt.Errorf("injected agent should use its own marshaller, got %v", err)
+			}
+		}()
+
+		go func() {
+			defer wg.Done()
+			body, err := plain.jsonMarshal(map[string]int{"n": 1})
+			if err != nil {
+				errs <- fmt.Errorf("sibling agent must not observe the injected marshaller: %v", err)
+				return
+			}
+			if string(body) != `{"n":1}` {
+				errs <- fmt.Errorf("sibling agent produced %q, want json.Marshal output", body)
+			}
+		}()
+	}
+
+	wg.Wait()
+	close(errs)
+	for err := range errs {
+		t.Error(err)
+	}
+
+	// The injected timer fires immediately; the sibling's nil field must still
+	// resolve to a real time.NewTimer rather than the injected constructor.
+	injected := failing.newTimer(time.Hour)
+	defer stopTimer(injected)
+	select {
+	case <-injected.C:
+	case <-time.After(5 * time.Second):
+		t.Fatal("injected newTimerFn was not used by its own Agent")
+	}
+
+	sibling := plain.newTimer(time.Hour)
+	defer stopTimer(sibling)
+	select {
+	case <-sibling.C:
+		t.Fatal("sibling agent observed the injected immediate timer; the seam is not per-Agent")
+	case <-time.After(50 * time.Millisecond):
+	}
+}
+
+// TestSendCommandAckUsesReceiverMarshaller proves the acknowledgement transport
+// path in agent.go marshals through the receiver's own seam, so an injected
+// failure stays scoped to the Agent under test.
+func TestSendCommandAckUsesReceiverMarshaller(t *testing.T) {
+	marshalErr := errors.New("injected ack marshal failure")
+	agent := &Agent{
+		hostID: "host1",
+		jsonMarshalFn: func(any) ([]byte, error) {
+			return nil, marshalErr
+		},
+	}
+
+	err := agent.sendCommandAck(context.Background(), TargetConfig{URL: "http://example"}, "cmd", "status", "msg")
+	if err == nil {
+		t.Fatal("expected the injected marshaller to fail the acknowledgement")
+	}
+	if !errors.Is(err, marshalErr) {
+		t.Fatalf("acknowledgement error %v does not wrap the injected failure; the ack path is not using a.jsonMarshal", err)
+	}
+	if !strings.Contains(err.Error(), "marshal command acknowledgement") {
+		t.Fatalf("acknowledgement error %v lost its marshal context", err)
+	}
+}

internal/dockeragent/container_update_test.go

@@ -748,3 +748,89 @@ func TestUpdateContainer_SharedNamespaceCreateConfig(t *testing.T) {
 		}
 	})
 }
+
+// TestDecodeUpdateContainerPayloadUsesReceiverMarshaller pins the update-command
+// decode path to the receiver's own JSON seam. decodeUpdateContainerPayload was
+// a package function reading a package-level jsonMarshalFn; a test swapping that
+// global raced with async goroutines leaked from earlier tests. It is now an
+// Agent method reading a per-Agent field injected at construction, so a failure
+// injected into one Agent can never be observed by another.
+func TestDecodeUpdateContainerPayloadUsesReceiverMarshaller(t *testing.T) {
+	t.Run("injected marshaller fails only its own Agent", func(t *testing.T) {
+		marshalErr := errors.New("injected update marshal failure")
+		failing := &Agent{
+			jsonMarshalFn: func(any) ([]byte, error) {
+				return nil, marshalErr
+			},
+		}
+
+		if _, err := failing.decodeUpdateContainerPayload(map[string]any{"containerId": "abc"}); err == nil {
+			t.Fatal("expected the injected marshaller to fail the decode")
+		} else if !errors.Is(err, marshalErr) {
+			t.Fatalf("decode error %v does not wrap the injected failure; the decode is not using a.jsonMarshal", err)
+		} else if !strings.Contains(err.Error(), "marshal update command payload") {
+			t.Fatalf("decode error %v lost its marshal context", err)
+		}
+
+		plain := &Agent{}
+		decoded, err := plain.decodeUpdateContainerPayload(map[string]any{"containerId": "abc"})
+		if err != nil {
+			t.Fatalf("sibling Agent must not observe the injected marshaller: %v", err)
+		}
+		if decoded.ContainerID != "abc" {
+			t.Fatalf("sibling Agent decoded %q, want %q", decoded.ContainerID, "abc")
+		}
+	})
+
+	t.Run("concurrent Agents keep their own marshaller", func(t *testing.T) {
+		marshalErr := errors.New("injected update marshal failure")
+		failing := &Agent{
+			jsonMarshalFn: func(any) ([]byte, error) {
+				return nil, marshalErr
+			},
+		}
+		plain := &Agent{}
+
+		var wg sync.WaitGroup
+		problems := make(chan string, 64)
+
+		for range 16 {
+			wg.Add(2)
+
+			go func() {
+				defer wg.Done()
+				if _, err := failing.decodeUpdateContainerPayload(map[string]any{"containerId": "abc"}); !errors.Is(err, marshalErr) {
+					problems <- "injected Agent lost its marshaller"
+				}
+			}()
+
+			go func() {
+				defer wg.Done()
+				decoded, err := plain.decodeUpdateContainerPayload(map[string]any{"containerId": "abc"})
+				if err != nil || decoded.ContainerID != "abc" {
+					problems <- "sibling Agent observed the injected marshaller"
+				}
+			}()
+		}
+
+		wg.Wait()
+		close(problems)
+		for problem := range problems {
+			t.Error(problem)
+		}
+	})
+
+	t.Run("decodeUpdateContainerPayload is an Agent method", func(t *testing.T) {
+		// A compile-time pin: if the decode is ever hoisted back to a package
+		// function reading a package global, this assignment stops building.
+		var decode func(*Agent, map[string]any) (updateContainerCommandPayload, error) = (*Agent).decodeUpdateContainerPayload
+
+		decoded, err := decode(&Agent{}, map[string]any{"containerId": " abc "})
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+		if decoded.ContainerID != "abc" {
+			t.Fatalf("decoded %q, want %q", decoded.ContainerID, "abc")
+		}
+	})
+}