Resolve discovery lookups across forked host identities

Host identities fork onto "<base>-<hex>" when an agent re-enrolls under a new token (re-running an install with --proxmox is enough), and discovery then stores records under the forked host ID while the PVE node keeps reporting the base agent UUID as its linked agent. Store.ListByTarget matches TargetID byte-for-byte and ListDiscoveriesByTarget did no alias resolution, so the resource drawer's list call returned an empty 200 and the Discovery tab reported "no saved discovery run" for hosts that had been analyzed. GetDiscoveryByResource's normalization missed too: a base UUID matches neither the forked host ID nor its hostname. Expand the requested target into every spelling that provably names the same target (equivalentDiscoveryTargetIDs): a matched node's linked agent ID and name, a matched host's own ID/hostname pairing, and hex fork suffixes of any of those, including the twice-forked truncated-base shape. Filter the list path against that set and seed the same spellings into the single-record alias path. Trimmed from PR #1715 as submitted: the transitive hostname closure (node name -> hostnames -> other hosts' IDs) is dropped. Multi-estate deployments legitimately reuse node names and hostnames, and the hop would have surfaced another estate's records in the drawer, discovery readiness, and AI context. A regression test pins the non-bridging behavior.
2026-09-10 02:25:56 +00:00 · 2026-08-12 07:30:50 +00:00
parent 08e501bbc8
commit 0e8f63cdd5
2 changed files with 347 additions and 1 deletions
@@ -3035,6 +3035,15 @@ func (s *Service) GetDiscoveryByResource(resourceType ResourceType, targetID, re
 		ResourceID:   resourceID,
 	}
 	aliasIDs := []string{MakeResourceID(resourceType, targetID, resourceID)}
+	// A forked host identity is not reachable through normalizeDiscoveryRequest,
+	// which only matches exact snapshot IDs and hostnames. Seed the equivalent
+	// spellings of the target so records stored under a fork still resolve.
+	for _, candidate := range s.equivalentDiscoveryTargetIDs(targetID) {
+		aliasIDs = append(aliasIDs, MakeResourceID(resourceType, candidate, candidate))
+		if resourceID != "" && resourceID != candidate {
+			aliasIDs = append(aliasIDs, MakeResourceID(resourceType, candidate, resourceID))
+		}
+	}
 	req = s.normalizeDiscoveryRequest(req, &aliasIDs)

 	d, err := s.store.GetByResource(resourceType, req.TargetID, req.ResourceID)
@@ -3091,11 +3100,38 @@ func (s *Service) ListDiscoveriesByType(resourceType ResourceType) ([]*ResourceD
 }

 // ListDiscoveriesByTarget returns discoveries for a specific target ID.
+//
+// The store matches TargetID byte-for-byte, but callers legitimately hold a
+// different spelling of the same target than the record was stored under: a PVE
+// node reports its linked agent by base agent UUID while the host record itself
+// may have forked onto a suffixed identity. Resolve the request through
+// equivalentDiscoveryTargetIDs so those spellings still find their records.
 func (s *Service) ListDiscoveriesByTarget(targetID string) ([]*ResourceDiscovery, error) {
-	discoveries, err := s.store.ListByTarget(targetID)
+	candidates := s.equivalentDiscoveryTargetIDs(targetID)
+	if len(candidates) == 0 {
+		return nil, nil
+	}
+
+	all, err := s.store.List()
 	if err != nil {
 		return nil, fmt.Errorf("list discoveries by target %q: %w", targetID, err)
 	}
+
+	wanted := make(map[string]struct{}, len(candidates))
+	for _, candidate := range candidates {
+		wanted[candidate] = struct{}{}
+	}
+
+	discoveries := make([]*ResourceDiscovery, 0, len(all))
+	for _, d := range all {
+		if d == nil {
+			continue
+		}
+		if _, ok := wanted[strings.TrimSpace(d.TargetID)]; ok {
+			discoveries = append(discoveries, d)
+		}
+	}
+
 	discoveries = s.deduplicateDiscoveries(discoveries)
 	for _, d := range discoveries {
 		s.upgradeCLIAccessIfNeeded(d)
@@ -3103,6 +3139,104 @@ func (s *Service) ListDiscoveriesByTarget(targetID string) ([]*ResourceDiscovery
 	return discoveries, nil
 }

+// equivalentDiscoveryTargetIDs returns every target ID that identifies the same
+// target as targetID, starting with targetID itself.
+//
+// Host identities fork onto a "<base>-<hex suffix>" form when an agent re-enrolls
+// under a new token against an existing record (see the host token binding logic
+// in internal/monitoring) - re-running an install with --proxmox is enough to
+// trigger it. Discovery then stores records under the forked host ID while the
+// PVE node still reports the base agent UUID as its linked agent, so a lookup by
+// base UUID finds nothing. Bridge the two through the live snapshot.
+//
+// Expansion is limited to spellings that provably name the requested target: a
+// matched node's linked agent ID and name, a matched host's own ID/hostname
+// pairing, and hex fork suffixes of any of those. Hostname equality is never
+// followed to a different host's ID - multi-estate deployments legitimately
+// reuse node names and hostnames, and a transitive hop would surface another
+// estate's records here, in discovery readiness, and in AI context.
+func (s *Service) equivalentDiscoveryTargetIDs(targetID string) []string {
+	trimmed := strings.TrimSpace(targetID)
+	if trimmed == "" {
+		return nil
+	}
+
+	ids := []string{trimmed}
+	seen := map[string]struct{}{trimmed: {}}
+	add := func(id string) {
+		id = strings.TrimSpace(id)
+		if id == "" {
+			return
+		}
+		if _, ok := seen[id]; ok {
+			return
+		}
+		seen[id] = struct{}{}
+		ids = append(ids, id)
+	}
+
+	snap, ok := s.getSnapshot()
+	if !ok {
+		return ids
+	}
+
+	for _, node := range snap.Nodes {
+		if node.LinkedAgentID == trimmed || node.ID == trimmed || node.Name == trimmed {
+			add(node.LinkedAgentID)
+			add(node.Name)
+		}
+	}
+	for _, host := range snap.Hosts {
+		switch {
+		case host.ID == trimmed:
+			add(host.Hostname)
+		case host.Hostname != "" && host.Hostname == trimmed:
+			add(host.ID)
+		}
+	}
+
+	// A fork suffix can hang off any spelling collected above, not just the
+	// requested one: a node-name lookup reaches the fork through the node's
+	// linked agent UUID.
+	direct := ids
+	for _, host := range snap.Hosts {
+		for _, candidate := range direct {
+			if isForkedHostIdentity(host.ID, candidate) {
+				add(host.ID)
+				break
+			}
+		}
+	}
+
+	return ids
+}
+
+// isForkedHostIdentity reports whether candidate is base carrying one or more
+// host-identity fork suffixes. Forks append "-<hex>", and a base longer than 40
+// characters is truncated before the next suffix is appended, so a twice-forked
+// identity carries a partial suffix followed by a full one. Requiring every
+// trailing segment to be hex keeps unrelated IDs that merely share a prefix out.
+func isForkedHostIdentity(candidate, base string) bool {
+	if candidate == "" || base == "" || candidate == base {
+		return false
+	}
+	tail, ok := strings.CutPrefix(candidate, base+"-")
+	if !ok {
+		return false
+	}
+	for _, segment := range strings.Split(tail, "-") {
+		if segment == "" {
+			return false
+		}
+		for _, r := range segment {
+			if (r < '0' || r > '9') && (r < 'a' || r > 'f') {
+				return false
+			}
+		}
+	}
+	return true
+}
+
 // 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.
@@ -2455,3 +2455,215 @@ func TestService_DiscoverResource_ReturnsUpgradedCachedDiscovery(t *testing.T) {
 		t.Fatalf("expected cleaned AI reasoning without legacy URL note, got %q", got.AIReasoning)
 	}
 }
+
+// Regression: a host identity that forked onto a "<base>-<hex>" spelling (an
+// agent re-enrolling under a new token against an existing record) stored its
+// discovery under the forked host ID, while the PVE node kept reporting the base
+// agent UUID as its linked agent. The store matches TargetID byte-for-byte, so
+// the resource drawer's list-by-agent call returned an empty 200 and the
+// Discovery tab reported "no saved discovery run" for a host it had analyzed.
+func TestService_ListDiscoveriesByTarget_ResolvesForkedHostIdentity(t *testing.T) {
+	const baseAgentID = "342f337b-d2a9-4316-a998-d09a2abe3e8f"
+	const forkedAgentID = baseAgentID + "-bffd0339"
+
+	store, err := NewStore(t.TempDir())
+	if err != nil {
+		t.Fatalf("NewStore error: %v", err)
+	}
+	store.crypto = nil
+
+	service := NewService(store, nil, DefaultConfig())
+	service.SetReadState(readStateFromSnapshot(StateSnapshot{
+		Hosts: []Host{{ID: forkedAgentID, Hostname: "nova.biz", Status: "online"}},
+		Nodes: []Node{{ID: "node-nova", Name: "nova", LinkedAgentID: baseAgentID}},
+	}))
+
+	discovery := &ResourceDiscovery{
+		ID:           MakeResourceID(ResourceTypeAgent, forkedAgentID, forkedAgentID),
+		ResourceType: ResourceTypeAgent,
+		TargetID:     forkedAgentID,
+		ResourceID:   forkedAgentID,
+		Hostname:     "nova.biz",
+		ServiceType:  "proxmox",
+	}
+	if err := store.Save(discovery); err != nil {
+		t.Fatalf("Save error: %v", err)
+	}
+
+	got, err := service.ListDiscoveriesByTarget(baseAgentID)
+	if err != nil {
+		t.Fatalf("ListDiscoveriesByTarget error: %v", err)
+	}
+	if len(got) != 1 {
+		t.Fatalf("expected 1 discovery for base agent ID, got %d", len(got))
+	}
+	if got[0].TargetID != forkedAgentID {
+		t.Fatalf("TargetID = %q, want %q", got[0].TargetID, forkedAgentID)
+	}
+
+	// The forked spelling must keep working, as must the single-record path.
+	if direct, err := service.ListDiscoveriesByTarget(forkedAgentID); err != nil || len(direct) != 1 {
+		t.Fatalf("ListDiscoveriesByTarget(forked) = %d discoveries, err %v; want 1, nil", len(direct), err)
+	}
+	byResource, err := service.GetDiscoveryByResource(ResourceTypeAgent, baseAgentID, baseAgentID)
+	if err != nil {
+		t.Fatalf("GetDiscoveryByResource error: %v", err)
+	}
+	if byResource == nil || byResource.TargetID != forkedAgentID {
+		t.Fatalf("GetDiscoveryByResource returned %#v, want the forked record", byResource)
+	}
+}
+
+// A base longer than 40 characters is truncated before the next fork suffix is
+// appended, so a twice-forked identity carries a partial suffix then a full one.
+func TestService_ListDiscoveriesByTarget_ResolvesTwiceForkedHostIdentity(t *testing.T) {
+	const baseAgentID = "ea79df24-0d3b-453f-8ce0-cc08e2b96f86"
+	const forkedAgentID = baseAgentID + "-d3b-83adbde6"
+
+	store, err := NewStore(t.TempDir())
+	if err != nil {
+		t.Fatalf("NewStore error: %v", err)
+	}
+	store.crypto = nil
+
+	service := NewService(store, nil, DefaultConfig())
+	service.SetReadState(readStateFromSnapshot(StateSnapshot{
+		Hosts: []Host{{ID: forkedAgentID, Hostname: "luna.vp.diskiller.net", Status: "online"}},
+	}))
+
+	if err := store.Save(&ResourceDiscovery{
+		ID:           MakeResourceID(ResourceTypeAgent, forkedAgentID, forkedAgentID),
+		ResourceType: ResourceTypeAgent,
+		TargetID:     forkedAgentID,
+		ResourceID:   forkedAgentID,
+		Hostname:     "luna.vp.diskiller.net",
+		ServiceType:  "proxmox",
+	}); err != nil {
+		t.Fatalf("Save error: %v", err)
+	}
+
+	got, err := service.ListDiscoveriesByTarget(baseAgentID)
+	if err != nil {
+		t.Fatalf("ListDiscoveriesByTarget error: %v", err)
+	}
+	if len(got) != 1 {
+		t.Fatalf("expected 1 discovery for twice-forked identity, got %d", len(got))
+	}
+}
+
+// Target expansion must not sweep in unrelated hosts that merely share a prefix,
+// and must stay quiet when no snapshot is available.
+func TestService_EquivalentDiscoveryTargetIDs_Bounds(t *testing.T) {
+	const baseAgentID = "342f337b-d2a9-4316-a998-d09a2abe3e8f"
+
+	service := NewService(nil, nil, DefaultConfig())
+	if got := service.equivalentDiscoveryTargetIDs(baseAgentID); len(got) != 1 || got[0] != baseAgentID {
+		t.Fatalf("without a snapshot, expected only the requested ID, got %#v", got)
+	}
+	if got := service.equivalentDiscoveryTargetIDs("   "); got != nil {
+		t.Fatalf("expected nil for a blank target, got %#v", got)
+	}
+
+	service.SetReadState(readStateFromSnapshot(StateSnapshot{
+		Hosts: []Host{
+			{ID: baseAgentID + "-bffd0339", Hostname: "nova.biz", Status: "online"},
+			{ID: baseAgentID + "-replica", Hostname: "clone.biz", Status: "online"},
+			{ID: "unrelated-agent", Hostname: "other.biz", Status: "online"},
+		},
+	}))
+
+	got := service.equivalentDiscoveryTargetIDs(baseAgentID)
+	for _, unwanted := range []string{baseAgentID + "-replica", "unrelated-agent"} {
+		for _, id := range got {
+			if id == unwanted {
+				t.Fatalf("expansion pulled in %q: %#v", unwanted, got)
+			}
+		}
+	}
+	var sawFork bool
+	for _, id := range got {
+		if id == baseAgentID+"-bffd0339" {
+			sawFork = true
+		}
+	}
+	if !sawFork {
+		t.Fatalf("expansion missed the hex-suffixed fork: %#v", got)
+	}
+}
+
+func TestIsForkedHostIdentity(t *testing.T) {
+	const base = "342f337b-d2a9-4316-a998-d09a2abe3e8f"
+	tests := []struct {
+		candidate string
+		want      bool
+	}{
+		{base + "-bffd0339", true},
+		{base + "-d3b-83adbde6", true},
+		{base, false},
+		{base + "-replica", false},
+		{base + "-", false},
+		{base + "-bffd0339-", false},
+		{"other-" + base, false},
+		{"", false},
+	}
+	for _, tt := range tests {
+		if got := isForkedHostIdentity(tt.candidate, base); got != tt.want {
+			t.Errorf("isForkedHostIdentity(%q, base) = %v, want %v", tt.candidate, got, tt.want)
+		}
+	}
+}
+
+// Multi-estate deployments legitimately run same-named nodes and hosts. A
+// lookup by one estate's agent UUID must not hop through the shared name to the
+// other estate's host ID and surface its records - the drawer, discovery
+// readiness, and AI context would all show the wrong host's results.
+func TestService_ListDiscoveriesByTarget_DoesNotBridgeSharedHostnames(t *testing.T) {
+	const agentA = "aaaaaaaa-1111-4316-a998-d09a2abe3e8f"
+	const agentB = "bbbbbbbb-2222-4316-a998-d09a2abe3e8f"
+
+	store, err := NewStore(t.TempDir())
+	if err != nil {
+		t.Fatalf("NewStore error: %v", err)
+	}
+	store.crypto = nil
+
+	service := NewService(store, nil, DefaultConfig())
+	service.SetReadState(readStateFromSnapshot(StateSnapshot{
+		Hosts: []Host{
+			{ID: agentA, Hostname: "pve1", Status: "online"},
+			{ID: agentB, Hostname: "pve1", Status: "online"},
+		},
+		Nodes: []Node{
+			{ID: "estate-a/pve1", Name: "pve1", LinkedAgentID: agentA},
+			{ID: "estate-b/pve1", Name: "pve1", LinkedAgentID: agentB},
+		},
+	}))
+
+	// Estate B has run discovery; estate A has not.
+	if err := store.Save(&ResourceDiscovery{
+		ID:           MakeResourceID(ResourceTypeAgent, agentB, agentB),
+		ResourceType: ResourceTypeAgent,
+		TargetID:     agentB,
+		ResourceID:   agentB,
+		Hostname:     "pve1",
+		ServiceType:  "proxmox",
+	}); err != nil {
+		t.Fatalf("Save error: %v", err)
+	}
+
+	got, err := service.ListDiscoveriesByTarget(agentA)
+	if err != nil {
+		t.Fatalf("ListDiscoveriesByTarget error: %v", err)
+	}
+	if len(got) != 0 {
+		t.Fatalf("expected no discoveries for estate A, got %d (leaked across shared hostname)", len(got))
+	}
+
+	own, err := service.ListDiscoveriesByTarget(agentB)
+	if err != nil {
+		t.Fatalf("ListDiscoveriesByTarget(agentB) error: %v", err)
+	}
+	if len(own) != 1 {
+		t.Fatalf("expected estate B to keep finding its own record, got %d", len(own))
+	}
+}