fix(catalog): register both duplicate keys in one pass on one host key

Two asymmetries between the `dup-id` and `dup-address` rules:

- `dup-address` keyed on the resolved logical host while `dup-id` keyed on an
  empty placeholder, so under `--host h` a host-less declaration and an explicit
  `host "h"` one — one physical subject — were reported only under the address
  code. Both rules now read the same host key.
- Suppressing the address check for a declaration already refused for a
  duplicate ID also left that declaration's address unregistered, so a third
  subject could claim it undetected — including through `st2 agent address`,
  whose gate re-runs exactly this rule. Both keys now register in one pass
  before either is reported, and one physical conflict is still one diagnostic.

agent-identity: dev3.direct.omp.v6c4mkm2
agent-persona: generalist
agent-supervisor: unavailable
agent-tool: OMP
agent-tool-version: 18.1.2
agent-runtime: OMP 18.1.2
tooling-profile: dotfiles@7534055

Author: schickling-assistant

src/validate.rs

@@ -240,9 +240,12 @@ pub(crate) fn validate_discovered(
     // hosts is legal, and equal bytes in the ID namespace never collide with it. Keyed by
     // (resolved logical host, effective address).
     let mut seen_addresses: HashMap<(String, String), PathBuf> = HashMap::new();
-    // Placeholder host for bus-id collision: catalogs carry explicit host, and an empty host still
-    // makes two unset-host same-identity specs collide (which is the real bug).
-    let collision_host = "";
+    // One host key for both namespaces, so the two rules judge the same physical subject: under
+    // `--host h` a host-less declaration and an explicit `host "h"` one resolve to one agent and
+    // collide on both keys. Without a selected host the key is the empty placeholder, which still
+    // makes two unset-host same-identity specs collide (the real bug) while keeping an explicit
+    // host's own bytes.
+    let collision_host = this_host.unwrap_or_default();
 
     for s in &d.specs {
         let rp = rel(root, &s.path);
@@ -269,13 +272,33 @@ pub(crate) fn validate_discovered(
         // legacy/legacy, explicit/explicit across different hosts, and an explicit `id` that
         // collides with another subject's still-unmigrated frozen identity.
         //
-        // DELTA-003: the structurally archived subject set joins this check once
+        // Duplicate effective address, inside one resolved logical host. A retired subject does
+        // not resolve, so it releases its address and neither claims nor collides; a suspended
+        // subject still occupies the namespace. Explicit-vs-explicit and
+        // explicit-vs-identity-fallback collisions are the same collision here, because both
+        // sides are read through `effective_address`.
+        //
+        // Both keys register in one pass, before either is reported: a declaration refused for a
+        // duplicate ID still claims its address, so the authoring gate — `refuse_address_collision`
+        // re-runs exactly this rule over a prospective catalog — cannot admit a second claim on an
+        // address an already-duplicated declaration holds. Only the first fault is reported: an
+        // unmigrated duplicate identity is one authoring fault in one place, and reporting it
+        // twice would only inflate a legacy catalog's diagnostics.
+        //
+        // DELTA-003: the structurally archived subject set joins the ID check once
         // `st2 catalog migrate-ids` lands (PR D2) — migration may freeze an archived subject's
         // legacy bytes only while they remain unique across the combined live-and-archived set,
         // and `st2 catalog unarchive` validates ID uniqueness against that prospective
         // live-and-archived set rather than the live catalog alone.
         let bid = s.effective_id(collision_host);
         let duplicate_id = seen.insert(bid.clone(), s.path.clone());
+        let address_host = s.resolved_host(collision_host).to_string();
+        let address = s.effective_address().to_string();
+        let duplicate_address = (!s.desired_state.is_retired())
+            .then(|| {
+                seen_addresses.insert((address_host.clone(), address.clone()), s.path.clone())
+            })
+            .flatten();
         if let Some(prev) = &duplicate_id {
             issues.push(Issue::error(
                 "dup-id",
@@ -287,36 +310,21 @@ pub(crate) fn validate_discovered(
                     rel(root, prev)
                 ),
             ));
-        }
-
-        // Duplicate effective address within one resolved logical host. A retired subject does not
-        // resolve, so it releases its address and neither claims nor collides; a suspended subject
-        // still occupies the namespace. Explicit-vs-explicit and explicit-vs-identity-fallback
-        // collisions are the same collision here, because both sides are read through
-        // `effective_address`. A declaration already refused for a duplicate ID says nothing
-        // further about its address: an unmigrated duplicate identity is one authoring fault in
-        // one place, and reporting it twice would only inflate a legacy catalog's diagnostics.
-        if duplicate_id.is_none() && !s.desired_state.is_retired() {
-            let address_host = s.resolved_host(this_host.unwrap_or_default()).to_string();
-            let address = s.effective_address().to_string();
-            if let Some(prev) =
-                seen_addresses.insert((address_host.clone(), address.clone()), s.path.clone())
-            {
-                issues.push(Issue::error(
-                    "dup-address",
-                    rp.clone(),
-                    ag.clone(),
-                    format!(
-                        "duplicate agent address '{address}' on host '{}' (also declared in {})",
-                        if address_host.is_empty() {
-                            "<default>"
-                        } else {
-                            &address_host
-                        },
-                        rel(root, &prev)
-                    ),
-                ));
-            }
+        } else if let Some(prev) = &duplicate_address {
+            issues.push(Issue::error(
+                "dup-address",
+                rp.clone(),
+                ag.clone(),
+                format!(
+                    "duplicate agent address '{address}' on host '{}' (also declared in {})",
+                    if address_host.is_empty() {
+                        "<default>"
+                    } else {
+                        &address_host
+                    },
+                    rel(root, prev)
+                ),
+            ));
         }
 
         if let Some(Err(error)) = &compiled {

tests/validate.rs

@@ -1302,3 +1302,65 @@ fn a_legacy_catalog_produces_byte_identical_duplicate_diagnostics() {
         r.issues
     );
 }
+
+/// Both duplicate rules read the same host key, so one physical subject declared twice — once
+/// host-less, once with an explicit `host "h"` — is a duplicate ID under `--host h`, not a
+/// conflict reported only under the address code.
+#[test]
+fn a_host_less_and_an_explicit_host_declaration_collide_as_one_duplicate_id() {
+    let c = catalog(&[
+        (
+            "h/worker/agent.kdl",
+            r#"agent "worker" { host "h"; command "x" }"#,
+        ),
+        ("worker.kdl", r#"agent "worker" { command "x" }"#),
+    ]);
+
+    let r = validate_for_host(c.path(), "h");
+    assert!(
+        r.issues.iter().any(|i| i.code == "dup-id"
+            && i.severity == Severity::Error
+            && i.message.contains("duplicate agent id 'h.worker'")),
+        "one resolved subject declared twice must collide on the id key: {:?}",
+        r.issues
+    );
+    assert!(
+        !has(&r, "dup-address", Severity::Error),
+        "one physical conflict is one diagnostic: {:?}",
+        r.issues
+    );
+}
+
+/// A declaration refused for a duplicate ID still claims its address. Otherwise a third subject
+/// could take that address undetected — and `st2 agent address` admits exactly what this rule
+/// admits, so the writer's gate would hand out a second claim on one route.
+#[test]
+fn a_declaration_refused_for_a_duplicate_id_still_claims_its_address() {
+    let c = catalog(&[
+        ("h/a/agent.kdl", r#"agent "x" { host "h"; command "x" }"#),
+        (
+            "h/b/agent.kdl",
+            r#"agent "x" { host "h"; address "ops"; command "x" }"#,
+        ),
+        (
+            "h/c/agent.kdl",
+            r#"agent "c" { host "h"; address "ops"; command "x" }"#,
+        ),
+    ]);
+
+    let r = validate(c.path());
+    assert!(
+        has(&r, "dup-id", Severity::Error),
+        "the duplicate identity must still be reported: {:?}",
+        r.issues
+    );
+    assert!(
+        r.issues.iter().any(|i| i.code == "dup-address"
+            && i.severity == Severity::Error
+            && i.message
+                .contains("duplicate agent address 'ops' on host 'h'")
+            && i.message.contains("h/b/agent.kdl")),
+        "the address a duplicated-id declaration holds must still collide: {:?}",
+        r.issues
+    );
+}