Learn where Proxmox guests are, and stop a sweep duplicating them

Proxmox only records a guest's address when someone set one statically, so
on a DHCP estate every guest arrived with no address at all. That cost
more than the field: a network sweep of another subnet has no ARP entry to
work from, so it identifies a host by address alone — and with the guests
carrying no address there was nothing to match, leaving a second, emptier
card beside every guest the hypervisor had already described in full.

Guests are now asked where they are. A VM answers through its
qemu-guest-agent, a container through its running interfaces; a stopped
guest is never asked, since it has nothing to report and the call is one
round trip per guest.

Choosing among the answers is the hard part. A guest that runs containers
reports several interfaces — docker0 and its per-network bridges, Home
Assistant's hassio, any VPN tunnel — and recording 172.17.0.1 as the
machine's address would be worse than recording nothing, because every
container host on the estate reports the same one. The NIC MACs Proxmox
assigned are the discriminator: an interface carrying one is a NIC the
hypervisor gave the guest, anything else the guest invented. Where the
MACs cannot be read, nothing is claimed. A statically configured address
still wins, being what the administrator asked for.

With addresses in hand a sweep's find can be matched to the guest it
actually is, so the resolver gains an address bridge beside the MAC one.
Narrow on purpose: only a scan-grade card that produced no MAC of its own,
only against an agent-grade card of the same kind, and only when exactly
one stored system claims that address. Services anchor to the stored card
in preference to a sweep's stand-in for the same reason.

On a live two-node estate this turned nine scanned cards on the guest
subnet into six guests updated in place and two genuinely unknown hosts,
with the discovered web services landing on the real VMs rather than on
stand-ins.

The read orchestration moves into the domain on the way past. The CLI and
the MCP tool each had a copy, and the copies had already drifted — the MCP
one dropped the guests' MACs, silently costing every guest its chance of
unifying with a scan. Now there is one, and it is tested.

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

RackPeek.Domain/Discovery/DiscoveryIdResolver.cs

@@ -41,6 +41,7 @@ public static class DiscoveryIdResolver {
             .ToDictionary(r => r.DiscoveryId!, r => r, StringComparer.OrdinalIgnoreCase);
 
         Dictionary<string, Resource> existingByMac = BuildMacMap(existing);
+        Dictionary<string, Resource> existingByIp = BuildIpMap(existing);
 
         // Tolerant of a hand-edited file that managed to get two resources of the
         // same name: the first wins, rather than crashing the import.
@@ -52,7 +53,7 @@ public static class DiscoveryIdResolver {
         var renames = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase);
 
         foreach (Resource resource in incomingWithId) {
-            var resolved = ResolveName(resource, existingById, existingByName, existingByMac);
+            var resolved = ResolveName(resource, existingById, existingByName, existingByMac, existingByIp);
 
             if (resolved.Equals(resource.Name, StringComparison.OrdinalIgnoreCase))
                 continue;
@@ -102,18 +103,9 @@ public static class DiscoveryIdResolver {
 
         // Both sides count: the host may have arrived in this very payload (discover
         // docker emits it alongside its services) or be sitting in the inventory already.
-        var systemsByIp = new Dictionary<string, List<string>>(StringComparer.OrdinalIgnoreCase);
-
-        foreach (SystemResource system in existing.OfType<SystemResource>().Concat(incoming.OfType<SystemResource>())) {
-            if (string.IsNullOrWhiteSpace(system.Ip))
-                continue;
-
-            if (!systemsByIp.TryGetValue(system.Ip, out List<string>? names))
-                systemsByIp[system.Ip] = names = [];
-
-            if (!names.Contains(system.Name, StringComparer.OrdinalIgnoreCase))
-                names.Add(system.Name);
-        }
+        // Stored systems are gathered separately because they win — see below.
+        Dictionary<string, List<string>> storedByIp = IndexByIp(existing);
+        Dictionary<string, List<string>> arrivingByIp = IndexByIp(incoming);
 
         foreach (Service service in services) {
             var ip = service.Network?.Ip;
@@ -127,11 +119,37 @@ public static class DiscoveryIdResolver {
             if (anchored)
                 continue;
 
-            if (!systemsByIp.TryGetValue(ip, out List<string>? candidates) || candidates.Count != 1)
+            // A stored system beats one arriving in this payload when both claim the
+            // address. They are usually the same machine seen twice — a hypervisor knows
+            // its guest by name and specification, a sweep only found something
+            // answering — and the stored card is the one a person recognises.
+            List<string>? candidates =
+                storedByIp.TryGetValue(ip, out List<string>? stored) ? stored
+                : arrivingByIp.TryGetValue(ip, out List<string>? arriving) ? arriving
+                : null;
+
+            if (candidates is not [var host])
+                continue;
+
+            service.RunsOn = [host];
+        }
+    }
+
+    private static Dictionary<string, List<string>> IndexByIp(IReadOnlyList<Resource> resources) {
+        var byIp = new Dictionary<string, List<string>>(StringComparer.OrdinalIgnoreCase);
+
+        foreach (SystemResource system in resources.OfType<SystemResource>()) {
+            if (string.IsNullOrWhiteSpace(system.Ip))
                 continue;
 
-            service.RunsOn = [candidates[0]];
+            if (!byIp.TryGetValue(system.Ip, out List<string>? names))
+                byIp[system.Ip] = names = [];
+
+            if (!names.Contains(system.Name, StringComparer.OrdinalIgnoreCase))
+                names.Add(system.Name);
         }
+
+        return byIp;
     }
 
     /// <summary>
@@ -170,7 +188,8 @@ public static class DiscoveryIdResolver {
         Resource resource,
         Dictionary<string, Resource> existingById,
         Dictionary<string, Resource> existingByName,
-        Dictionary<string, Resource> existingByMac) {
+        Dictionary<string, Resource> existingByMac,
+        Dictionary<string, Resource> existingByIp) {
         // Known id: the stored resource wins on name, whatever the user has renamed it to.
         if (existingById.TryGetValue(resource.DiscoveryId!, out Resource? matched))
             return matched.Name;
@@ -180,6 +199,9 @@ public static class DiscoveryIdResolver {
         if (TryUnifyByMac(resource, existingByMac, out var unifiedName))
             return unifiedName;
 
+        if (TryUnifyByIp(resource, existingByIp, out unifiedName))
+            return unifiedName;
+
         // Unknown id and the name is free: nothing to reconcile.
         if (!existingByName.TryGetValue(resource.Name, out Resource? sameName))
             return resource.Name;
@@ -258,6 +280,76 @@ public static class DiscoveryIdResolver {
         return false;
     }
 
+    /// <summary>
+    ///     The bridge for machines a sweep cannot identify by MAC at all: ARP is
+    ///     link-local, so a host on any subnet but the scanner's own yields no MAC and
+    ///     its identity falls back to its address. Once a hypervisor reports its guests'
+    ///     addresses, that same address is the only thing tying the sweep's find to the
+    ///     guest the inventory already describes in full.
+    ///     <para>
+    ///         Narrow on purpose. It applies only to a scan-grade card that produced no
+    ///         MAC of its own — one that has a MAC was either already unified above or
+    ///         genuinely disagrees, and a MAC is better evidence than an address. The
+    ///         stored card must be agent-grade and of the same kind, and must be the only
+    ///         one claiming that address: two cards on one address is a conflict or an
+    ///         overlapping subnet, neither of which is evidence of anything.
+    ///     </para>
+    /// </summary>
+    private static bool TryUnifyByIp(
+        Resource resource,
+        Dictionary<string, Resource> existingByIp,
+        out string unifiedName) {
+        unifiedName = string.Empty;
+
+        if (DiscoveryId.Scheme(resource.DiscoveryId) != DiscoveryId.NetworkScheme)
+            return false;
+
+        // A scan that saw a MAC has better evidence than an address, and the MAC rule
+        // above has already had its say.
+        if (MacsOf(resource).Any())
+            return false;
+
+        if (resource is not SystemResource { Ip: { } ip } || string.IsNullOrWhiteSpace(ip))
+            return false;
+
+        if (!existingByIp.TryGetValue(ip, out Resource? stored)
+            || stored.GetType() != resource.GetType())
+            return false;
+
+        // Another scan card at the same address says nothing: both are stand-ins.
+        if (DiscoveryId.Scheme(stored.DiscoveryId) == DiscoveryId.NetworkScheme)
+            return false;
+
+        // Same as the MAC bridge: the scan's weaker identity is dropped so the merge
+        // cannot downgrade the stored one.
+        resource.DiscoveryId = null;
+        unifiedName = stored.Name;
+
+        return true;
+    }
+
+    /// <summary>
+    ///     Systems by address, excluding any address more than one of them claims — an
+    ///     ambiguous address is not evidence.
+    /// </summary>
+    private static Dictionary<string, Resource> BuildIpMap(IReadOnlyList<Resource> existing) {
+        var byIp = new Dictionary<string, Resource>(StringComparer.OrdinalIgnoreCase);
+        var ambiguous = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
+
+        foreach (SystemResource system in existing.OfType<SystemResource>()) {
+            if (string.IsNullOrWhiteSpace(system.Ip))
+                continue;
+
+            if (!byIp.TryAdd(system.Ip, system))
+                ambiguous.Add(system.Ip);
+        }
+
+        foreach (var ip in ambiguous)
+            byIp.Remove(ip);
+
+        return byIp;
+    }
+
     /// <summary>The MACs a resource claims, from its "mac" and "macs" labels, normalised.</summary>
     private static IEnumerable<string> MacsOf(Resource resource) {
         IEnumerable<string?> raw = [

RackPeek.Domain/Discovery/IProxmoxClient.cs

@@ -41,4 +41,16 @@ public interface IProxmoxClient {
         string endpoint,
         int vmId,
         CancellationToken cancellationToken = default);
+
+    /// <summary>
+    ///     Addresses the guest reports for its own interfaces — the only way to learn a
+    ///     DHCP guest's address, since the config only carries one when it was set
+    ///     statically. Needs the guest agent for a VM and a running container for LXC,
+    ///     so an empty list is the normal answer for anything that has neither.
+    /// </summary>
+    Task<IReadOnlyList<ProxmoxGuestAddress>> GetGuestAddressesAsync(
+        string node,
+        string endpoint,
+        int vmId,
+        CancellationToken cancellationToken = default);
 }

RackPeek.Domain/Discovery/ProxmoxApiClient.cs

@@ -1,3 +1,4 @@
+using System.Text.Json;
 using System.Net.Security;
 
 namespace RackPeek.Domain.Discovery;
@@ -143,6 +144,35 @@ public sealed class ProxmoxApiClient : IProxmoxClient, IDisposable {
         }
     }
 
+    public async Task<IReadOnlyList<ProxmoxGuestAddress>> GetGuestAddressesAsync(
+        string node,
+        string endpoint,
+        int vmId,
+        CancellationToken cancellationToken = default) {
+        // Every failure here means the same thing: the guest cannot say where it is.
+        // No agent installed, agent not running, container stopped, guest deleted since
+        // it was listed, or a token without VM.Monitor — all leave the address unknown,
+        // which is what the guest config already told us.
+        try {
+            var path = endpoint == LxcEndpoint
+                ? $"nodes/{Uri.EscapeDataString(node)}/{endpoint}/{vmId}/interfaces"
+                : $"nodes/{Uri.EscapeDataString(node)}/{endpoint}/{vmId}/agent/network-get-interfaces";
+
+            var json = await GetAsync(path, cancellationToken);
+
+            return endpoint == LxcEndpoint
+                ? ProxmoxResponseParser.ParseContainerInterfaces(json)
+                : ProxmoxResponseParser.ParseAgentInterfaces(json);
+        }
+        catch (HttpRequestException) {
+            return [];
+        }
+        catch (JsonException) {
+            // A node that answers the agent call with an error body rather than a status.
+            return [];
+        }
+    }
+
     private async Task<string> FirstNodeAsync(CancellationToken cancellationToken) {
         IReadOnlyList<ProxmoxNode> nodes = await GetNodesAsync(cancellationToken);
 

RackPeek.Domain/Discovery/ProxmoxDiscovery.cs

@@ -0,0 +1,107 @@
+using RackPeek.Domain.Resources;
+
+namespace RackPeek.Domain.Discovery;
+
+/// <summary>
+///     Reads a whole Proxmox endpoint and maps it to resources.
+///     <para>
+///         Lives here rather than in a command so the CLI and the MCP tool run the same
+///         code. They used to hold a copy each, and the copies had already drifted — the
+///         MCP one dropped the guests' MACs, which silently cost every guest its chance
+///         of unifying with a network scan.
+///     </para>
+/// </summary>
+public static class ProxmoxDiscovery {
+    public static async Task<List<Resource>> ReadAsync(
+        IProxmoxClient client,
+        CancellationToken cancellationToken = default) {
+        var scope = await client.GetIdentityScopeAsync(cancellationToken);
+        IReadOnlyList<ProxmoxNode> listed = await client.GetNodesAsync(cancellationToken);
+
+        var nodes = new List<ProxmoxNode>();
+        var guests = new List<ProxmoxGuest>();
+
+        foreach (ProxmoxNode listedNode in listed) {
+            // Node detail needs a broader permission than listing guests does, so it is
+            // enrichment rather than a requirement — a read-only token still gets a tree.
+            ProxmoxNode node = await client.EnrichAsync(listedNode, cancellationToken);
+            nodes.Add(node);
+
+            var nodeName = node.Name;
+
+            foreach (var endpoint in new[] { ProxmoxApiClient.QemuEndpoint, ProxmoxApiClient.LxcEndpoint }) {
+                IReadOnlyList<ProxmoxGuest> listedGuests =
+                    await client.GetGuestsAsync(nodeName, endpoint, cancellationToken);
+
+                // The list call knows nothing about the OS, and for a container it does
+                // not know the address either. Both live in the guest's own config — one
+                // call per guest, so they run concurrently rather than one at a time.
+                ProxmoxGuestConfig[] configs = await Task.WhenAll(listedGuests.Select(g =>
+                    client.GetGuestConfigAsync(nodeName, endpoint, g.VmId, cancellationToken)));
+
+                // Ask the running guests where they are. The config only carries an
+                // address when someone set one statically, so on a DHCP estate this is
+                // the difference between every guest having an address and none of them
+                // having one — and an address is what lets a guest line up with the host
+                // a network sweep found at that address.
+                IReadOnlyList<ProxmoxGuestAddress>[] addresses = await Task.WhenAll(
+                    listedGuests.Select(g => IsRunning(g)
+                        ? client.GetGuestAddressesAsync(nodeName, endpoint, g.VmId, cancellationToken)
+                        : Task.FromResult<IReadOnlyList<ProxmoxGuestAddress>>([])));
+
+                for (var i = 0; i < listedGuests.Count; i++) {
+                    IReadOnlyList<string> macs = configs[i].Macs ?? [];
+
+                    guests.Add(listedGuests[i] with {
+                        Os = configs[i].Os,
+                        Ip = configs[i].Ip ?? SelectGuestIp(addresses[i], macs),
+                        Disks = configs[i].DiskBytes,
+                        PassthroughAddresses = configs[i].PassthroughAddresses,
+                        Macs = macs
+                    });
+                }
+            }
+        }
+
+        return ProxmoxResourceMapper.ToResources(scope, nodes, guests);
+    }
+
+    /// <summary>
+    ///     The address that belongs to the guest itself.
+    ///     <para>
+    ///         A guest agent reports every interface inside the machine, and a guest that
+    ///         runs containers has several: Docker's <c>docker0</c> and its per-network
+    ///         bridges, Home Assistant's <c>hassio</c>, any VPN tunnel. Recording
+    ///         172.17.0.1 as the machine's address would be worse than recording nothing,
+    ///         because every Docker host on the estate reports the same one.
+    ///     </para>
+    ///     <para>
+    ///         The NIC MACs Proxmox assigned are the discriminator: they are already read
+    ///         from the guest's config, and an interface carrying one is a NIC the
+    ///         hypervisor gave the guest rather than something the guest invented. When
+    ///         the MACs are unknown — a container, or a config the token cannot read —
+    ///         nothing is claimed, since a guess here is indistinguishable from a fact.
+    ///     </para>
+    /// </summary>
+    public static string? SelectGuestIp(
+        IReadOnlyList<ProxmoxGuestAddress> addresses,
+        IReadOnlyList<string> configuredMacs) {
+        if (addresses.Count == 0 || configuredMacs.Count == 0)
+            return null;
+
+        var allowed = new HashSet<string>(
+            configuredMacs.Select(ArpTableParser.NormaliseMac).Where(m => m != null)!,
+            StringComparer.OrdinalIgnoreCase);
+
+        if (allowed.Count == 0)
+            return null;
+
+        return addresses
+            .Where(a => ArpTableParser.NormaliseMac(a.Mac) is { } mac && allowed.Contains(mac))
+            .Select(a => a.Ip)
+            .FirstOrDefault();
+    }
+
+    private static bool IsRunning(ProxmoxGuest guest) =>
+        string.Equals(guest.Status, "running", StringComparison.OrdinalIgnoreCase);
+}

RackPeek.Domain/Discovery/ProxmoxModels.cs

@@ -62,6 +62,12 @@ public sealed record ProxmoxGuest {
 
     public IReadOnlyList<string> Tags { get; init; } = [];
 
+    /// <summary>
+    ///     <c>running</c>, <c>stopped</c> and friends, from the guest list. Only a running
+    ///     guest can be asked where it is, and a stopped one has no address to report.
+    /// </summary>
+    public string? Status { get; init; }
+
     /// <summary>Filled in from the guest's config, which is the only place it is known.</summary>
     public string? Os { get; init; }
 
@@ -424,6 +430,7 @@ public static class ProxmoxResponseParser {
             Cores = GetInt(element, "cpus") ?? 0,
             MemoryBytes = GetLong(element, "maxmem") ?? 0,
             DiskBytes = GetLong(element, "maxdisk") ?? 0,
+            Status = GetString(element, "status"),
             Tags = ParseTags(GetString(element, "tags"))
         };
     }
@@ -464,9 +471,105 @@ public static class ProxmoxResponseParser {
         element.TryGetProperty(name, out JsonElement value) && value.TryGetInt64(out var result)
             ? result
             : null;
+
+    /// <summary>
+    ///     Addresses a QEMU guest reports through its guest agent
+    ///     (<c>agent/network-get-interfaces</c>). The agent sees every interface inside
+    ///     the guest, including the bridges Docker and Home Assistant create, so the
+    ///     caller filters by the NIC MACs Proxmox actually assigned — see
+    ///     <see cref="ProxmoxDiscovery.SelectGuestIp" />.
+    /// </summary>
+    public static List<ProxmoxGuestAddress> ParseAgentInterfaces(string json) {
+        var addresses = new List<ProxmoxGuestAddress>();
+
+        using var document = JsonDocument.Parse(json);
+
+        if (!document.RootElement.TryGetProperty("data", out JsonElement data)
+            || data.ValueKind != JsonValueKind.Object
+            || !data.TryGetProperty("result", out JsonElement result)
+            || result.ValueKind != JsonValueKind.Array)
+            return addresses;
+
+        foreach (JsonElement iface in result.EnumerateArray()) {
+            if (iface.ValueKind != JsonValueKind.Object)
+                continue;
+
+            var name = GetString(iface, "name");
+            var mac = GetString(iface, "hardware-address");
+
+            if (!iface.TryGetProperty("ip-addresses", out JsonElement ips)
+                || ips.ValueKind != JsonValueKind.Array)
+                continue;
+
+            foreach (JsonElement entry in ips.EnumerateArray()) {
+                if (entry.ValueKind != JsonValueKind.Object)
+                    continue;
+
+                if (!string.Equals(GetString(entry, "ip-address-type"), "ipv4", StringComparison.OrdinalIgnoreCase))
+                    continue;
+
+                var ip = GetString(entry, "ip-address");
+
+                if (IsUsableAddress(ip))
+                    addresses.Add(new ProxmoxGuestAddress(name, mac, ip!));
+            }
+        }
+
+        return addresses;
+    }
+
+    /// <summary>
+    ///     Addresses a container reports through <c>lxc/{vmid}/interfaces</c>, which is a
+    ///     flat list rather than the agent's nested shape and spells the address with its
+    ///     prefix (<c>10.0.0.5/24</c>).
+    /// </summary>
+    public static List<ProxmoxGuestAddress> ParseContainerInterfaces(string json) {
+        var addresses = new List<ProxmoxGuestAddress>();
+
+        using var document = JsonDocument.Parse(json);
+
+        if (!document.RootElement.TryGetProperty("data", out JsonElement data)
+            || data.ValueKind != JsonValueKind.Array)
+            return addresses;
+
+        foreach (JsonElement iface in data.EnumerateArray()) {
+            if (iface.ValueKind != JsonValueKind.Object)
+                continue;
+
+            var ip = GetString(iface, "inet");
+
+            // "10.0.0.5/24" — the prefix belongs to the interface, not to the address
+            // the inventory records.
+            var slash = ip?.IndexOf('/') ?? -1;
+
+            if (slash > 0)
+                ip = ip![..slash];
+
+            if (IsUsableAddress(ip))
+                addresses.Add(new ProxmoxGuestAddress(
+                    GetString(iface, "name"),
+                    GetString(iface, "hwaddr"),
+                    ip!));
+        }
+
+        return addresses;
+    }
+
+    /// <summary>
+    ///     Whether an address is worth recording: a real IPv4 that is neither loopback
+    ///     nor the 169.254 a guest assigns itself when DHCP fails.
+    /// </summary>
+    private static bool IsUsableAddress(string? ip) =>
+        !string.IsNullOrWhiteSpace(ip)
+        && !ip.StartsWith("127.", StringComparison.Ordinal)
+        && !ip.StartsWith("169.254.", StringComparison.Ordinal)
+        && ip.Count(c => c == '.') == 3;
 }
 
 /// <summary>The parts of a guest's config worth recording. Everything is optional.</summary>
+/// <summary>One address a guest reports for one of its own interfaces.</summary>
+public sealed record ProxmoxGuestAddress(string? Interface, string? Mac, string Ip);
+
 public sealed record ProxmoxGuestConfig(
     string? Os,
     string? Ip,

RackPeek.Mcp/Tools/DiscoveryTools.cs

@@ -125,7 +125,7 @@ public sealed class DiscoveryTools(IServiceProvider services) {
 
             List<Resource> resources;
             try {
-                resources = await ReadProxmoxAsync(client, cancellationToken);
+                resources = await ProxmoxDiscovery.ReadAsync(client, cancellationToken);
             }
             catch (HttpRequestException ex) {
                 var hint = !insecure && ex.InnerException is System.Security.Authentication.AuthenticationException
@@ -156,40 +156,6 @@ public sealed class DiscoveryTools(IServiceProvider services) {
                });
     }
 
-    /// <summary>Same read orchestration as `rpk discover proxmox`.</summary>
-    private static async Task<List<Resource>> ReadProxmoxAsync(
-        IProxmoxClient client,
-        CancellationToken cancellationToken) {
-        var scope = await client.GetIdentityScopeAsync(cancellationToken);
-        IReadOnlyList<ProxmoxNode> listed = await client.GetNodesAsync(cancellationToken);
-
-        var nodes = new List<ProxmoxNode>();
-        var guests = new List<ProxmoxGuest>();
-
-        foreach (ProxmoxNode listedNode in listed) {
-            ProxmoxNode node = await client.EnrichAsync(listedNode, cancellationToken);
-            nodes.Add(node);
-
-            foreach (var endpoint in new[] { ProxmoxApiClient.QemuEndpoint, ProxmoxApiClient.LxcEndpoint }) {
-                IReadOnlyList<ProxmoxGuest> listedGuests =
-                    await client.GetGuestsAsync(node.Name, endpoint, cancellationToken);
-
-                ProxmoxGuestConfig[] configs = await Task.WhenAll(listedGuests.Select(g =>
-                    client.GetGuestConfigAsync(node.Name, endpoint, g.VmId, cancellationToken)));
-
-                for (var i = 0; i < listedGuests.Count; i++)
-                    guests.Add(listedGuests[i] with {
-                        Os = configs[i].Os,
-                        Ip = configs[i].Ip,
-                        Disks = configs[i].DiskBytes,
-                        PassthroughAddresses = configs[i].PassthroughAddresses
-                    });
-            }
-        }
-
-        return ProxmoxResourceMapper.ToResources(scope, nodes, guests);
-    }
-
     private async Task<DiscoveryResult> EmitAsync(List<Resource> resources, int skipped, bool apply) {
         var yaml = DiscoveryDocument.ToYaml(resources);
 

Shared.Rcl/Commands/Discovery/DiscoverProxmoxCommand.cs

@@ -73,7 +73,7 @@ public sealed class DiscoverProxmoxCommand : AsyncCommand<DiscoverProxmoxSetting
         List<Resource> resources;
 
         try {
-            resources = await ReadAsync(client, cancellationToken);
+            resources = await ProxmoxDiscovery.ReadAsync(client, cancellationToken);
         }
         catch (HttpRequestException ex) {
             AnsiConsole.MarkupLine(
@@ -98,45 +98,4 @@ public sealed class DiscoverProxmoxCommand : AsyncCommand<DiscoverProxmoxSetting
 
         return await DiscoveryOutput.EmitAsync(resources, settings, cancellationToken);
     }
-
-    private static async Task<List<Resource>> ReadAsync(
-        IProxmoxClient client,
-        CancellationToken cancellationToken) {
-        var scope = await client.GetIdentityScopeAsync(cancellationToken);
-        IReadOnlyList<ProxmoxNode> listed = await client.GetNodesAsync(cancellationToken);
-
-        var nodes = new List<ProxmoxNode>();
-        var guests = new List<ProxmoxGuest>();
-
-        foreach (ProxmoxNode listedNode in listed) {
-            // Node detail needs a broader permission than listing guests does, so it is
-            // enrichment rather than a requirement — a read-only token still gets a tree.
-            ProxmoxNode node = await client.EnrichAsync(listedNode, cancellationToken);
-            nodes.Add(node);
-
-            var nodeName = node.Name;
-
-            foreach (var endpoint in new[] { ProxmoxApiClient.QemuEndpoint, ProxmoxApiClient.LxcEndpoint }) {
-                IReadOnlyList<ProxmoxGuest> listedGuests =
-                    await client.GetGuestsAsync(nodeName, endpoint, cancellationToken);
-
-                // The list call knows nothing about the OS, and for a container it does
-                // not know the address either. Both live in the guest's own config — one
-                // call per guest, so they run concurrently rather than one at a time.
-                ProxmoxGuestConfig[] configs = await Task.WhenAll(listedGuests.Select(g =>
-                    client.GetGuestConfigAsync(nodeName, endpoint, g.VmId, cancellationToken)));
-
-                for (var i = 0; i < listedGuests.Count; i++)
-                    guests.Add(listedGuests[i] with {
-                        Os = configs[i].Os,
-                        Ip = configs[i].Ip,
-                        Disks = configs[i].DiskBytes,
-                        PassthroughAddresses = configs[i].PassthroughAddresses,
-                        Macs = configs[i].Macs ?? []
-                    });
-            }
-        }
-
-        return ProxmoxResourceMapper.ToResources(scope, nodes, guests);
-    }
 }

Shared.Rcl/wwwroot/raw_docs/discovery-guide.md

@@ -319,6 +319,33 @@ with no cluster uses its node name as the scope instead.
 
 ---
 
+### Where a guest's address comes from
+
+Proxmox only records an address in a guest's config when someone set one statically, so
+on a DHCP estate the config knows nothing. The guest itself does, and will say so: a VM
+through its **qemu-guest-agent**, a container through its running interfaces. Discovery
+asks every *running* guest, which is one extra call per guest and nothing at all for one
+that is switched off.
+
+A guest that runs containers has several interfaces — Docker's `docker0` and its
+per-network bridges, Home Assistant's `hassio`, any VPN tunnel — and recording
+`172.17.0.1` as the machine's address would be worse than recording nothing, because
+every container host on the estate reports the same one. So the NIC MACs Proxmox assigned
+are used as the discriminator: an interface carrying one is a NIC the hypervisor gave the
+guest, anything else is something the guest invented. Where the MACs cannot be read,
+nothing is claimed.
+
+No agent, a stopped guest, or a token without `VM.Monitor` all mean the same thing —
+the address stays unknown, exactly as before.
+
+**Why it matters beyond the address itself.** A guest's address is what lets
+`rpk discover network` recognise it. ARP is link-local, so a sweep of any subnet but its
+own gets no MAC and can only identify a host by address; once the hypervisor has reported
+that same address, the sweep's find is matched to the guest the hypervisor already
+described in full rather than becoming a second, emptier card beside it.
+
+---
+
 ## `rpk discover network`
 
 The collector for machines nothing else can describe: no agent, no API — just an

Tests.Discovery/Fixtures/pve-agent-interfaces.json

@@ -0,0 +1,36 @@
+{
+  "data": {
+    "result": [
+      {
+        "name": "lo",
+        "hardware-address": "00:00:00:00:00:00",
+        "ip-addresses": [
+          { "ip-address": "127.0.0.1", "ip-address-type": "ipv4", "prefix": 8 },
+          { "ip-address": "::1", "ip-address-type": "ipv6", "prefix": 128 }
+        ]
+      },
+      {
+        "name": "ens18",
+        "hardware-address": "bc:24:11:00:1a:01",
+        "ip-addresses": [
+          { "ip-address": "192.0.2.105", "ip-address-type": "ipv4", "prefix": 24 },
+          { "ip-address": "fe80::be24:11ff:fe00:1a01", "ip-address-type": "ipv6", "prefix": 64 }
+        ]
+      },
+      {
+        "name": "docker0",
+        "hardware-address": "02:42:9a:11:22:33",
+        "ip-addresses": [
+          { "ip-address": "172.17.0.1", "ip-address-type": "ipv4", "prefix": 16 }
+        ]
+      },
+      {
+        "name": "br-0d764d475870",
+        "hardware-address": "02:42:7e:44:55:66",
+        "ip-addresses": [
+          { "ip-address": "172.18.0.1", "ip-address-type": "ipv4", "prefix": 16 }
+        ]
+      }
+    ]
+  }
+}

Tests.Discovery/Fixtures/pve-lxc-interfaces.json

@@ -0,0 +1,16 @@
+{
+  "data": [
+    {
+      "name": "lo",
+      "hwaddr": "00:00:00:00:00:00",
+      "inet": "127.0.0.1/8",
+      "inet6": "::1/128"
+    },
+    {
+      "name": "eth0",
+      "hwaddr": "bc:24:11:00:1a:09",
+      "inet": "192.0.2.150/24",
+      "inet6": "fe80::be24:11ff:fe00:1a09/64"
+    }
+  ]
+}

Tests.Discovery/ProxmoxGuestAddressTests.cs

@@ -0,0 +1,207 @@
+using RackPeek.Domain.Discovery;
+using RackPeek.Domain.Resources;
+using RackPeek.Domain.Resources.SystemResources;
+
+namespace Tests.Discovery;
+
+/// <summary>
+///     Learning where a guest actually is. Proxmox only records an address in a guest's
+///     config when someone set one statically, so on a DHCP estate every guest arrived
+///     without one — and an address is what lets a guest line up with the host a network
+///     sweep found. The guest itself knows, and will say so through its agent.
+///     <para>
+///         The hard part is that the agent reports every interface inside the machine,
+///         including the bridges Docker and Home Assistant create. Picking the wrong one
+///         would record 172.17.0.1 as the machine's address, which every container host
+///         on the estate would also report.
+///     </para>
+/// </summary>
+public class ProxmoxGuestAddressTests {
+    private const string _nicMac = "bc:24:11:00:1a:01";
+
+    private static List<ProxmoxGuestAddress> AgentAddresses() =>
+        ProxmoxResponseParser.ParseAgentInterfaces(Fixture.Read("pve-agent-interfaces.json"));
+
+    [Fact]
+    public void An_agent_reports_every_routable_ipv4_it_can_see() {
+        List<ProxmoxGuestAddress> addresses = AgentAddresses();
+
+        // Loopback and the IPv6 entries are dropped; the NIC and both docker bridges stay,
+        // because deciding between them is the caller's job, not the parser's.
+        Assert.Equal(["192.0.2.105", "172.17.0.1", "172.18.0.1"], addresses.Select(a => a.Ip));
+    }
+
+    [Fact]
+    public void The_guests_own_nic_wins_over_the_bridges_it_created() =>
+        Assert.Equal("192.0.2.105", ProxmoxDiscovery.SelectGuestIp(AgentAddresses(), [_nicMac]));
+
+    [Fact]
+    // Proxmox configs upper-case them; agents vary. Both go through the same normaliser
+    // the ARP reader uses, so a scan and this collector always agree.
+    public void The_mac_is_matched_however_either_side_spells_it() =>
+        Assert.Equal("192.0.2.105", ProxmoxDiscovery.SelectGuestIp(AgentAddresses(), ["BC-24-11-00-1A-01"]));
+
+    [Fact]
+    // Every address on offer belongs to something the guest invented. Recording one
+    // would be worse than recording nothing.
+    public void Nothing_is_claimed_when_no_interface_carries_a_configured_mac() =>
+        Assert.Null(ProxmoxDiscovery.SelectGuestIp(AgentAddresses(), ["bc:24:11:ff:ff:ff"]));
+
+    [Fact]
+    // A token that cannot read the guest config leaves no discriminator, so there is no
+    // way to tell a NIC from a bridge.
+    public void Nothing_is_claimed_when_the_configured_macs_are_unknown() =>
+        Assert.Null(ProxmoxDiscovery.SelectGuestIp(AgentAddresses(), []));
+
+    [Fact]
+    public void An_agent_that_answers_nothing_yields_nothing() {
+        Assert.Empty(ProxmoxResponseParser.ParseAgentInterfaces("""{"data":null}"""));
+        Assert.Empty(ProxmoxResponseParser.ParseAgentInterfaces("""{"data":{"result":[]}}"""));
+    }
+
+    [Fact]
+    public void A_container_reports_its_address_with_the_prefix_stripped() {
+        List<ProxmoxGuestAddress> addresses =
+            ProxmoxResponseParser.ParseContainerInterfaces(Fixture.Read("pve-lxc-interfaces.json"));
+
+        ProxmoxGuestAddress only = Assert.Single(addresses);
+        Assert.Equal("192.0.2.150", only.Ip);
+        Assert.Equal("eth0", only.Interface);
+    }
+
+    [Fact]
+    public void A_guest_that_failed_dhcp_is_not_recorded_at_its_self_assigned_address() {
+        // 169.254 means "I could not get an address", which is not an address worth
+        // writing into an inventory.
+        var json = """
+                   {"data":{"result":[{"name":"ens18","hardware-address":"bc:24:11:00:1a:01",
+                   "ip-addresses":[{"ip-address":"169.254.12.7","ip-address-type":"ipv4","prefix":16}]}]}}
+                   """;
+
+        Assert.Empty(ProxmoxResponseParser.ParseAgentInterfaces(json));
+    }
+
+    [Fact]
+    public async Task A_statically_configured_address_still_wins_over_the_agent() {
+        // The config is what the administrator asked for; the agent is what the guest
+        // happens to report. Where both exist they agree, and where they do not the
+        // configured one is the intent.
+        var client = new ScriptedProxmoxClient {
+            Guests = [Guest(100, "static-guest")],
+            Configs = { [100] = new ProxmoxGuestConfig("Linux", "192.0.2.9", [], [], [_nicMac]) },
+            Addresses = { [100] = [new ProxmoxGuestAddress("ens18", _nicMac, "192.0.2.105")] }
+        };
+
+        List<Resource> resources = await ProxmoxDiscovery.ReadAsync(client);
+
+        Assert.Equal("192.0.2.9", GuestCard(resources, "static-guest").Ip);
+    }
+
+    [Fact]
+    public async Task A_dhcp_guest_takes_the_address_its_agent_reports() {
+        var client = new ScriptedProxmoxClient {
+            Guests = [Guest(101, "dhcp-guest")],
+            Configs = { [101] = new ProxmoxGuestConfig("Linux", null, [], [], [_nicMac]) },
+            Addresses = { [101] = [new ProxmoxGuestAddress("ens18", _nicMac, "192.0.2.105")] }
+        };
+
+        List<Resource> resources = await ProxmoxDiscovery.ReadAsync(client);
+
+        Assert.Equal("192.0.2.105", GuestCard(resources, "dhcp-guest").Ip);
+    }
+
+    [Fact]
+    public async Task A_stopped_guest_is_never_asked_where_it_is() {
+        // It has no address to report, and asking costs a round trip per guest on an
+        // estate where most guests may be off.
+        var client = new ScriptedProxmoxClient {
+            Guests = [Guest(102, "stopped-guest", "stopped")],
+            Configs = { [102] = new ProxmoxGuestConfig("Linux", null, [], [], [_nicMac]) }
+        };
+
+        await ProxmoxDiscovery.ReadAsync(client);
+
+        Assert.Empty(client.AddressCalls);
+    }
+
+    [Fact]
+    public async Task The_guests_macs_reach_the_card() {
+        // The MCP tool used to run its own copy of this orchestration which dropped the
+        // MACs, silently costing every guest its chance of unifying with a scan.
+        var client = new ScriptedProxmoxClient {
+            Guests = [Guest(103, "mac-guest")],
+            Configs = { [103] = new ProxmoxGuestConfig("Linux", null, [], [], [_nicMac]) }
+        };
+
+        List<Resource> resources = await ProxmoxDiscovery.ReadAsync(client);
+
+        Assert.Equal(_nicMac, GuestCard(resources, "mac-guest").Labels["macs"]);
+    }
+
+    private static SystemResource GuestCard(List<Resource> resources, string name) =>
+        resources.OfType<SystemResource>().Single(r => r.Name == name);
+
+    private static ProxmoxGuest Guest(int vmId, string name, string status = "running") =>
+        new() {
+            VmId = vmId,
+            Node = "pve01",
+            Name = name,
+            Type = "vm",
+            Status = status
+        };
+
+    private sealed class ScriptedProxmoxClient : IProxmoxClient {
+        public List<ProxmoxGuest> Guests { get; init; } = [];
+        public Dictionary<int, ProxmoxGuestConfig> Configs { get; } = [];
+        public Dictionary<int, List<ProxmoxGuestAddress>> Addresses { get; } = [];
+        public List<int> AddressCalls { get; } = [];
+
+        public string Endpoint => "https://pve.example.com:8006";
+
+        public Task<string> GetIdentityScopeAsync(CancellationToken cancellationToken = default) =>
+            Task.FromResult("example-cluster");
+
+        public Task<IReadOnlyList<ProxmoxNode>> GetNodesAsync(CancellationToken cancellationToken = default) =>
+            Task.FromResult<IReadOnlyList<ProxmoxNode>>([new ProxmoxNode { Name = "pve01" }]);
+
+        public Task<ProxmoxNode> EnrichAsync(ProxmoxNode node, CancellationToken cancellationToken = default) =>
+            Task.FromResult(node);
+
+        public Task<IReadOnlyList<ProxmoxGuest>> GetGuestsAsync(
+            string node,
+            string endpoint,
+            CancellationToken cancellationToken = default) =>
+            Task.FromResult<IReadOnlyList<ProxmoxGuest>>(
+                endpoint == ProxmoxApiClient.QemuEndpoint ? Guests : []);
+
+        public Task<IReadOnlyList<ProxmoxDisk>> GetDisksAsync(
+            string node,
+            CancellationToken cancellationToken = default) =>
+            Task.FromResult<IReadOnlyList<ProxmoxDisk>>([]);
+
+        public Task<IReadOnlyList<ProxmoxGpu>> GetGpusAsync(
+            string node,
+            CancellationToken cancellationToken = default) =>
+            Task.FromResult<IReadOnlyList<ProxmoxGpu>>([]);
+
+        public Task<ProxmoxGuestConfig> GetGuestConfigAsync(
+            string node,
+            string endpoint,
+            int vmId,
+            CancellationToken cancellationToken = default) =>
+            Task.FromResult(Configs.TryGetValue(vmId, out ProxmoxGuestConfig? config)
+                ? config
+                : new ProxmoxGuestConfig(null, null, [], []));
+
+        public Task<IReadOnlyList<ProxmoxGuestAddress>> GetGuestAddressesAsync(
+            string node,
+            string endpoint,
+            int vmId,
+            CancellationToken cancellationToken = default) {
+            AddressCalls.Add(vmId);
+
+            return Task.FromResult<IReadOnlyList<ProxmoxGuestAddress>>(
+                Addresses.TryGetValue(vmId, out List<ProxmoxGuestAddress>? found) ? found : []);
+        }
+    }
+}

Tests.Discovery/RunsOnByIpTests.cs

@@ -79,6 +79,22 @@ public class RunsOnByIpTests {
         Assert.Equal(["some-vm"], incoming.OfType<Service>().Single().RunsOn);
     }
 
+    [Fact]
+    public void A_stored_host_beats_a_scanned_stand_in_at_the_same_address() {
+        // Once a hypervisor reports its guests' addresses, a sweep of that subnet finds
+        // the same machines again and contributes a sparse card per address. The service
+        // belongs on the guest the hypervisor described, not on the sweep's stand-in.
+        List<Resource> existing = [System("app-vm", "192.0.2.50")];
+        List<Resource> incoming = [
+            System("host-1a2b3c4d", "192.0.2.50", "rpk1:net:c"),
+            Service("immich", "192.0.2.50", "SOMEWHERE.lan")
+        ];
+
+        DiscoveryIdResolver.ResolveNames(existing, incoming);
+
+        Assert.Equal(["app-vm"], incoming.OfType<Service>().Single().RunsOn);
+    }
+
     [Fact]
     public void An_address_two_systems_claim_anchors_nothing() {
         // Overlapping subnets across sites, or a stale card nobody cleaned up. An
@@ -125,4 +141,87 @@ public class RunsOnByIpTests {
 
         Assert.Empty(incoming.OfType<SystemResource>().Single().RunsOn);
     }
+
+    // ---------------------------------------------------------------
+    // Unifying a sweep's find with the guest a hypervisor already described
+    // ---------------------------------------------------------------
+
+    private static SystemResource Scanned(string name, string ip, string? mac = null) {
+        var card = new SystemResource {
+            Kind = SystemResource.KindLabel,
+            Name = name,
+            Ip = ip,
+            DiscoveryId = DiscoveryId.Create(DiscoveryId.NetworkScheme, mac ?? $"ip:{ip}")
+        };
+
+        if (mac != null)
+            card.Labels["mac"] = mac;
+
+        return card;
+    }
+
+    private static SystemResource Guest(string name, string ip, string id = "abc123") =>
+        new() {
+            Kind = SystemResource.KindLabel,
+            Name = name,
+            Ip = ip,
+            DiscoveryId = $"rpk1:pve:{id}"
+        };
+
+    [Fact]
+    public void A_sweep_find_becomes_the_guest_the_hypervisor_already_described() {
+        // ARP is link-local, so a guest on another subnet gives the sweep no MAC and its
+        // identity falls back to the address. Now that the hypervisor reports that same
+        // address, it is the only thing tying the two records together.
+        List<Resource> existing = [Guest("app-vm", "192.0.2.105")];
+        List<Resource> incoming = [Scanned("host-1a2b3c4d", "192.0.2.105")];
+
+        DiscoveryIdResolver.ResolveNames(existing, incoming);
+
+        SystemResource card = incoming.OfType<SystemResource>().Single();
+        Assert.Equal("app-vm", card.Name);
+        // The sweep's weaker identity is dropped so the merge cannot downgrade the
+        // hypervisor's.
+        Assert.Null(card.DiscoveryId);
+    }
+
+    [Fact]
+    public void A_sweep_find_that_saw_a_mac_is_left_to_the_mac_rule() {
+        // A MAC is better evidence than an address. If it did not unify above, the two
+        // records disagree, and an address must not override that.
+        List<Resource> existing = [Guest("app-vm", "192.0.2.105")];
+        List<Resource> incoming = [Scanned("host-1a2b3c4d", "192.0.2.105", "bc:24:11:00:1a:01")];
+
+        DiscoveryIdResolver.ResolveNames(existing, incoming);
+
+        Assert.Equal("host-1a2b3c4d", incoming.OfType<SystemResource>().Single().Name);
+    }
+
+    [Fact]
+    public void Two_stored_systems_at_one_address_unify_nothing() {
+        // Overlapping subnets across sites, or a stale card. Ambiguity is not evidence.
+        List<Resource> existing = [
+            Guest("site-a-vm", "192.0.2.105", "aaa111"),
+            Guest("site-b-vm", "192.0.2.105", "bbb222")
+        ];
+        List<Resource> incoming = [Scanned("host-1a2b3c4d", "192.0.2.105")];
+
+        DiscoveryIdResolver.ResolveNames(existing, incoming);
+
+        Assert.Equal("host-1a2b3c4d", incoming.OfType<SystemResource>().Single().Name);
+    }
+
+    [Fact]
+    public void One_sweep_find_never_unifies_with_another() {
+        // A card the sweep itself produced is a stand-in for something nobody has
+        // described. Two stand-ins at one address say nothing about each other, so the
+        // address rule stays out of it — here the stored one was identified by MAC on
+        // its own segment, and the incoming one only by address.
+        List<Resource> existing = [Scanned("host-99999999", "192.0.2.105", "bc:24:11:00:1a:09")];
+        List<Resource> incoming = [Scanned("host-1a2b3c4d", "192.0.2.105")];
+
+        DiscoveryIdResolver.ResolveNames(existing, incoming);
+
+        Assert.Equal("host-1a2b3c4d", incoming.OfType<SystemResource>().Single().Name);
+    }
 }