Unify Proxmox discovery results

- Redirect PVE node lookups to linked Host Agent ID when available.
- Implement deduplication in discovery lists to prefer Host Agent data over redundant Node entries.
- Add fallback mechanism to original Node ID for discovery retrieval ensuring compatibility with legacy data.
- Update data adapters and added comprehensive unit tests for redirection and deduplication logic.

internal/ai/discovery_adapter.go

@@ -191,11 +191,22 @@ func (a *discoveryStateAdapter) GetState() servicediscovery.StateSnapshot {
 		}
 	}
 
+	// Convert Nodes
+	nodes := make([]servicediscovery.Node, len(state.Nodes))
+	for i, n := range state.Nodes {
+		nodes[i] = servicediscovery.Node{
+			ID:                n.ID,
+			Name:              n.Name,
+			LinkedHostAgentID: n.LinkedHostAgentID,
+		}
+	}
+
 	return servicediscovery.StateSnapshot{
 		VMs:         vms,
 		Containers:  containers,
 		DockerHosts: dockerHosts,
 		Hosts:       hosts,
+		Nodes:       nodes,
 	}
 }
 

internal/ai/discovery_adapter_test.go

@@ -43,6 +43,13 @@ func TestDiscoveryStateAdapter(t *testing.T) {
 				},
 			},
 		},
+		Hosts: []models.Host{
+			{ID: "host-agent-1", Hostname: "server1", Platform: "linux"},
+		},
+		Nodes: []models.Node{
+			{ID: "node-1", Name: "pve1", LinkedHostAgentID: "host-agent-1"},
+			{ID: "node-2", Name: "pve2", LinkedHostAgentID: ""}, // No linked agent
+		},
 	}
 
 	provider := &MockStateProvider{state: mockState}
@@ -86,6 +93,42 @@ func TestDiscoveryStateAdapter(t *testing.T) {
 			}
 		}
 	}
+
+	// Test Hosts conversion
+	if len(result.Hosts) != 1 {
+		t.Errorf("Expected 1 Host, got %d", len(result.Hosts))
+	} else {
+		if result.Hosts[0].ID != "host-agent-1" {
+			t.Errorf("Expected Host ID 'host-agent-1', got %s", result.Hosts[0].ID)
+		}
+		if result.Hosts[0].Hostname != "server1" {
+			t.Errorf("Expected Host Hostname 'server1', got %s", result.Hosts[0].Hostname)
+		}
+	}
+
+	// Test Nodes conversion (critical for discovery redirection)
+	if len(result.Nodes) != 2 {
+		t.Errorf("Expected 2 Nodes, got %d", len(result.Nodes))
+	} else {
+		// Verify first node with linked agent
+		if result.Nodes[0].ID != "node-1" {
+			t.Errorf("Expected Node[0].ID 'node-1', got %s", result.Nodes[0].ID)
+		}
+		if result.Nodes[0].Name != "pve1" {
+			t.Errorf("Expected Node[0].Name 'pve1', got %s", result.Nodes[0].Name)
+		}
+		if result.Nodes[0].LinkedHostAgentID != "host-agent-1" {
+			t.Errorf("Expected Node[0].LinkedHostAgentID 'host-agent-1', got %s", result.Nodes[0].LinkedHostAgentID)
+		}
+
+		// Verify second node without linked agent
+		if result.Nodes[1].Name != "pve2" {
+			t.Errorf("Expected Node[1].Name 'pve2', got %s", result.Nodes[1].Name)
+		}
+		if result.Nodes[1].LinkedHostAgentID != "" {
+			t.Errorf("Expected Node[1].LinkedHostAgentID to be empty, got %s", result.Nodes[1].LinkedHostAgentID)
+		}
+	}
 }
 
 func TestDiscoveryStateAdapter_NilProvider(t *testing.T) {

internal/servicediscovery/service.go

@@ -81,6 +81,14 @@ type StateSnapshot struct {
 	DockerHosts        []DockerHost
 	KubernetesClusters []KubernetesCluster
 	Hosts              []Host
+	Nodes              []Node
+}
+
+// Node represents a Proxmox VE node.
+type Node struct {
+	ID                string
+	Name              string
+	LinkedHostAgentID string
 }
 
 // Host represents a host system (via host-agent).
@@ -1053,6 +1061,26 @@ func (s *Service) analyzeDockerContainer(ctx context.Context, analyzer AIAnalyze
 // - Runs discovery only when fingerprint changed or discovery is too old
 // - Prevents duplicate concurrent discoveries for the same resource
 func (s *Service) DiscoverResource(ctx context.Context, req DiscoveryRequest) (*ResourceDiscovery, error) {
+	// Redirect PVE node requests to linked host agent if available
+	// This ensures we always scan and store data under the canonical Host Agent ID
+	if req.ResourceType == ResourceTypeHost && s.stateProvider != nil {
+		state := s.stateProvider.GetState()
+		for _, node := range state.Nodes {
+			// Check if the requested ID matches the Node Name or ID
+			if node.Name == req.HostID || node.Name == req.ResourceID || node.ID == req.ResourceID {
+				if node.LinkedHostAgentID != "" {
+					log.Info().
+						Str("from_host", req.HostID).
+						Str("to_agent", node.LinkedHostAgentID).
+						Msg("Redirecting discovery scan to linked host agent")
+					req.HostID = node.LinkedHostAgentID
+					req.ResourceID = node.LinkedHostAgentID
+				}
+				break
+			}
+		}
+	}
+
 	resourceID := MakeResourceID(req.ResourceType, req.HostID, req.ResourceID)
 
 	// Get current fingerprint (if available)
@@ -1726,9 +1754,48 @@ func (s *Service) GetDiscovery(id string) (*ResourceDiscovery, error) {
 	return d, nil
 }
 
-// GetDiscoveryByResource retrieves a discovery by resource type and ID.
 func (s *Service) GetDiscoveryByResource(resourceType ResourceType, hostID, resourceID string) (*ResourceDiscovery, error) {
+	originalHostID := hostID
+	originalResourceID := resourceID
+	redirected := false
+
+	// Redirect PVE node lookups to linked host agent if available
+	// This ensures UI components looking up a PVE node by name (e.g. NodeDrawer) get the data associated with the Host Agent
+	if resourceType == ResourceTypeHost && s.stateProvider != nil {
+		state := s.stateProvider.GetState()
+		for _, node := range state.Nodes {
+			if node.Name == hostID || node.Name == resourceID || node.ID == resourceID {
+				if node.LinkedHostAgentID != "" {
+					log.Debug().
+						Str("from_host", hostID).
+						Str("to_agent", node.LinkedHostAgentID).
+						Msg("Redirecting discovery lookup to linked host agent")
+					hostID = node.LinkedHostAgentID
+					resourceID = node.LinkedHostAgentID
+					redirected = true
+				}
+				break
+			}
+		}
+	}
+
 	d, err := s.store.GetByResource(resourceType, hostID, resourceID)
+	// If redirected and not found, try the original ID (fallback for unmigrated data)
+	if (err != nil || d == nil) && redirected {
+		log.Debug().
+			Str("redirected_host", hostID).
+			Str("original_host", originalHostID).
+			Msg("Redirected lookup failed, trying fallback to original ID")
+		dOriginal, errOriginal := s.store.GetByResource(resourceType, originalHostID, originalResourceID)
+		if errOriginal == nil && dOriginal != nil {
+			log.Debug().
+				Str("original_host", originalHostID).
+				Msg("Fallback lookup succeeded - returning legacy discovery")
+			s.upgradeCLIAccessIfNeeded(dOriginal)
+			return dOriginal, nil
+		}
+	}
+
 	if err != nil || d == nil {
 		return d, err
 	}
@@ -1742,6 +1809,7 @@ func (s *Service) ListDiscoveries() ([]*ResourceDiscovery, error) {
 	if err != nil {
 		return nil, err
 	}
+	discoveries = s.deduplicateDiscoveries(discoveries)
 	for _, d := range discoveries {
 		s.upgradeCLIAccessIfNeeded(d)
 	}
@@ -1754,6 +1822,7 @@ func (s *Service) ListDiscoveriesByType(resourceType ResourceType) ([]*ResourceD
 	if err != nil {
 		return nil, err
 	}
+	discoveries = s.deduplicateDiscoveries(discoveries)
 	for _, d := range discoveries {
 		s.upgradeCLIAccessIfNeeded(d)
 	}
@@ -1766,12 +1835,80 @@ func (s *Service) ListDiscoveriesByHost(hostID string) ([]*ResourceDiscovery, er
 	if err != nil {
 		return nil, err
 	}
+	discoveries = s.deduplicateDiscoveries(discoveries)
 	for _, d := range discoveries {
 		s.upgradeCLIAccessIfNeeded(d)
 	}
 	return discoveries, nil
 }
 
+// deduplicateDiscoveries filters out redundant discoveries where a PVE node
+// is represented by both its Node Name and its Linked Host Agent ID.
+// The Host Agent ID is preferred.
+func (s *Service) deduplicateDiscoveries(discoveries []*ResourceDiscovery) []*ResourceDiscovery {
+	if s.stateProvider == nil {
+		return discoveries
+	}
+
+	state := s.stateProvider.GetState()
+	if len(state.Nodes) == 0 {
+		return discoveries
+	}
+
+	// Map linked agent IDs to their PVE node source(s)
+	// AgentID -> NodeName
+	linkedAgents := make(map[string]string)
+	for _, node := range state.Nodes {
+		if node.LinkedHostAgentID != "" {
+			linkedAgents[node.LinkedHostAgentID] = node.Name
+		}
+	}
+
+	if len(linkedAgents) == 0 {
+		return discoveries
+	}
+
+	// Check which agents actually have discovery data
+	hasAgentDiscovery := make(map[string]bool)
+	for _, d := range discoveries {
+		if d.ResourceType == ResourceTypeHost {
+			// d.HostID is usually the agent ID for host resources
+			if _, ok := linkedAgents[d.HostID]; ok {
+				hasAgentDiscovery[d.HostID] = true
+			}
+		}
+	}
+
+	// Filter out PVE node discoveries if the corresponding agent discovery exists
+	filtered := make([]*ResourceDiscovery, 0, len(discoveries))
+	for _, d := range discoveries {
+		if d.ResourceType == ResourceTypeHost {
+			// If this discovery is for a PVE node (by name/ID)
+			// check if it maps to an agent that ALREADY has a discovery in this list
+
+			// Is this discovery's ID satisfying a Node check?
+			isPVENode := false
+			var linkedAgentID string
+
+			for _, node := range state.Nodes {
+				if d.HostID == node.Name || d.HostID == node.ID || d.ResourceID == node.Name {
+					isPVENode = true
+					linkedAgentID = node.LinkedHostAgentID
+					break
+				}
+			}
+
+			if isPVENode && linkedAgentID != "" && hasAgentDiscovery[linkedAgentID] && d.HostID != linkedAgentID {
+				// We have the agent discovery, so skip this redundant PVE node discovery
+				continue
+			}
+		}
+		filtered = append(filtered, d)
+	}
+
+	return filtered
+}
+
 // upgradeDiscoveryIfNeeded upgrades cached discovery fields to current versions.
 // This ensures cached discoveries get the new instructional CLI access format
 // and have hostname populated without requiring a full re-discovery.

internal/servicediscovery/service_test.go

@@ -957,3 +957,136 @@ func TestParseDockerMounts(t *testing.T) {
 		})
 	}
 }
+
+func TestService_Redirection(t *testing.T) {
+	// Setup store
+	store, err := NewStore(t.TempDir())
+	if err != nil {
+		t.Fatalf("NewStore error: %v", err)
+	}
+	store.crypto = nil
+
+	// Setup state with a linked PVE node
+	state := StateSnapshot{
+		Nodes: []Node{
+			{
+				ID:                "pve-id-1",
+				Name:              "pve1",
+				LinkedHostAgentID: "agent-pve1",
+			},
+		},
+		Hosts: []Host{
+			{
+				ID:       "agent-pve1",
+				Hostname: "pve1-host",
+			},
+		},
+	}
+
+	// Setup service
+	service := NewService(store, nil, stubStateProvider{state: state}, DefaultConfig())
+	service.SetAIAnalyzer(&stubAnalyzer{
+		response: `{"service_type":"proxmox","service_name":"Proxmox VE","service_version":"8.0","category":"virtualizer","cli_access":"ssh root@pve1","facts":[],"config_paths":[],"data_paths":[],"ports":[],"confidence":0.9,"reasoning":"test"}`,
+	})
+
+	ctx := context.Background()
+
+	// 1. Test DiscoverResource redirection
+	// Trigger discovery for the PVE node "pve1"
+	req := DiscoveryRequest{
+		ResourceType: ResourceTypeHost,
+		HostID:       "pve1",
+		ResourceID:   "pve1",
+		Force:        true,
+	}
+
+	discovery, err := service.DiscoverResource(ctx, req)
+	if err != nil {
+		t.Fatalf("DiscoverResource error: %v", err)
+	}
+
+	// The discovery should be associated with the AGENT ID, not the NODE ID
+	expectedID := MakeResourceID(ResourceTypeHost, "agent-pve1", "agent-pve1") // Host resources usually have HostID == ResourceID
+	if discovery.ID != expectedID {
+		t.Errorf("DiscoverResource ID mismatch. Got %s, want %s (should have redirected to agent ID)", discovery.ID, expectedID)
+	}
+	if discovery.HostID != "agent-pve1" {
+		t.Errorf("DiscoverResource HostID mismatch. Got %s, want agent-pve1", discovery.HostID)
+	}
+
+	// 2. Test GetDiscoveryByResource redirection (standard case)
+	// Try to get discovery using the PVE node name
+	got, err := service.GetDiscoveryByResource(ResourceTypeHost, "pve1", "pve1")
+	if err != nil {
+		t.Fatalf("GetDiscoveryByResource error: %v", err)
+	}
+	if got == nil {
+		t.Fatalf("GetDiscoveryByResource returned nil")
+	}
+
+	// It should return the discovery we just created (which is under agent-pve1)
+	if got.ID != expectedID {
+		t.Errorf("GetDiscoveryByResource returned wrong discovery. Got ID %s, want %s", got.ID, expectedID)
+	}
+
+	// 3. Test GetDiscoveryByResource fallback
+	// Create a "legacy" discovery that only exists under the node ID
+	legacyID := MakeResourceID(ResourceTypeHost, "pve1", "pve1")
+	legacyDiscovery := &ResourceDiscovery{
+		ID:           legacyID,
+		ResourceType: ResourceTypeHost,
+		HostID:       "pve1",
+		ResourceID:   "pve1",
+		ServiceName:  "Legacy PVE",
+	}
+	if err := store.Save(legacyDiscovery); err != nil {
+		t.Fatalf("Failed to save legacy discovery: %v", err)
+	}
+
+	// Temporarily remove the "agent" discovery to force fallback
+	if err := store.Delete(expectedID); err != nil {
+		t.Fatalf("Failed to delete agent discovery: %v", err)
+	}
+
+	// Try to get "pve1" again. It should redirect to agent-pve1 (not found), then fallback to pve1 (found)
+	gotLegacy, err := service.GetDiscoveryByResource(ResourceTypeHost, "pve1", "pve1")
+	if err != nil {
+		t.Fatalf("GetDiscoveryByResource fallback error: %v", err)
+	}
+	if gotLegacy == nil {
+		t.Fatalf("GetDiscoveryByResource fallback returned nil")
+	}
+	if gotLegacy.ID != legacyID {
+		t.Errorf("GetDiscoveryByResource fallback returned wrong ID. Got %s, want %s", gotLegacy.ID, legacyID)
+	}
+
+	// 4. Test Deduplication
+	// Restore the agent discovery, so we have BOTH legacy (pve1) and agent (agent-pve1)
+	if err := store.Save(discovery); err != nil {
+		t.Fatalf("Failed to restore agent discovery: %v", err)
+	}
+
+	list, err := service.ListDiscoveries()
+	if err != nil {
+		t.Fatalf("ListDiscoveries error: %v", err)
+	}
+
+	// We expect deduplication to remove the legacy pve1 entry because agent-pve1 exists
+	foundLegacy := false
+	foundAgent := false
+	for _, d := range list {
+		if d.ID == legacyID {
+			foundLegacy = true
+		}
+		if d.ID == expectedID {
+			foundAgent = true
+		}
+	}
+
+	if foundLegacy {
+		t.Errorf("Deduplication failed: Legacy PVE node discovery should have been filtered out")
+	}
+	if !foundAgent {
+		t.Errorf("Deduplication failed: Agent discovery should be present")
+	}
+}