فهرست منبع

Let names and identity improve as collectors learn more

A collector names a card from whatever it could see, and when it could see
nothing the name falls back to a slug of the card's own id — host-1a2b3c4d says
only that something is there. A later run, or a collector that can see more,
often does know the machine's name: a firewall knows what it handed out over
DHCP, a hypervisor knows what its guest is called. Those placeholders now give
way to real names, and everything pointing at the old name follows: stored
runsOn, stored connections, services named after their host, and the incoming
payload's own references.

A new `userNamed` flag draws the line discovery must not cross. Renaming sets
it, every rename path goes through one use case, and nothing in discovery
touches the name again. A resource with no discoveryId is user-named whatever
the flag says — nothing but a person could have written it. The upgrade is
one-way, placeholder to real, and one real name never replaces another, or two
collectors that each knew a different name would rename a box back and forth on
every run.

The address bridge is now symmetric. It fired only when the incoming card was
the scan and the stored one was agent-grade, so sweeping before running the
hypervisor left both cards behind where the other order produced one. Which
collector ran first is an accident of what someone typed and must not decide
what the inventory holds. Two scan cards can bridge as well, but only when
exactly one of them saw a MAC: a firewall's neighbour table names the interface
answering at an address where a sweep of a subnet it does not sit on only knows
something replied, and those are not two stand-ins. Cards of equal standing
never bridge.

Measured on a nine-subnet lab, sweeping first: fourteen machines were in the
inventory twice, and are now in it once.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Tim Jones 12 ساعت پیش
والد
کامیت
8900008c55

+ 10 - 2
RackPeek.Domain/Api/UpsertInventoryUseCase.cs

@@ -63,9 +63,18 @@ public class UpsertInventoryUseCase(
         List<Resource>? incomingResources = incomingRoot.Resources;
         IReadOnlyList<Resource> currentResources = await repo.GetAllOfTypeAsync<Resource>();
 
+        IReadOnlyList<Connection> currentConnections = await repo.GetConnectionsAsync();
+
         // Line discovered resources up with what they already map to before anything
         // else looks at names, so the diff below reports against the right resources.
-        DiscoveryIdResolver.ResolveNames(currentResources, incomingResources, incomingRoot.Connections);
+        // A dry run gets the same reconciliation but is not allowed to improve stored
+        // names, because that rewrites the inventory and a dry run must not.
+        DiscoveryIdResolver.ResolveNames(
+            currentResources,
+            incomingResources,
+            incomingRoot.Connections,
+            currentConnections,
+            !request.DryRun);
 
         IGrouping<string, Resource>? duplicate = incomingResources
             .GroupBy(r => r.Name, StringComparer.OrdinalIgnoreCase)
@@ -123,7 +132,6 @@ public class UpsertInventoryUseCase(
             else if (oldYaml != newYaml) response.Updated.Add(incoming.Name);
         }
 
-        IReadOnlyList<Connection> currentConnections = await repo.GetConnectionsAsync();
         List<Connection>? mergedConnections = ConnectionMerger.Merge(
             currentConnections,
             incomingRoot.Connections,

+ 192 - 23
RackPeek.Domain/Discovery/DiscoveryIdResolver.cs

@@ -21,11 +21,21 @@ public static class DiscoveryIdResolver {
     ///     the stored resources the ids point at. Also rewrites <c>runsOn</c> references
     ///     between incoming resources — and the payload's <paramref name="connections" />,
     ///     which name resources the same way — so a rename does not break the tree.
+    ///     <para>
+    ///         The one exception is <paramref name="improveStoredNames" />, which lets a
+    ///         stored placeholder nobody chose be replaced by a real name this payload
+    ///         knows — see <see cref="CanImproveName" />. That rewrites the stored side, so
+    ///         only a caller that is about to persist should ask for it, and it must hand
+    ///         over <paramref name="storedConnections" /> for the same reason the incoming
+    ///         side hands over its own.
+    ///     </para>
     /// </summary>
     public static void ResolveNames(
         IReadOnlyList<Resource> existing,
         IReadOnlyList<Resource> incoming,
-        IReadOnlyList<Connection>? connections = null) {
+        IReadOnlyList<Connection>? connections = null,
+        IReadOnlyList<Connection>? storedConnections = null,
+        bool improveStoredNames = false) {
         var incomingWithId = incoming
             .Where(r => !string.IsNullOrWhiteSpace(r.DiscoveryId))
             .ToList();
@@ -52,13 +62,37 @@ public static class DiscoveryIdResolver {
 
         var renames = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase);
 
+        var storedRenames = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase);
+
         foreach (Resource resource in incomingWithId) {
+            // Captured before resolution, which may null the id as part of unifying.
+            var offeredName = resource.Name;
+            var offeredId = resource.DiscoveryId;
+
             var resolved = ResolveName(resource, existingById, existingByName, existingByMac, existingByIp);
 
-            if (resolved.Equals(resource.Name, StringComparison.OrdinalIgnoreCase))
+            if (resolved.Equals(offeredName, StringComparison.OrdinalIgnoreCase))
                 continue;
 
-            renames[resource.Name] = resolved;
+            // The stored card is about to lend its name to this one. If that name is a
+            // placeholder nobody chose and this collector has a real one, the better name
+            // should win instead — so the card improves as more is learned about it.
+            if (improveStoredNames
+                && existingByName.TryGetValue(resolved, out Resource? stored)
+                && !existingByName.ContainsKey(offeredName)
+                && CanImproveName(stored, offeredName, offeredId)) {
+                storedRenames[stored.Name] = offeredName;
+
+                // The name index has to follow, or a later card in this same payload would
+                // resolve onto a name that no longer exists.
+                existingByName.Remove(stored.Name);
+                stored.Name = offeredName;
+                existingByName[offeredName] = stored;
+
+                continue;
+            }
+
+            renames[offeredName] = resolved;
             resource.Name = resolved;
         }
 
@@ -67,10 +101,111 @@ public static class DiscoveryIdResolver {
             RewriteConnections(connections, renames);
         }
 
+        // The stored side has its own references to fix up, and its own connections. The
+        // incoming side gets the same treatment because a payload may well be a re-push of
+        // previously exported YAML, which still names the resource the way it was stored.
+        if (storedRenames.Count > 0) {
+            RewriteRunsOn(existing, storedRenames);
+
+            // Now that the hosts answer to their new names, the services named after the
+            // old ones follow. Done here so the renames below travel together.
+            foreach ((var from, var to) in RenameServicesAfterTheirHost(existing, storedRenames, existingByName))
+                storedRenames[from] = to;
+
+            RewriteConnections(storedConnections, storedRenames);
+            RewriteRunsOn(incoming, storedRenames);
+            RewriteConnections(connections, storedRenames);
+        }
+
         PreserveStoredRunsOn(incomingWithId, incoming, existingById, existingByName);
         AnchorRunsOnByIp(existing, incoming, existingByName);
     }
 
+    /// <summary>
+    ///     Carries a service's name along when the host it runs on stops being a
+    ///     placeholder.
+    ///     <para>
+    ///         A sweep names what it finds on a port after the host it found it on, so a
+    ///         machine it could only call <c>host-1a2b3c4d</c> gets a <c>host-1a2b3c4d-ssh</c>
+    ///         beside it. When the firewall or the hypervisor later supplies the real name
+    ///         the host becomes <c>forgejo</c> and the service is left announcing a machine
+    ///         that no longer exists — the link still resolves, but the name reads as a
+    ///         leftover, which is exactly what it is.
+    ///     </para>
+    ///     <para>
+    ///         Only names this collector's own convention produced are touched: the
+    ///         service must be named for the old host and must actually run on it, must
+    ///         not be a name a person chose, and the name it would take must be free.
+    ///     </para>
+    /// </summary>
+    private static Dictionary<string, string> RenameServicesAfterTheirHost(
+        IReadOnlyList<Resource> existing,
+        Dictionary<string, string> hostRenames,
+        Dictionary<string, Resource> existingByName) {
+        var renamed = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase);
+
+        foreach (Service service in existing.OfType<Service>()) {
+            if (service.IsUserNamed())
+                continue;
+
+            foreach ((var oldHost, var newHost) in hostRenames) {
+                if (!service.Name.StartsWith($"{oldHost}-", StringComparison.OrdinalIgnoreCase))
+                    continue;
+
+                // runsOn has already been rewritten, so this is the new name by now. A
+                // service merely named like the host without running on it is a
+                // coincidence, and coincidences are not renamed.
+                if (!service.RunsOn.Contains(newHost, StringComparer.OrdinalIgnoreCase))
+                    continue;
+
+                var candidate = $"{newHost}{service.Name[oldHost.Length..]}";
+
+                if (existingByName.ContainsKey(candidate))
+                    break;
+
+                renamed[service.Name] = candidate;
+                existingByName.Remove(service.Name);
+                service.Name = candidate;
+                existingByName[candidate] = service;
+
+                break;
+            }
+        }
+
+        return renamed;
+    }
+
+    /// <summary>
+    ///     Whether a stored card's name is a placeholder that this collector can improve
+    ///     on.
+    ///     <para>
+    ///         A discovered card is named from whatever the collector could see, and when
+    ///         that was nothing it falls back to a slug of its own id — <c>host-1a2b3c4d</c>
+    ///         says only that something is there. A later run, or a collector that can see
+    ///         more, often does know the machine's name: a firewall knows what it handed
+    ///         out over DHCP, a hypervisor knows what its guest is called. Keeping the
+    ///         placeholder in that case would mean the inventory never improved.
+    ///     </para>
+    ///     <para>
+    ///         Only ever placeholder to real name, and never over a name a person chose.
+    ///         Real to real is left alone on purpose: two collectors that each know a
+    ///         different name for a machine would otherwise rename it back and forth on
+    ///         every run.
+    ///     </para>
+    /// </summary>
+    private static bool CanImproveName(Resource stored, string offeredName, string? offeredId) =>
+        !stored.IsUserNamed()
+        && IsGeneratedName(stored.Name, stored.DiscoveryId)
+        && !IsGeneratedName(offeredName, offeredId);
+
+    /// <summary>
+    ///     Whether a name is the slug-of-its-own-id form <see cref="DiscoveryNaming.Suggest" />
+    ///     falls back to when the collector had nothing better to offer.
+    /// </summary>
+    private static bool IsGeneratedName(string name, string? discoveryId) =>
+        !string.IsNullOrWhiteSpace(discoveryId)
+        && name.EndsWith($"-{DiscoveryId.ShortSuffix(discoveryId)}", StringComparison.OrdinalIgnoreCase);
+
     /// <summary>
     ///     Gives a service whose <c>runsOn</c> names nothing the host it is plainly
     ///     running on: the system at its own address.
@@ -285,14 +420,33 @@ public static class DiscoveryIdResolver {
     ///     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.
+    ///     guest the inventory 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.
+    ///         Which of the two arrived first must not matter, so this reads the same in
+    ///         both directions: one scan-grade card that produced no MAC, one agent-grade
+    ///         card, one address, same kind. The agent-grade identity always wins — it is
+    ///         dropped from the incoming card when the incoming card is the scan (so the
+    ///         merge cannot downgrade the stored one) and kept when the incoming card is
+    ///         the agent (so the merge upgrades the stored one).
+    ///     </para>
+    ///     <para>
+    ///         Two scan cards can bridge as well, but only when exactly one of them saw a
+    ///         MAC. A firewall's neighbour table gives an address <em>and</em> the NIC
+    ///         answering at it; a sweep of a subnet it does not sit on gives an address
+    ///         and nothing else. Those are not two stand-ins — one is a direct observation
+    ///         of a specific interface and the other is "something replied" — so the
+    ///         MAC-bearing card wins the identity and the address-only card folds into it.
+    ///     </para>
+    ///     <para>
+    ///         Narrow on purpose. Against an agent-grade card the scan side must have no
+    ///         MAC at all: one that has a MAC either unified through the MAC bridge
+    ///         already or genuinely disagrees, and disagreement is not evidence. Two cards
+    ///         of equal standing never bridge — both agent-grade, both scan-grade with a
+    ///         MAC, or both scan-grade without one — because an address adds nothing when
+    ///         neither side can better it. And the address must be claimed by exactly one
+    ///         stored card, which <see cref="BuildIpMap" /> guarantees: 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(
@@ -301,14 +455,6 @@ public static class DiscoveryIdResolver {
         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;
 
@@ -316,13 +462,36 @@ public static class DiscoveryIdResolver {
             || 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)
+        var incomingIsNet = DiscoveryId.Scheme(resource.DiscoveryId) == DiscoveryId.NetworkScheme;
+        var storedIsNet = DiscoveryId.Scheme(stored.DiscoveryId) == DiscoveryId.NetworkScheme;
+
+        var incomingHasMac = MacsOf(resource).Any();
+        var storedHasMac = MacsOf(stored).Any();
+
+        // Which side holds the weaker identity, and so folds into the other. Null means
+        // the two are of equal standing and the address settles nothing.
+        bool? incomingIsWeaker =
+            incomingIsNet != storedIsNet
+                // Agent grade against scan grade. The scan is the weaker one, but only
+                // when it saw no MAC of its own — one that did either unified through the
+                // MAC bridge already or disagrees with the card it would be folded into.
+                ? (incomingIsNet ? incomingHasMac : storedHasMac) ? null : incomingIsNet
+            : !incomingIsNet
+                // Two agent-grade identities. A guest and the machine-id of the OS inside
+                // it are two cards on purpose; sharing an address does not change that.
+                ? null
+                // Two scan-grade cards: a MAC beats an address, and nothing beats nothing.
+                : incomingHasMac == storedHasMac ? null : !incomingHasMac;
+
+        if (incomingIsWeaker is not { } weaker)
             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;
+        // Same as the MAC bridge: the weaker identity is dropped so the merge cannot
+        // downgrade the stronger one. Where the stronger card is the one arriving, its id
+        // survives and the merge stamps it onto the stored card instead.
+        if (weaker)
+            resource.DiscoveryId = null;
+
         unifiedName = stored.Name;
 
         return true;

+ 3 - 1
RackPeek.Domain/Persistence/Yaml/YamlResourceCollection.cs

@@ -161,7 +161,9 @@ public sealed class YamlResourceCollection(
             DiscoveryIdResolver.ResolveNames(
                 resourceCollection.Resources,
                 incomingResources,
-                incomingRoot.Connections);
+                incomingRoot.Connections,
+                resourceCollection.Connections,
+                true);
 
             List<Resource> merged = ResourceCollectionMerger.Merge(
                 resourceCollection.Resources,

+ 28 - 0
RackPeek.Domain/Resources/Resource.cs

@@ -59,6 +59,34 @@ public abstract class Resource {
     /// </summary>
     public string? DiscoveryId { get; set; }
 
+    /// <summary>
+    ///     Whether a person chose this name. Set the moment anyone renames the resource,
+    ///     and never unset.
+    ///     <para>
+    ///         A discovered resource starts out named by whatever the collector could see,
+    ///         which is often a placeholder derived from its own id. Later runs — or a
+    ///         better collector — may learn the machine's real name, and should be able to
+    ///         improve on a placeholder. They must never touch a name a person typed.
+    ///     </para>
+    ///     <para>
+    ///         Absent means "not stated". A resource with no <see cref="DiscoveryId" /> was
+    ///         entered by hand and is therefore user-named whatever this says; see
+    ///         <see cref="IsUserNamed" />.
+    ///     </para>
+    /// </summary>
+    public bool? UserNamed { get; set; }
+
+    /// <summary>
+    ///     Whether this resource's name is a person's choice and so off limits to
+    ///     discovery. True when the flag says so, and true for anything with no
+    ///     discovery id at all — nothing but a person could have written it.
+    /// </summary>
+    /// <remarks>
+    ///     A method rather than a property because everything public on a resource is
+    ///     serialised, and this is derived from what is stored rather than part of it.
+    /// </remarks>
+    public bool IsUserNamed() => UserNamed ?? string.IsNullOrWhiteSpace(DiscoveryId);
+
     public string[] Tags { get; set; } = [];
     public Dictionary<string, string> Labels { get; set; } = new();
     public string? Notes { get; set; }

+ 4 - 0
RackPeek.Domain/UseCases/RenameResourceUseCase.cs

@@ -27,6 +27,10 @@ public class RenameResourceUseCase<T>(IResourceCollection repo) : IRenameResourc
             throw new NotFoundException($"Resource '{originalName}' not found.");
 
         original.Name = newName;
+
+        // A person has now chosen this name, so discovery must stop improving on it.
+        original.UserNamed = true;
+
         await repo.UpdateAsync(original);
 
         IReadOnlyList<Resource> allResources = await repo.GetAllOfTypeAsync<Resource>();

+ 4 - 0
RackPeek.Web.Viewer/wwwroot/schemas/v4/schema.v4.json

@@ -68,6 +68,10 @@
           "description": "Stable machine-generated identity set by 'rpk discover'. Absent on hand-written resources. The leading rpk<n> is the format version, so the way the id is derived can change without old ids being mistaken for new ones.",
           "pattern": "^rpk[0-9]+:[a-z0-9]+:[0-9a-f]{16}$"
         },
+        "userNamed": {
+          "type": "boolean",
+          "description": "True when a person chose this resource's name, which discovery then never changes."
+        },
         "tags": {
           "type": "array",
           "items": {

+ 4 - 0
RackPeek.Web/wwwroot/schemas/v4/schema.v4.json

@@ -68,6 +68,10 @@
           "description": "Stable machine-generated identity set by 'rpk discover'. Absent on hand-written resources. The leading rpk<n> is the format version, so the way the id is derived can change without old ids being mistaken for new ones.",
           "pattern": "^rpk[0-9]+:[a-z0-9]+:[0-9a-f]{16}$"
         },
+        "userNamed": {
+          "type": "boolean",
+          "description": "True when a person chose this resource's name, which discovery then never changes."
+        },
         "tags": {
           "type": "array",
           "items": {

+ 171 - 0
Tests.Discovery/IpBridgeSymmetryTests.cs

@@ -0,0 +1,171 @@
+using RackPeek.Domain.Discovery;
+using RackPeek.Domain.Resources;
+using RackPeek.Domain.Resources.Servers;
+using RackPeek.Domain.Resources.SystemResources;
+
+namespace Tests.Discovery;
+
+/// <summary>
+///     The address bridge has to read the same whichever collector ran first.
+///     <para>
+///         ARP is link-local, so a sweep of a routed subnet gets an address and no MAC,
+///         and the card it writes can only be called <c>host-&lt;hash&gt;</c>. A hypervisor
+///         knows that guest by name, by specification, and by the address it holds — so
+///         the address is the one thing tying the two together. Whether the sweep or the
+///         hypervisor got there first is an accident of what the person typed, and must
+///         not decide whether the inventory ends up with one card or two.
+///     </para>
+///     <para>
+///         A live run of nine subnets found this the hard way: sweeping before running
+///         the Proxmox and firewall collectors produced fourteen machines twice over,
+///         where the same commands in the other order produced one card each.
+///     </para>
+/// </summary>
+public class IpBridgeSymmetryTests {
+    private const string _ip = "10.0.50.105";
+    private const string _mac = "bc:24:11:00:4a:01";
+
+    /// <summary>What a sweep of a routed subnet can write: an address, and nothing else.</summary>
+    private static SystemResource ScanCard(string? ip = _ip) => new() {
+        Kind = SystemResource.KindLabel,
+        Name = "host-2ed3bfd7",
+        DiscoveryId = DiscoveryId.Create(DiscoveryId.NetworkScheme, $"ip:{ip}"),
+        Ip = ip
+    };
+
+    /// <summary>What the hypervisor knows about the same box.</summary>
+    private static SystemResource GuestCard(string name = "forgejo", string? ip = _ip) => new() {
+        Kind = SystemResource.KindLabel,
+        Name = name,
+        DiscoveryId = DiscoveryId.Create(ProxmoxResourceMapper.Scheme, "vmid-105"),
+        Type = "vm",
+        Os = "Linux",
+        Ip = ip,
+        Labels = { ["macs"] = _mac }
+    };
+
+    private static void Resolve(List<Resource> existing, List<Resource> incoming) =>
+        DiscoveryIdResolver.ResolveNames(existing, incoming, null, null, true);
+
+    [Fact]
+    public void The_hypervisor_claims_the_card_a_sweep_left_behind() {
+        // Sweep ran first. This is the direction that was silently broken.
+        List<Resource> existing = [ScanCard()];
+        List<Resource> incoming = [GuestCard()];
+
+        Resolve(existing, incoming);
+
+        // One card, not two: the guest lands on the stored one.
+        Assert.Equal(existing[0].Name, incoming[0].Name);
+    }
+
+    [Fact]
+    public void The_sweep_lands_on_the_card_the_hypervisor_left_behind() {
+        // The direction that already worked, kept honest.
+        List<Resource> existing = [GuestCard()];
+        List<Resource> incoming = [ScanCard()];
+
+        Resolve(existing, incoming);
+
+        Assert.Equal("forgejo", incoming[0].Name);
+    }
+
+    [Fact]
+    public void The_agent_identity_wins_whichever_way_round_it_arrives() {
+        // Arriving second, the hypervisor's id survives so the merge can upgrade the
+        // stored card from a stand-in to a real identity.
+        List<Resource> incoming = [GuestCard()];
+        Resolve([ScanCard()], incoming);
+        Assert.StartsWith("rpk1:pve:", incoming[0].DiscoveryId);
+
+        // Arriving second, the sweep's id is dropped so the merge cannot downgrade it.
+        incoming = [ScanCard()];
+        Resolve([GuestCard()], incoming);
+        Assert.Null(incoming[0].DiscoveryId);
+    }
+
+    [Fact]
+    public void The_placeholder_name_gives_way_to_the_real_one() {
+        // The point of unifying: the machine stops being a hash. This is the upgrade
+        // that could never fire while the cards stayed separate.
+        List<Resource> existing = [ScanCard()];
+
+        Resolve(existing, [GuestCard()]);
+
+        Assert.Equal("forgejo", existing[0].Name);
+    }
+
+    [Fact]
+    public void Two_stand_ins_at_one_address_still_unify_nothing() {
+        // Neither side knows anything the other does not, so an address proves nothing.
+        List<Resource> existing = [ScanCard()];
+        SystemResource other = ScanCard();
+        other.Name = "host-9999aaaa";
+        other.DiscoveryId = DiscoveryId.Create(DiscoveryId.NetworkScheme, "ip:other");
+
+        Resolve(existing, [other]);
+
+        Assert.Equal("host-9999aaaa", other.Name);
+    }
+
+    [Fact]
+    public void Two_agent_grade_cards_at_one_address_still_unify_nothing() {
+        // A guest and the machine-id of the OS inside it are both agent-grade. They are
+        // two cards on purpose; an address must not collapse them.
+        var fromAgent = new SystemResource {
+            Kind = SystemResource.KindLabel,
+            Name = "forgejo-inside",
+            DiscoveryId = DiscoveryId.Create(DiscoveryId.SystemScheme, "machine-a"),
+            Ip = _ip
+        };
+
+        Resolve([GuestCard()], [fromAgent]);
+
+        Assert.Equal("forgejo-inside", fromAgent.Name);
+    }
+
+    [Fact]
+    public void A_sweep_that_saw_a_mac_is_left_to_the_mac_bridge() {
+        // A MAC is better evidence than an address, whichever side is holding it. A scan
+        // card with one either unified on the MAC already or genuinely disagrees.
+        SystemResource scan = ScanCard();
+        scan.Labels["mac"] = "bc:24:11:00:4a:99";
+
+        Resolve([scan], [GuestCard()]);
+
+        Assert.Equal("host-2ed3bfd7", scan.Name);
+    }
+
+    [Fact]
+    public void A_different_kind_of_thing_at_the_same_address_is_not_the_same_thing() {
+        // The box documented as hardware and the OS a sweep saw on it are separate cards
+        // on purpose — the merge replaces on a type change, so unifying would delete one.
+        var server = new Server {
+            Kind = Server.KindLabel,
+            Name = "kepler",
+            DiscoveryId = DiscoveryId.Create(ProxmoxResourceMapper.Scheme, "node-kepler")
+        };
+
+        List<Resource> existing = [ScanCard()];
+
+        Resolve(existing, [server]);
+
+        Assert.Equal("kepler", server.Name);
+        Assert.Equal("host-2ed3bfd7", existing[0].Name);
+    }
+
+    [Fact]
+    public void An_address_two_stored_cards_claim_unifies_nothing() {
+        // Overlapping subnets, or a stale card. Ambiguity is not evidence.
+        SystemResource first = ScanCard();
+        SystemResource second = ScanCard();
+        second.Name = "host-bbbbcccc";
+        second.DiscoveryId = DiscoveryId.Create(DiscoveryId.NetworkScheme, "ip:dup");
+
+        List<Resource> incoming = [GuestCard()];
+
+        Resolve([first, second], incoming);
+
+        Assert.Equal("forgejo", incoming[0].Name);
+    }
+}

+ 321 - 0
Tests.Discovery/PlaceholderNameUpgradeTests.cs

@@ -0,0 +1,321 @@
+using RackPeek.Domain.Discovery;
+using RackPeek.Domain.Resources;
+using RackPeek.Domain.Resources.Connections;
+using RackPeek.Domain.Resources.Services;
+using RackPeek.Domain.Resources.SystemResources;
+
+namespace Tests.Discovery;
+
+/// <summary>
+///     A placeholder name is a stand-in, and a later collector that knows the machine's
+///     real name should be allowed to replace it.
+///     <para>
+///         A sweep of a routed subnet can see that something answers and nothing else, so
+///         it writes host-1a2b3c4d. Run the firewall collector and that same machine has
+///         a DHCP name; ask the hypervisor and it has a guest name. Without this the
+///         inventory would keep the hash for ever and the better name would be discarded
+///         on every run.
+///     </para>
+///     <para>
+///         The line it must not cross is a name a person typed. That is what
+///         <see cref="Resource.UserNamed" /> records, and once set it is never unset.
+///     </para>
+/// </summary>
+public class PlaceholderNameUpgradeTests {
+    private const string _mac = "bc:24:11:00:3a:01";
+
+    private static string PlaceholderFor(string discoveryId) =>
+        $"host-{DiscoveryId.ShortSuffix(discoveryId)}";
+
+    /// <summary>A sweep's card: an address, a MAC, and a name that is just its own hash.</summary>
+    private static SystemResource Stored(string? name = null, bool? userNamed = null) {
+        var id = DiscoveryId.Create(DiscoveryId.NetworkScheme, _mac);
+
+        return new SystemResource {
+            Kind = SystemResource.KindLabel,
+            Name = name ?? PlaceholderFor(id),
+            DiscoveryId = id,
+            UserNamed = userNamed,
+            Ip = "192.0.2.50",
+            Labels = { ["mac"] = _mac }
+        };
+    }
+
+    /// <summary>The firewall's view of the same box: same identity, but it knows the name.</summary>
+    private static SystemResource Incoming(string name = "forgejo") => new() {
+        Kind = SystemResource.KindLabel,
+        Name = name,
+        DiscoveryId = DiscoveryId.Create(DiscoveryId.NetworkScheme, _mac),
+        Ip = "192.0.2.50",
+        Labels = { ["mac"] = _mac }
+    };
+
+    private static List<Resource> Resolve(
+        Resource stored,
+        Resource incoming,
+        IReadOnlyList<Connection>? storedConnections = null,
+        params Resource[] alsoStored) {
+        List<Resource> existing = [stored, .. alsoStored];
+
+        DiscoveryIdResolver.ResolveNames(
+            existing,
+            [incoming],
+            null,
+            storedConnections,
+            true);
+
+        return existing;
+    }
+
+    [Fact]
+    public void A_real_name_replaces_a_placeholder_nobody_chose() {
+        SystemResource stored = Stored();
+
+        Resolve(stored, Incoming());
+
+        Assert.Equal("forgejo", stored.Name);
+    }
+
+    [Fact]
+    public void A_name_a_person_typed_is_never_touched() {
+        SystemResource stored = Stored("the-blue-one", true);
+
+        Resolve(stored, Incoming());
+
+        Assert.Equal("the-blue-one", stored.Name);
+    }
+
+    [Fact]
+    public void A_placeholder_a_person_chose_to_keep_is_still_theirs() {
+        // Renaming a card back to its hash is a strange thing to do, but it is a choice,
+        // and the flag is what records that rather than the shape of the name.
+        SystemResource stored = Stored(userNamed: true);
+        var before = stored.Name;
+
+        Resolve(stored, Incoming());
+
+        Assert.Equal(before, stored.Name);
+    }
+
+    [Fact]
+    public void A_placeholder_never_replaces_a_real_name() {
+        // The reverse direction: the sweep runs after the firewall and knows less. The
+        // upgrade is one-way or the card would flip names on alternate runs.
+        SystemResource stored = Stored("forgejo");
+        var placeholder = PlaceholderFor(stored.DiscoveryId!);
+
+        Resolve(stored, Incoming(placeholder));
+
+        Assert.Equal("forgejo", stored.Name);
+    }
+
+    [Fact]
+    public void One_real_name_does_not_replace_another() {
+        // Two collectors that each know a different name for one box would otherwise
+        // rename it back and forth every run. First real name wins and stays.
+        SystemResource stored = Stored("forgejo");
+
+        Resolve(stored, Incoming("git-server"));
+
+        Assert.Equal("forgejo", stored.Name);
+    }
+
+    [Fact]
+    public void A_hand_written_resource_counts_as_user_named_without_the_flag() {
+        // Nothing but a person could have written a resource with no discovery id, so
+        // the absent flag must not be read as permission.
+        var stored = new SystemResource {
+            Kind = SystemResource.KindLabel,
+            Name = "forgejo",
+            Ip = "192.0.2.50"
+        };
+
+        Assert.True(stored.IsUserNamed());
+        Assert.False(Stored().IsUserNamed());
+    }
+
+    [Fact]
+    public void Everything_pointing_at_the_old_name_follows_it() {
+        SystemResource stored = Stored();
+        var placeholder = stored.Name;
+
+        var service = new Service {
+            Kind = Service.KindLabel,
+            Name = "forgejo-https",
+            RunsOn = [placeholder]
+        };
+
+        Resolve(stored, Incoming(), null, service);
+
+        Assert.Equal(["forgejo"], service.RunsOn);
+    }
+
+    [Fact]
+    public void Stored_connections_follow_it_too() {
+        SystemResource stored = Stored();
+        var placeholder = stored.Name;
+
+        var connection = new Connection {
+            A = new PortReference { Resource = placeholder },
+            B = new PortReference { Resource = "core-switch", PortIndex = 12 }
+        };
+
+        Resolve(stored, Incoming(), [connection]);
+
+        Assert.Equal("forgejo", connection.A.Resource);
+    }
+
+    [Fact]
+    public void A_re_push_of_exported_yaml_follows_the_rename_too() {
+        // The payload still calls the machine by the name it was stored under, because
+        // that is what was exported. Its links have to land on the upgraded card.
+        SystemResource stored = Stored();
+        var placeholder = stored.Name;
+
+        var arriving = new Service {
+            Kind = Service.KindLabel,
+            Name = "forgejo-https",
+            DiscoveryId = DiscoveryId.Create(DiscoveryId.NetworkScheme, $"{_mac}:443"),
+            RunsOn = [placeholder]
+        };
+
+        DiscoveryIdResolver.ResolveNames([stored], [Incoming(), arriving], null, null, true);
+
+        Assert.Equal("forgejo", stored.Name);
+        Assert.Equal(["forgejo"], arriving.RunsOn);
+    }
+
+    [Fact]
+    public void A_service_named_after_the_host_follows_it() {
+        // The sweep names what it finds on a port after the host it found it on, so the
+        // leftover reads host-1a2b3c4d-ssh running on forgejo until this carries it over.
+        SystemResource stored = Stored();
+        var placeholder = stored.Name;
+
+        var ssh = new Service {
+            Kind = Service.KindLabel,
+            Name = $"{placeholder}-ssh",
+            DiscoveryId = DiscoveryId.Create(DiscoveryId.NetworkScheme, $"{_mac}:22"),
+            RunsOn = [placeholder]
+        };
+
+        Resolve(stored, Incoming(), null, ssh);
+
+        Assert.Equal("forgejo-ssh", ssh.Name);
+        Assert.Equal(["forgejo"], ssh.RunsOn);
+    }
+
+    [Fact]
+    public void A_service_a_person_named_keeps_its_name() {
+        SystemResource stored = Stored();
+        var placeholder = stored.Name;
+
+        var ssh = new Service {
+            Kind = Service.KindLabel,
+            Name = $"{placeholder}-ssh",
+            DiscoveryId = DiscoveryId.Create(DiscoveryId.NetworkScheme, $"{_mac}:22"),
+            UserNamed = true,
+            RunsOn = [placeholder]
+        };
+
+        Resolve(stored, Incoming(), null, ssh);
+
+        Assert.Equal($"{placeholder}-ssh", ssh.Name);
+    }
+
+    [Fact]
+    public void A_service_that_only_looks_like_the_host_is_not_renamed() {
+        // Named for the old host but running somewhere else entirely: a coincidence,
+        // and coincidences are not evidence of anything.
+        SystemResource stored = Stored();
+        var placeholder = stored.Name;
+
+        var stray = new Service {
+            Kind = Service.KindLabel,
+            Name = $"{placeholder}-ssh",
+            DiscoveryId = DiscoveryId.Create(DiscoveryId.NetworkScheme, "elsewhere:22"),
+            RunsOn = ["some-other-box"]
+        };
+
+        Resolve(stored, Incoming(), null, stray);
+
+        Assert.Equal($"{placeholder}-ssh", stray.Name);
+    }
+
+    [Fact]
+    public void A_service_whose_new_name_is_taken_keeps_the_old_one() {
+        SystemResource stored = Stored();
+        var placeholder = stored.Name;
+
+        var ssh = new Service {
+            Kind = Service.KindLabel,
+            Name = $"{placeholder}-ssh",
+            DiscoveryId = DiscoveryId.Create(DiscoveryId.NetworkScheme, $"{_mac}:22"),
+            RunsOn = [placeholder]
+        };
+
+        var occupier = new Service {
+            Kind = Service.KindLabel,
+            Name = "forgejo-ssh",
+            DiscoveryId = DiscoveryId.Create(DiscoveryId.NetworkScheme, "other:22"),
+            RunsOn = ["some-other-box"]
+        };
+
+        Resolve(stored, Incoming(), null, ssh, occupier);
+
+        Assert.Equal($"{placeholder}-ssh", ssh.Name);
+    }
+
+    [Fact]
+    public void A_connection_to_a_renamed_service_follows_it() {
+        SystemResource stored = Stored();
+        var placeholder = stored.Name;
+
+        var ssh = new Service {
+            Kind = Service.KindLabel,
+            Name = $"{placeholder}-ssh",
+            DiscoveryId = DiscoveryId.Create(DiscoveryId.NetworkScheme, $"{_mac}:22"),
+            RunsOn = [placeholder]
+        };
+
+        var connection = new Connection {
+            A = new PortReference { Resource = $"{placeholder}-ssh" },
+            B = new PortReference { Resource = "core-switch", PortIndex = 3 }
+        };
+
+        DiscoveryIdResolver.ResolveNames(
+            [stored, ssh], [Incoming()], null, [connection], true);
+
+        Assert.Equal("forgejo-ssh", connection.A.Resource);
+    }
+
+    [Fact]
+    public void A_name_already_in_use_is_left_alone() {
+        // Renaming onto an occupied name would collide two unrelated resources in the
+        // merge, which keys on name. Keeping the placeholder is the safe outcome.
+        SystemResource stored = Stored();
+        var placeholder = stored.Name;
+
+        var other = new SystemResource {
+            Kind = SystemResource.KindLabel,
+            Name = "forgejo",
+            Ip = "192.0.2.99"
+        };
+
+        Resolve(stored, Incoming(), null, other);
+
+        Assert.Equal(placeholder, stored.Name);
+    }
+
+    [Fact]
+    public void A_dry_run_leaves_the_stored_name_where_it_was() {
+        // The upgrade rewrites the inventory, so a caller that is only reporting what
+        // would happen must not ask for it — the default is off for exactly this reason.
+        SystemResource stored = Stored();
+        var placeholder = stored.Name;
+
+        DiscoveryIdResolver.ResolveNames([stored], [Incoming()]);
+
+        Assert.Equal(placeholder, stored.Name);
+    }
+}

+ 52 - 5
Tests.Discovery/RunsOnByIpTests.cs

@@ -212,16 +212,63 @@ public class RunsOnByIpTests {
     }
 
     [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.
+    public void Two_sweep_finds_with_nothing_between_them_are_the_same_card_already() {
+        // Neither card is more than "something replied at this address", and an address
+        // is exactly what seeds their identity when no MAC was seen — so they carry the
+        // same id and the address rule never gets a say. The stored name wins, as it
+        // does for any re-run.
+        List<Resource> existing = [Scanned("host-99999999", "192.0.2.105")];
+        List<Resource> incoming = [Scanned("host-1a2b3c4d", "192.0.2.105")];
+
+        Assert.Equal(existing[0].DiscoveryId, incoming[0].DiscoveryId);
+
+        DiscoveryIdResolver.ResolveNames(existing, incoming);
+
+        Assert.Equal("host-99999999", incoming.OfType<SystemResource>().Single().Name);
+    }
+
+    [Fact]
+    public void A_sweep_find_folds_into_one_that_saw_the_mac() {
+        // Not two stand-ins: the stored card names the NIC answering at that address —
+        // a firewall's neighbour table does this for every subnet it routes — where the
+        // incoming one only knows something replied. The MAC is the better identity, so
+        // the address-only card folds into it rather than becoming a second machine.
         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);
 
+        SystemResource card = incoming.OfType<SystemResource>().Single();
+        Assert.Equal("host-99999999", card.Name);
+
+        // The weaker address-seeded identity is dropped so the merge cannot downgrade
+        // the MAC-seeded one it is landing on.
+        Assert.Null(card.DiscoveryId);
+    }
+
+    [Fact]
+    public void The_mac_wins_arriving_second_too() {
+        // Sweep the routed subnet first, ask the firewall after: same two facts, same
+        // one card. Here the MAC-seeded identity is the one that survives.
+        List<Resource> existing = [Scanned("host-1a2b3c4d", "192.0.2.105")];
+        List<Resource> incoming = [Scanned("host-99999999", "192.0.2.105", "bc:24:11:00:1a:09")];
+
+        DiscoveryIdResolver.ResolveNames(existing, incoming);
+
+        SystemResource card = incoming.OfType<SystemResource>().Single();
+        Assert.Equal("host-1a2b3c4d", card.Name);
+        Assert.NotNull(card.DiscoveryId);
+    }
+
+    [Fact]
+    public void Two_sweep_finds_that_each_saw_a_mac_never_unify() {
+        // Both name a NIC, and they name different ones. The MAC bridge has already had
+        // its say; sharing an address now is a conflict or an overlapping subnet.
+        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", "bc:24:11:00:1a:0a")];
+
+        DiscoveryIdResolver.ResolveNames(existing, incoming);
+
         Assert.Equal("host-1a2b3c4d", incoming.OfType<SystemResource>().Single().Name);
     }
 }

+ 4 - 0
schemas/v4/schema.v4.json

@@ -68,6 +68,10 @@
           "description": "Stable machine-generated identity set by 'rpk discover'. Absent on hand-written resources. The leading rpk<n> is the format version, so the way the id is derived can change without old ids being mistaken for new ones.",
           "pattern": "^rpk[0-9]+:[a-z0-9]+:[0-9a-f]{16}$"
         },
+        "userNamed": {
+          "type": "boolean",
+          "description": "True when a person chose this resource's name, which discovery then never changes."
+        },
         "tags": {
           "type": "array",
           "items": {