hostagent: avoid host ID collisions and prefer LAN IP

internal/hostagent/agent.go

@@ -70,6 +70,8 @@ type Agent struct {
 
 const defaultInterval = 30 * time.Second
 
+var readFile = os.ReadFile
+
 // New constructs a fully initialised host Agent.
 func New(cfg Config) (*Agent, error) {
 	if cfg.Interval <= 0 {
@@ -100,6 +102,7 @@ func New(cfg Config) (*Agent, error) {
 		pulseURL = "http://localhost:7655"
 	}
 	pulseURL = strings.TrimRight(pulseURL, "/")
+	cfg.PulseURL = pulseURL
 
 	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
 	defer cancel()
@@ -650,7 +653,7 @@ func (a *Agent) runProxmoxSetup(ctx context.Context) {
 // gopsutil to return identical HostIDs for all LXC containers on the same host.
 func isLXCContainer() bool {
 	// Check systemd-detect-virt if available
-	if data, err := os.ReadFile("/run/systemd/container"); err == nil {
+	if data, err := readFile("/run/systemd/container"); err == nil {
 		container := strings.TrimSpace(string(data))
 		if strings.Contains(container, "lxc") {
 			return true
@@ -658,14 +661,14 @@ func isLXCContainer() bool {
 	}
 
 	// Check /proc/1/environ for container=lxc
-	if data, err := os.ReadFile("/proc/1/environ"); err == nil {
+	if data, err := readFile("/proc/1/environ"); err == nil {
 		if strings.Contains(string(data), "container=lxc") {
 			return true
 		}
 	}
 
 	// Check /proc/1/cgroup for lxc markers
-	if data, err := os.ReadFile("/proc/1/cgroup"); err == nil {
+	if data, err := readFile("/proc/1/cgroup"); err == nil {
 		text := string(data)
 		if strings.Contains(text, "/lxc/") || strings.Contains(text, "lxc.payload") {
 			return true
@@ -676,15 +679,21 @@ func isLXCContainer() bool {
 }
 
 // getReliableMachineID returns a machine ID that's unique per container/host.
-// In LXC containers, gopsutil's HostID may use /sys/class/dmi/id/product_uuid
-// which is shared with the host, causing ID collisions. This function detects
-// LXC and prefers /etc/machine-id which is unique per container.
+// On Linux, /etc/machine-id is always preferred over gopsutil's HostID because:
+// - LXC containers share the host's /sys/class/dmi/id/product_uuid
+// - Cloned VMs/hosts may share the same DMI product UUID
+// - Proxmox cluster nodes with identical hardware may have the same UUID
+// The /etc/machine-id file is guaranteed unique per installation.
 func getReliableMachineID(gopsutilHostID string, logger zerolog.Logger) string {
 	gopsutilID := strings.TrimSpace(gopsutilHostID)
 
-	// For LXC containers, prefer /etc/machine-id to avoid ID collisions
-	if isLXCContainer() {
-		if data, err := os.ReadFile("/etc/machine-id"); err == nil {
+	// On Linux, always prefer /etc/machine-id as it's guaranteed unique per installation.
+	// This avoids ID collisions from:
+	// - LXC containers sharing host's DMI product UUID
+	// - Cloned VMs with identical hardware UUIDs
+	// - Proxmox cluster nodes with same hardware configuration
+	if runtime.GOOS == "linux" {
+		if data, err := readFile("/etc/machine-id"); err == nil {
 			machineID := strings.TrimSpace(string(data))
 			if len(machineID) >= 32 {
 				// Format as UUID if it's a 32-char hex string (like machine-id typically is)
@@ -693,9 +702,15 @@ func getReliableMachineID(gopsutilHostID string, logger zerolog.Logger) string {
 						machineID[0:8], machineID[8:12], machineID[12:16],
 						machineID[16:20], machineID[20:32])
 				}
-				logger.Debug().
-					Str("machineID", machineID).
-					Msg("LXC container detected, using /etc/machine-id for unique identification")
+				if isLXCContainer() {
+					logger.Debug().
+						Str("machineID", machineID).
+						Msg("LXC container detected, using /etc/machine-id for unique identification")
+				} else {
+					logger.Debug().
+						Str("machineID", machineID).
+						Msg("Linux host detected, using /etc/machine-id for unique identification")
+				}
 				return machineID
 			}
 		}

internal/hostagent/agent_test.go

@@ -2,6 +2,7 @@ package hostagent
 
 import (
 	"os"
+	"runtime"
 	"testing"
 
 	"github.com/rs/zerolog"
@@ -78,37 +79,57 @@ func TestNormalisePlatform(t *testing.T) {
 func TestGetReliableMachineID(t *testing.T) {
 	logger := zerolog.Nop()
 
-	// Check if we're running in an LXC container
-	inLXC := isLXCContainer()
-
-	t.Run("returns non-empty ID", func(t *testing.T) {
-		result := getReliableMachineID("test-gopsutil-id", logger)
-		if result == "" {
-			t.Error("getReliableMachineID returned empty string")
-		}
-	})
+	originalReadFile := readFile
+	t.Cleanup(func() { readFile = originalReadFile })
 
 	t.Run("trims whitespace", func(t *testing.T) {
+		readFile = func(string) ([]byte, error) { return nil, os.ErrNotExist }
 		result := getReliableMachineID("  test-id  ", logger)
-		if result == "  test-id  " {
-			t.Error("getReliableMachineID did not trim whitespace")
+		if result != "test-id" {
+			t.Errorf("getReliableMachineID trimmed result = %q, want %q", result, "test-id")
 		}
 	})
 
-	if inLXC {
-		t.Run("LXC uses machine-id", func(t *testing.T) {
-			// In LXC, we should get a machine-id regardless of gopsutil input
-			result := getReliableMachineID("gopsutil-product-uuid", logger)
-			if result == "gopsutil-product-uuid" {
-				t.Error("In LXC, getReliableMachineID should use /etc/machine-id, not gopsutil ID")
+	// On Linux, we always use /etc/machine-id to avoid ID collisions
+	// from LXC containers, cloned VMs, or identical hardware UUIDs
+	if runtime.GOOS == "linux" {
+		t.Run("Linux prefers /etc/machine-id and formats 32-char IDs", func(t *testing.T) {
+			readFile = func(name string) ([]byte, error) {
+				if name == "/etc/machine-id" {
+					return []byte("0123456789abcdef0123456789abcdef\n"), nil
+				}
+				return nil, os.ErrNotExist
 			}
-			// Verify it looks like a formatted UUID
-			if len(result) < 32 {
-				t.Errorf("Expected UUID-like result, got %q", result)
+
+			result := getReliableMachineID("gopsutil-product-uuid", logger)
+			const want = "01234567-89ab-cdef-0123-456789abcdef"
+			if result != want {
+				t.Errorf("getReliableMachineID() = %q, want %q", result, want)
+			}
+		})
+
+		t.Run("Linux falls back to gopsutil ID when /etc/machine-id missing", func(t *testing.T) {
+			readFile = func(string) ([]byte, error) { return nil, os.ErrNotExist }
+			result := getReliableMachineID("gopsutil-product-uuid", logger)
+			if result != "gopsutil-product-uuid" {
+				t.Errorf("getReliableMachineID() = %q, want %q", result, "gopsutil-product-uuid")
+			}
+		})
+
+		t.Run("Linux falls back when machine-id is too short", func(t *testing.T) {
+			readFile = func(name string) ([]byte, error) {
+				if name == "/etc/machine-id" {
+					return []byte("short\n"), nil
+				}
+				return nil, os.ErrNotExist
+			}
+			result := getReliableMachineID("gopsutil-product-uuid", logger)
+			if result != "gopsutil-product-uuid" {
+				t.Errorf("getReliableMachineID() = %q, want %q", result, "gopsutil-product-uuid")
 			}
 		})
 	} else {
-		t.Run("non-LXC uses gopsutil ID", func(t *testing.T) {
+		t.Run("non-Linux uses gopsutil ID", func(t *testing.T) {
 			result := getReliableMachineID("12345678-1234-1234-1234-123456789abc", logger)
 			if result != "12345678-1234-1234-1234-123456789abc" {
 				t.Errorf("Expected gopsutil ID, got %q", result)
@@ -118,20 +139,60 @@ func TestGetReliableMachineID(t *testing.T) {
 }
 
 func TestIsLXCContainer(t *testing.T) {
-	// This test documents the detection behavior.
-	// On non-LXC systems, isLXCContainer should return false.
-	// We can't easily test the true case without mocking filesystem.
-	result := isLXCContainer()
+	originalReadFile := readFile
+	t.Cleanup(func() { readFile = originalReadFile })
 
-	// Check if we're actually in an LXC container
-	isActuallyLXC := false
-	if data, err := os.ReadFile("/run/systemd/container"); err == nil {
-		if string(data) == "lxc" || string(data) == "lxc\n" {
-			isActuallyLXC = true
+	t.Run("/run/systemd/container detects lxc", func(t *testing.T) {
+		readFile = func(name string) ([]byte, error) {
+			if name == "/run/systemd/container" {
+				return []byte("lxc\n"), nil
+			}
+			return nil, os.ErrNotExist
 		}
-	}
+		if !isLXCContainer() {
+			t.Fatalf("expected lxc container to be detected")
+		}
+	})
 
-	if result != isActuallyLXC {
-		t.Logf("isLXCContainer() = %v (expected %v based on environment)", result, isActuallyLXC)
-	}
+	t.Run("/proc/1/environ detects container=lxc", func(t *testing.T) {
+		readFile = func(name string) ([]byte, error) {
+			if name == "/proc/1/environ" {
+				return []byte("foo=bar\x00container=lxc\x00baz=qux"), nil
+			}
+			return nil, os.ErrNotExist
+		}
+		if !isLXCContainer() {
+			t.Fatalf("expected lxc container to be detected")
+		}
+	})
+
+	t.Run("/proc/1/cgroup detects /lxc/", func(t *testing.T) {
+		readFile = func(name string) ([]byte, error) {
+			if name == "/proc/1/cgroup" {
+				return []byte("0::/lxc/abcd\n"), nil
+			}
+			return nil, os.ErrNotExist
+		}
+		if !isLXCContainer() {
+			t.Fatalf("expected lxc container to be detected")
+		}
+	})
+
+	t.Run("non-lxc container returns false", func(t *testing.T) {
+		readFile = func(name string) ([]byte, error) {
+			if name == "/run/systemd/container" {
+				return []byte("docker\n"), nil
+			}
+			if name == "/proc/1/environ" {
+				return []byte("container=podman\x00"), nil
+			}
+			if name == "/proc/1/cgroup" {
+				return []byte("0::/system.slice\n"), nil
+			}
+			return nil, os.ErrNotExist
+		}
+		if isLXCContainer() {
+			t.Fatalf("expected non-lxc container to not be detected as lxc")
+		}
+	})
 }

internal/hostagent/command_client_test.go

@@ -0,0 +1,116 @@
+package hostagent
+
+import (
+	"io"
+	"testing"
+
+	"github.com/rs/zerolog"
+)
+
+func TestNew_DefaultPulseURLUsedForCommandClient(t *testing.T) {
+	logger := zerolog.New(io.Discard)
+
+	agent, err := New(Config{
+		APIToken: "test-token",
+		LogLevel: zerolog.InfoLevel,
+		Logger:   &logger,
+	})
+	if err != nil {
+		t.Fatalf("New: %v", err)
+	}
+
+	const want = "http://localhost:7655"
+	if agent.trimmedPulseURL != want {
+		t.Fatalf("trimmedPulseURL = %q, want %q", agent.trimmedPulseURL, want)
+	}
+	if agent.cfg.PulseURL != want {
+		t.Fatalf("cfg.PulseURL = %q, want %q", agent.cfg.PulseURL, want)
+	}
+	if agent.commandClient == nil {
+		t.Fatalf("commandClient should be initialized")
+	}
+	if agent.commandClient.pulseURL != want {
+		t.Fatalf("commandClient.pulseURL = %q, want %q", agent.commandClient.pulseURL, want)
+	}
+}
+
+func TestNew_TrimsPulseURLForCommandClient(t *testing.T) {
+	logger := zerolog.New(io.Discard)
+
+	agent, err := New(Config{
+		PulseURL: "https://example.invalid/",
+		APIToken: "test-token",
+		LogLevel: zerolog.InfoLevel,
+		Logger:   &logger,
+	})
+	if err != nil {
+		t.Fatalf("New: %v", err)
+	}
+
+	const want = "https://example.invalid"
+	if agent.trimmedPulseURL != want {
+		t.Fatalf("trimmedPulseURL = %q, want %q", agent.trimmedPulseURL, want)
+	}
+	if agent.cfg.PulseURL != want {
+		t.Fatalf("cfg.PulseURL = %q, want %q", agent.cfg.PulseURL, want)
+	}
+	if agent.commandClient == nil {
+		t.Fatalf("commandClient should be initialized")
+	}
+	if agent.commandClient.pulseURL != want {
+		t.Fatalf("commandClient.pulseURL = %q, want %q", agent.commandClient.pulseURL, want)
+	}
+}
+
+func TestCommandClientBuildWebSocketURL(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name     string
+		pulseURL string
+		want     string
+		wantErr  bool
+	}{
+		{
+			name:     "https becomes wss",
+			pulseURL: "https://example.invalid",
+			want:     "wss://example.invalid/api/agent/ws",
+		},
+		{
+			name:     "http becomes ws",
+			pulseURL: "http://example.invalid",
+			want:     "ws://example.invalid/api/agent/ws",
+		},
+		{
+			name:     "ws preserved",
+			pulseURL: "ws://example.invalid",
+			want:     "ws://example.invalid/api/agent/ws",
+		},
+		{
+			name:     "wss preserved",
+			pulseURL: "wss://example.invalid",
+			want:     "wss://example.invalid/api/agent/ws",
+		},
+		{
+			name:     "invalid url returns error",
+			pulseURL: "http://[::1",
+			wantErr:  true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			client := &CommandClient{pulseURL: tt.pulseURL}
+			got, err := client.buildWebSocketURL()
+			if (err != nil) != tt.wantErr {
+				t.Fatalf("buildWebSocketURL() err = %v, wantErr %v", err, tt.wantErr)
+			}
+			if tt.wantErr {
+				return
+			}
+			if got != tt.want {
+				t.Fatalf("buildWebSocketURL() = %q, want %q", got, tt.want)
+			}
+		})
+	}
+}

internal/hostagent/proxmox_setup.go

@@ -36,12 +36,12 @@ type ProxmoxSetupResult struct {
 }
 
 const (
-	proxmoxUser     = "pulse-monitor"
-	proxmoxUserPVE  = "pulse-monitor@pam"
-	proxmoxUserPBS  = "pulse-monitor@pbs"
-	proxmoxComment  = "Pulse monitoring service"
-	stateFilePath   = "/var/lib/pulse-agent/proxmox-registered"
-	stateFileDir    = "/var/lib/pulse-agent"
+	proxmoxUser    = "pulse-monitor"
+	proxmoxUserPVE = "pulse-monitor@pam"
+	proxmoxUserPBS = "pulse-monitor@pbs"
+	proxmoxComment = "Pulse monitoring service"
+	stateFilePath  = "/var/lib/pulse-agent/proxmox-registered"
+	stateFileDir   = "/var/lib/pulse-agent"
 )
 
 // NewProxmoxSetup creates a new ProxmoxSetup instance.
@@ -246,23 +246,20 @@ func (p *ProxmoxSetup) parsePBSTokenValue(output string) string {
 }
 
 // getHostURL constructs the host URL for this Proxmox node.
-// Prefers IP address over hostname since hostnames are often not DNS-resolvable.
+// Uses intelligent IP selection to prefer LAN addresses over internal cluster networks.
 func (p *ProxmoxSetup) getHostURL(ptype string) string {
 	port := "8006"
 	if ptype == "pbs" {
 		port = "8007"
 	}
 
-	// Always prefer IP address since hostnames often can't be resolved by Pulse
-	// (e.g., "pi" hostname won't resolve via DNS)
+	// Get all IPs and select the best one for external access
 	if out, err := exec.Command("hostname", "-I").Output(); err == nil {
 		ips := strings.Fields(string(out))
 		if len(ips) > 0 {
-			// Use first non-loopback IP
-			for _, ip := range ips {
-				if ip != "127.0.0.1" && ip != "::1" {
-					return fmt.Sprintf("https://%s:%s", ip, port)
-				}
+			bestIP := selectBestIP(ips)
+			if bestIP != "" {
+				return fmt.Sprintf("https://%s:%s", bestIP, port)
 			}
 		}
 	}
@@ -275,6 +272,95 @@ func (p *ProxmoxSetup) getHostURL(ptype string) string {
 	return fmt.Sprintf("https://%s:%s", hostname, port)
 }
 
+// selectBestIP picks the most likely externally-reachable IP from a list.
+// Prefers common LAN subnets (192.168.x.x, 10.x.x.x) and avoids internal
+// cluster networks (like corosync's 172.20.x.x) and link-local addresses.
+func selectBestIP(ips []string) string {
+	type scoredIP struct {
+		ip    string
+		score int
+	}
+
+	var candidates []scoredIP
+
+	for _, ip := range ips {
+		// Skip loopback and IPv6 link-local
+		if ip == "127.0.0.1" || ip == "::1" || strings.HasPrefix(ip, "fe80:") {
+			continue
+		}
+
+		// Skip IPv6 for simplicity (most Proxmox setups use IPv4)
+		if strings.Contains(ip, ":") {
+			continue
+		}
+
+		score := scoreIPv4(ip)
+		if score > 0 {
+			candidates = append(candidates, scoredIP{ip: ip, score: score})
+		}
+	}
+
+	if len(candidates) == 0 {
+		// Fallback: return first non-loopback IP if no good candidates
+		for _, ip := range ips {
+			if ip != "127.0.0.1" && ip != "::1" && !strings.HasPrefix(ip, "fe80:") {
+				return ip
+			}
+		}
+		return ""
+	}
+
+	// Return highest scored IP
+	best := candidates[0]
+	for _, c := range candidates[1:] {
+		if c.score > best.score {
+			best = c
+		}
+	}
+	return best.ip
+}
+
+// scoreIPv4 assigns a preference score to an IPv4 address.
+// Higher score = more likely to be externally reachable.
+func scoreIPv4(ip string) int {
+	parts := strings.Split(ip, ".")
+	if len(parts) != 4 {
+		return 0
+	}
+
+	// Parse first two octets
+	first := 0
+	second := 0
+	fmt.Sscanf(parts[0], "%d", &first)
+	fmt.Sscanf(parts[1], "%d", &second)
+
+	// Scoring logic:
+	// - 192.168.x.x: Very common home/office LAN, high priority (score 100)
+	// - 10.0.x.x - 10.31.x.x: Common corporate LAN ranges (score 90)
+	// - 10.x.x.x (other): Less common, but still likely LAN (score 70)
+	// - 172.16-31.x.x: Private range, often used for internal clusters (score 50)
+	//   Corosync often uses 172.20.x.x or similar for ring0/ring1
+	// - 169.254.x.x: Link-local, skip (score 0)
+	// - Other private/public: Unknown, low priority (score 30)
+
+	switch {
+	case first == 192 && second == 168:
+		return 100 // Most common home/office LAN
+	case first == 10 && second <= 31:
+		return 90 // Common corporate LAN
+	case first == 10:
+		return 70 // Other 10.x.x.x
+	case first == 172 && second >= 16 && second <= 31:
+		// This is private 172.16-31.x.x range, often used for internal clusters
+		// Corosync commonly uses 172.20.x.x for cluster communication
+		return 50 // Lower priority - often internal cluster
+	case first == 169 && second == 254:
+		return 0 // Link-local, skip
+	default:
+		return 30 // Unknown/public - low priority
+	}
+}
+
 // registerWithPulse calls the auto-register endpoint to add the node.
 func (p *ProxmoxSetup) registerWithPulse(ctx context.Context, ptype, hostURL, tokenID, tokenValue string) error {
 	payload := map[string]interface{}{

internal/hostagent/proxmox_setup_test.go

@@ -0,0 +1,134 @@
+package hostagent
+
+import "testing"
+
+func TestSelectBestIP(t *testing.T) {
+	tests := []struct {
+		name     string
+		ips      []string
+		expected string
+	}{
+		{
+			name:     "prefers 192.168.x.x over corosync 172.20.x.x",
+			ips:      []string{"172.20.0.80", "192.168.1.100"},
+			expected: "192.168.1.100",
+		},
+		{
+			name:     "prefers 192.168.x.x even when listed second",
+			ips:      []string{"10.0.0.1", "192.168.0.1"},
+			expected: "192.168.0.1",
+		},
+		{
+			name:     "prefers 10.x.x.x over 172.16-31.x.x",
+			ips:      []string{"172.20.0.1", "10.1.10.5"},
+			expected: "10.1.10.5",
+		},
+		{
+			name:     "handles single IP",
+			ips:      []string{"192.168.1.1"},
+			expected: "192.168.1.1",
+		},
+		{
+			name:     "skips loopback",
+			ips:      []string{"127.0.0.1", "192.168.1.1"},
+			expected: "192.168.1.1",
+		},
+		{
+			name:     "skips IPv6 loopback",
+			ips:      []string{"::1", "10.0.0.1"},
+			expected: "10.0.0.1",
+		},
+		{
+			name:     "skips link-local IPv6",
+			ips:      []string{"fe80::1", "192.168.1.1"},
+			expected: "192.168.1.1",
+		},
+		{
+			name:     "skips link-local IPv4",
+			ips:      []string{"169.254.1.1", "10.0.0.1"},
+			expected: "10.0.0.1",
+		},
+		{
+			name:     "returns corosync IP if only option",
+			ips:      []string{"127.0.0.1", "172.20.0.80"},
+			expected: "172.20.0.80",
+		},
+		{
+			name:     "empty list returns empty",
+			ips:      []string{},
+			expected: "",
+		},
+		{
+			name:     "only loopback returns empty",
+			ips:      []string{"127.0.0.1", "::1"},
+			expected: "",
+		},
+		{
+			name:     "common 10.1.x.x LAN preferred over 172.x.x",
+			ips:      []string{"172.16.0.1", "10.1.10.50"},
+			expected: "10.1.10.50",
+		},
+		{
+			name:     "prefers 10.0.x.x to 10.100.x.x (common ranges first)",
+			ips:      []string{"10.100.0.1", "10.0.0.1"},
+			expected: "10.0.0.1",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := selectBestIP(tt.ips)
+			if result != tt.expected {
+				t.Errorf("selectBestIP(%v) = %q, want %q", tt.ips, result, tt.expected)
+			}
+		})
+	}
+}
+
+func TestScoreIPv4(t *testing.T) {
+	tests := []struct {
+		ip            string
+		expectedScore int
+	}{
+		// 192.168.x.x - highest priority (100)
+		{"192.168.1.1", 100},
+		{"192.168.0.100", 100},
+		{"192.168.255.255", 100},
+
+		// 10.0-31.x.x - common corporate (90)
+		{"10.0.0.1", 90},
+		{"10.1.10.5", 90},
+		{"10.31.255.255", 90},
+
+		// 10.32+.x.x - less common (70)
+		{"10.32.0.1", 70},
+		{"10.100.0.1", 70},
+		{"10.255.255.255", 70},
+
+		// 172.16-31.x.x - private but often cluster (50)
+		{"172.16.0.1", 50},
+		{"172.20.0.80", 50}, // Corosync typical
+		{"172.31.255.255", 50},
+
+		// 169.254.x.x - link-local (0)
+		{"169.254.1.1", 0},
+
+		// Other/public (30)
+		{"8.8.8.8", 30},
+		{"1.1.1.1", 30},
+		{"203.0.113.1", 30},
+
+		// Invalid
+		{"not-an-ip", 0},
+		{"", 0},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.ip, func(t *testing.T) {
+			result := scoreIPv4(tt.ip)
+			if result != tt.expectedScore {
+				t.Errorf("scoreIPv4(%q) = %d, want %d", tt.ip, result, tt.expectedScore)
+			}
+		})
+	}
+}

internal/hostagent/send_report_test.go

@@ -0,0 +1,119 @@
+package hostagent
+
+import (
+	"context"
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"sync"
+	"testing"
+
+	agentshost "github.com/rcourtman/pulse-go-rewrite/pkg/agents/host"
+)
+
+func TestAgentSendReport_SetsHeadersAndPostsJSON(t *testing.T) {
+	t.Parallel()
+
+	type received struct {
+		method        string
+		path          string
+		authorization string
+		apiToken      string
+		contentType   string
+		userAgent     string
+		body          agentshost.Report
+	}
+
+	var (
+		mu  sync.Mutex
+		got received
+	)
+
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		var report agentshost.Report
+		if err := json.NewDecoder(r.Body).Decode(&report); err != nil {
+			http.Error(w, "bad json", http.StatusBadRequest)
+			return
+		}
+
+		mu.Lock()
+		got = received{
+			method:        r.Method,
+			path:          r.URL.Path,
+			authorization: r.Header.Get("Authorization"),
+			apiToken:      r.Header.Get("X-API-Token"),
+			contentType:   r.Header.Get("Content-Type"),
+			userAgent:     r.Header.Get("User-Agent"),
+			body:          report,
+		}
+		mu.Unlock()
+
+		w.WriteHeader(http.StatusOK)
+	}))
+	defer server.Close()
+
+	agent := &Agent{
+		cfg:             Config{APIToken: "test-token"},
+		httpClient:      server.Client(),
+		trimmedPulseURL: server.URL,
+	}
+
+	wantReport := agentshost.Report{
+		Agent: agentshost.AgentInfo{ID: "agent-1"},
+		Host:  agentshost.HostInfo{Hostname: "test-host"},
+	}
+
+	if err := agent.sendReport(context.Background(), wantReport); err != nil {
+		t.Fatalf("sendReport: %v", err)
+	}
+
+	mu.Lock()
+	defer mu.Unlock()
+
+	if got.method != http.MethodPost {
+		t.Fatalf("method = %q, want %q", got.method, http.MethodPost)
+	}
+	if got.path != "/api/agents/host/report" {
+		t.Fatalf("path = %q, want %q", got.path, "/api/agents/host/report")
+	}
+	if got.authorization != "Bearer test-token" {
+		t.Fatalf("Authorization = %q, want %q", got.authorization, "Bearer test-token")
+	}
+	if got.apiToken != "test-token" {
+		t.Fatalf("X-API-Token = %q, want %q", got.apiToken, "test-token")
+	}
+	if got.contentType != "application/json" {
+		t.Fatalf("Content-Type = %q, want %q", got.contentType, "application/json")
+	}
+	if got.userAgent == "" {
+		t.Fatalf("User-Agent should be set")
+	}
+	if got.body.Agent.ID != wantReport.Agent.ID {
+		t.Fatalf("decoded report Agent.ID = %q, want %q", got.body.Agent.ID, wantReport.Agent.ID)
+	}
+	if got.body.Host.Hostname != wantReport.Host.Hostname {
+		t.Fatalf("decoded report Host.Hostname = %q, want %q", got.body.Host.Hostname, wantReport.Host.Hostname)
+	}
+}
+
+func TestAgentSendReport_Non2xxReturnsError(t *testing.T) {
+	t.Parallel()
+
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusInternalServerError)
+	}))
+	defer server.Close()
+
+	agent := &Agent{
+		cfg:             Config{APIToken: "test-token"},
+		httpClient:      server.Client(),
+		trimmedPulseURL: server.URL,
+	}
+
+	err := agent.sendReport(context.Background(), agentshost.Report{
+		Agent: agentshost.AgentInfo{ID: "agent-1"},
+	})
+	if err == nil {
+		t.Fatalf("expected error for non-2xx response")
+	}
+}

pkg/discovery/discovery_result_test.go

@@ -0,0 +1,66 @@
+package discovery
+
+import (
+	"testing"
+)
+
+func TestDiscoveryResultAddError_PopulatesStructuredAndLegacy(t *testing.T) {
+	t.Parallel()
+
+	var r DiscoveryResult
+	r.AddError("docker_bridge_network", "timeout", "request timed out", "192.168.1.10", 8006)
+
+	if len(r.StructuredErrors) != 1 {
+		t.Fatalf("StructuredErrors len = %d, want %d", len(r.StructuredErrors), 1)
+	}
+	if len(r.Errors) != 1 {
+		t.Fatalf("Errors len = %d, want %d", len(r.Errors), 1)
+	}
+
+	se := r.StructuredErrors[0]
+	if se.Phase != "docker_bridge_network" {
+		t.Fatalf("StructuredErrors[0].Phase = %q, want %q", se.Phase, "docker_bridge_network")
+	}
+	if se.ErrorType != "timeout" {
+		t.Fatalf("StructuredErrors[0].ErrorType = %q, want %q", se.ErrorType, "timeout")
+	}
+	if se.Message != "request timed out" {
+		t.Fatalf("StructuredErrors[0].Message = %q, want %q", se.Message, "request timed out")
+	}
+	if se.IP != "192.168.1.10" || se.Port != 8006 {
+		t.Fatalf("StructuredErrors[0] address = %s:%d, want %s:%d", se.IP, se.Port, "192.168.1.10", 8006)
+	}
+	if se.Timestamp.IsZero() {
+		t.Fatalf("StructuredErrors[0].Timestamp should be set")
+	}
+
+	if r.Errors[0] != "Docker bridge network [192.168.1.10:8006]: request timed out" {
+		t.Fatalf("Errors[0] = %q", r.Errors[0])
+	}
+}
+
+func TestDiscoveryResultAddError_LegacyFormattingVariants(t *testing.T) {
+	t.Parallel()
+
+	t.Run("ip-only", func(t *testing.T) {
+		var r DiscoveryResult
+		r.AddError("extra_targets", "phase_error", "failed", "10.0.0.1", 0)
+		if len(r.Errors) != 1 {
+			t.Fatalf("Errors len = %d, want %d", len(r.Errors), 1)
+		}
+		if r.Errors[0] != "Additional targets [10.0.0.1]: failed" {
+			t.Fatalf("Errors[0] = %q", r.Errors[0])
+		}
+	})
+
+	t.Run("no-address", func(t *testing.T) {
+		var r DiscoveryResult
+		r.AddError("unknown_phase", "phase_error", "failed", "", 0)
+		if len(r.Errors) != 1 {
+			t.Fatalf("Errors len = %d, want %d", len(r.Errors), 1)
+		}
+		if r.Errors[0] != "unknown_phase: failed" {
+			t.Fatalf("Errors[0] = %q", r.Errors[0])
+		}
+	})
+}