feat(controller-peers-env-fallback): accept name= prefixes and report malformed controller entries cleanly (task 4.1)

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

.kiro/specs/controller-peers-env-fallback/tasks.md

@@ -71,7 +71,7 @@
   - _Requirements: 1.1, 1.2, 1.3, 1.4, 1.5, 1.7_
 
 - [ ] 4. xceadmin
-- [ ] 4.1 (P) Accept `name=` prefixes and report malformed entries cleanly when parsing controller candidates
+- [x] 4.1 (P) Accept `name=` prefixes and report malformed entries cleanly when parsing controller candidates
   - Write the failing tests first: a single `name=host:port`; a mixed named/bare list keeping its order; a non-numeric port raising the new controller-value error that quotes the entry; existing bare-host and list cases unchanged
   - Add the controller-value error next to the existing controller-reachability errors
   - Strip everything up to the first `=` in each entry before the existing host/port split; turn the failing port conversion into the new error

src/xceadmin/src/xceadmin/discovery.py

@@ -35,7 +35,13 @@ import sys
 
 import capnp
 
-from xcecommon.errors import ConnectionTimeoutError, NoPrimaryError, ReplyDecodeError, TooManyRedirectsError
+from xcecommon.errors import (
+    ConnectionTimeoutError,
+    ControllerValueError,
+    NoPrimaryError,
+    ReplyDecodeError,
+    TooManyRedirectsError,
+)
 from xcecommon.schema import load_schema
 
 #: "Follow at most 3 redirect hops before reporting an error" (3.2) -- up
@@ -67,17 +73,37 @@ def resolve_controllers(controller: str) -> list[tuple[str, int]]:
     Whitespace around each comma-separated entry is stripped and empty
     entries (e.g. a stray trailing comma) are dropped, so ``"a:1, b:2,"``
     parses the same as ``"a:1,b:2"``.
+
+    Each entry may carry an optional ``name=`` prefix (``"ctrl-1=host:port"``),
+    the ``xce_parse_peers()`` form ``XCE_CONTROLLER_PEERS`` uses: everything
+    up to and including the entry's first ``=`` is discarded before the
+    host/port split, so ``"a=h1:1, h2:2,c=h3"`` parses to
+    ``[("h1", 1), ("h2", 2), ("h3", 9991)]``. This applies to every caller,
+    an explicit ``--controller`` value included. No host validation is added:
+    ``"a=:5"`` parses exactly like ``":5"`` always has.
+
+    Raises :class:`ControllerValueError` for an entry whose port is not an
+    integer (``"h:x"``, ``"a=h:"``); the message quotes the entry as the user
+    wrote it, ``name=`` prefix included, so it can be found in the value.
     """
     candidates: list[tuple[str, int]] = []
     for part in controller.split(","):
         part = part.strip()
         if not part:
             continue
-        if ":" in part:
+        address = part.split("=", 1)[1] if "=" in part else part
-            host, port_str = part.rsplit(":", 1)
+        if ":" in address:
-            candidates.append((host, int(port_str)))
+            host, port_str = address.rsplit(":", 1)
+            try:
+                port = int(port_str)
+            except ValueError:
+                raise ControllerValueError(
+                    f"invalid --controller entry '{part}': port '{port_str}' is not an integer",
+                    entry=part,
+                ) from None
+            candidates.append((host, port))
         else:
-            candidates.append((part, _DEFAULT_PORT))
+            candidates.append((address, _DEFAULT_PORT))
     return candidates
 
 

src/xceadmin/src/xcecommon/errors.py

@@ -115,6 +115,22 @@ class TooManyRedirectsError(XceAdminError):
         self.hops = hops
 
 
+class ControllerValueError(XceAdminError):
+    """One comma-separated entry of a ``--controller``-shaped value
+    (``[name=]host[:port]``) is malformed, e.g. its port is not an integer.
+
+    controller-peers-env-fallback Requirement 2.8 (raised by
+    ``xceadmin.discovery.resolve_controllers``): report "invalid
+    --controller entry '<entry>'" instead of an uncaught ``ValueError``.
+    ``entry`` is the offending entry as the user wrote it (``name=``
+    included, surrounding whitespace stripped).
+    """
+
+    def __init__(self, message: str, entry: str) -> None:
+        super().__init__(message)
+        self.entry = entry
+
+
 class NoPrimaryError(XceAdminError):
     """No primary controller could be reached within the discovery
     timeout.

src/xceadmin/tests/test_discovery.py

@@ -8,7 +8,13 @@ import socket
 
 import pytest
 
-from xcecommon.errors import ConnectionTimeoutError, NoPrimaryError, ReplyDecodeError, TooManyRedirectsError
+from xcecommon.errors import (
+    ConnectionTimeoutError,
+    ControllerValueError,
+    NoPrimaryError,
+    ReplyDecodeError,
+    TooManyRedirectsError,
+)
 from xcecommon.schema import load_schema
 from xceadmin import discovery
 
@@ -261,6 +267,40 @@ def test_resolve_controllers_strips_whitespace_and_drops_empty_entries():
     ]
 
 
+def test_resolve_controllers_strips_name_prefix_from_single_entry():
+    # XCE_CONTROLLER_PEERS uses xce_parse_peers()'s "name=host:port" form;
+    # the name is the controller's Raft identity, not part of the address.
+    assert discovery.resolve_controllers("ctrl-1=somehost:1234") == [("somehost", 1234)]
+
+
+def test_resolve_controllers_mixed_named_and_bare_entries_keep_order():
+    assert discovery.resolve_controllers("a=h1:1, h2:2,c=h3") == [
+        ("h1", 1),
+        ("h2", 2),
+        ("h3", 9991),
+    ]
+
+
+def test_resolve_controllers_strips_only_up_to_the_first_equals_sign():
+    assert discovery.resolve_controllers("a=b=h:5") == [("b=h", 5)]
+
+
+def test_resolve_controllers_empty_host_after_name_passes_through_like_a_bare_entry():
+    # "a=:5" parses exactly like ":5" does today: the name= stripping adds
+    # no host validation of its own.
+    assert discovery.resolve_controllers("a=:5") == discovery.resolve_controllers(":5") == [("", 5)]
+
+
+@pytest.mark.parametrize("entry", ["a=h:x", "h:x", "ctrl-2=h:"])
+def test_resolve_controllers_non_numeric_port_raises_controller_value_error(entry):
+    with pytest.raises(ControllerValueError) as excinfo:
+        discovery.resolve_controllers(f"good:1, {entry} ,other:2")
+    # The entry is quoted exactly as the user wrote it (name= included,
+    # surrounding whitespace stripped) so it can be found in the value.
+    assert f"'{entry}'" in excinfo.value.message
+    assert excinfo.value.entry == entry
+
+
 # ---------------------------------------------------------------------------
 # follow_redirects_any(): tries each (host, port) candidate in order,
 # failing over past ones that are unreachable at the network level.