Re-running setup no longer collapses more than one services host (#891)

configure.py

@@ -179,18 +179,33 @@ def collect_answers():
     print("services and set +x, then /whois yourself - you want the host it")
     print("shows, ending in something like .users.undernet.org.")
     current_hostmasks = _current("ADMIN_HOSTMASKS")
-    default_host = (current_hostmasks[0].rsplit("@", 1)[-1]
-                    if current_hostmasks and current_hostmasks[0] else "")
+    # Shown for context only, never substituted in on a blank answer (#891)
+    # - see below. And only when it would itself pass admin_host_problem():
+    # the first entry being a wildcard pattern like "*.home.net" used to be
+    # offered anyway, so pressing Enter printed "That will not do:
+    # '*.home.net' has a '*' in it" about a value the operator never typed.
+    first_host = (current_hostmasks[0].rsplit("@", 1)[-1]
+                 if current_hostmasks and current_hostmasks[0] else "")
+    default_host = first_host if first_host and not settings_file.admin_host_problem(first_host) else ""
     suffix = f" [{default_host}]" if default_host else ""
-    admin_host = input(f"Your services host (blank to skip){suffix}: ").strip() or default_host
+    admin_host = input(f"Your services host (blank to skip){suffix}: ").strip()
     while admin_host:
         problem = settings_file.admin_host_problem(admin_host)
         if not problem:
             break
         print(f"  That will not do: {problem}.")
         admin_host = input("Your services host (blank to skip): ").strip()
     if admin_host:
+        # More than one already configured (home and phone, say) - said
+        # before replacing them, not after: a re-run that collapsed the list
+        # to just this one used to do it silently (#891).
+        if len(current_hostmasks) > 1:
+            print(f"  This replaces the {len(current_hostmasks)} services hosts "
+                 f"already configured with just this one.")
         changes["ADMIN_HOSTMASKS"] = [f"*!*@{admin_host}"]
+    # A blank answer leaves ADMIN_HOSTMASKS exactly as it is - nothing is
+    # written - rather than rewriting it from the first entry shown above,
+    # which is what silently dropped every host after the first (#891).
 
     print()
     print("Admin console password (for the DCC CHAT console - see")

docs/UPDATES-PUBLIC.md

@@ -2,6 +2,7 @@
 
 ## Unreleased
 
+- **Fixed: re-running setup with more than one services host configured (home and phone, say) and leaving the services-host question blank silently kept only the first one.** A blank answer now leaves your services hosts exactly as they are; typing a new one still replaces the list, and now says so first when it is replacing more than one.
 - **Added (mIRC console): the @DCCore window's button now lights up like a channel's.** It turns the message colour (red, by default) when there is new activity - a request, a send, a search - and the highlight colour on a failed transfer, so failures stand out. The five-minute status line and joins, parts and bans do not light it, as they would not in a channel. Needs mIRC 7 or later; update `dccore.mrc` and reload it.
 - **Fixed: pasting a batch of requests could mute you and then ban you for an hour, and the mute notice wrongly said your queue had been cleared.** Asking for a file counted toward the flood protection exactly like a search, so pasting about a dozen rows from the list - one album, the ordinary way these lists are used - could get the eleventh line muted and the twelfth banned for an hour, on any client or connection that sends a paste quickly. Asking for files is no longer rate-limited at all: the limit is your queue, as it always should have been (100 files each by default), and past it you are told once rather than once per line. Searches and the other commands are unchanged. The mute notice was also wrong in a way that caused the ban - what it drops is the bot's pending replies to you, never your queued files - so it now says that other commands are ignored for the next 30 seconds and that your queued files are safe.
 - **Fixed: pasting several lines from the list at once could be answered "Busy looking up other files - try again in a moment".** Requesting a file meant searching the library for it every single time - even for a file just sent, and even for the file next to it in the same album - and only two of those searches could run at once, so most of a pasted batch was refused outright (and "try again in a moment" is the one thing that risks tripping the flood protection). The bot now remembers where it found a file and which folders it has been finding them in, so a batch from one album costs one search instead of nine, and a request that arrives while the library is busy waits its turn instead of being turned away. What it remembers is dropped on `!rehash`, and is now also re-checked against your current folders on every use, so changing them from the dashboard - which needs no `!rehash` - takes effect for requests straight away rather than a few minutes later.

docs/UPDATES.md

@@ -4,6 +4,24 @@ All version changes, optimizations, and bug fixes made over time in the DCCore p
 
 ## 🟨 Unreleased
 
+### 🔧 Re-running setup no longer collapses more than one services host to just the first (#891)
+
+Found reviewing #811's merged change. A blank answer at the services-host prompt showed the first configured
+host as the default and, on Enter, wrote it back as the **only** entry: an operator with two hosts configured
+(home and phone, say) who re-ran `configure.py` and pressed Enter there had the second one silently drop admin
+access, to the console and the in-channel commands alike.
+
+A blank answer now writes nothing - `ADMIN_HOSTMASKS` is left exactly as it is, rather than rewritten from the
+first entry. A host actually typed still replaces the list as before, and now says so first when there was more
+than one: *"This replaces the 2 services hosts already configured with just this one."* A smaller oddity in the
+same prompt is fixed alongside it: a first entry that is itself a wildcard pattern (`*.home.net`) is no longer
+offered as the default, which used to make pressing Enter fail the validator on a value the operator never typed.
+
+`tests/test_configure.py` (3): a blank answer with two hosts configured writes nothing; a typed answer that
+replaces more than one says so, checked against the printed text; a wildcarded first entry is not offered as
+the default and prints no confusing refusal. The first fails on the old code with the exact reported shape -
+`ADMIN_HOSTMASKS` collapsed to the first entry alone.
+
 ### 🧪 The file-request exemption is executed, not just read (#897)
 
 #894 (#888) tested the file-request flood exemption two ways - `security.is_flooding()` directly, and the two

tests/test_configure.py

@@ -205,7 +205,9 @@ def fake_read_password(prompt):
         try:
             buffer = io.StringIO()
             with contextlib.redirect_stdout(buffer):
-                return configure.collect_answers()
+                result = configure.collect_answers()
+            self._last_output = buffer.getvalue()   # #891: what was printed
+            return result
         finally:
             builtins.input = real_input
 
@@ -251,6 +253,49 @@ def test_a_bad_services_host_is_reprompted_not_accepted(self):
 
         self.assertEqual(answers["ADMIN_HOSTMASKS"], ["*!*@myaccount.users.undernet.org"])
 
+    def test_a_blank_answer_does_not_collapse_multiple_existing_hosts(self):
+        """#891: a re-run with two hosts already configured (home and phone,
+        say) and a blank answer at the prompt must write NOTHING - the old
+        code rewrote ADMIN_HOSTMASKS from the first entry alone, silently
+        dropping the second."""
+        self.set_config(ADMIN_HOSTMASKS=["*!*@home.users.undernet.org",
+                                         "*!*@phone.example.net"])
+        answers, _hash = self._run_with_answers([
+            "MyBot", "", "#my-channel", "MyAdmin",
+            "",                               # blank: keep both, unchanged
+            self.tree.music, "n",
+        ])
+
+        self.assertNotIn("ADMIN_HOSTMASKS", answers)
+
+    def test_replacing_more_than_one_host_is_said_before_it_happens(self):
+        self.set_config(ADMIN_HOSTMASKS=["*!*@home.users.undernet.org",
+                                         "*!*@phone.example.net"])
+        answers, _hash = self._run_with_answers([
+            "MyBot", "", "#my-channel", "MyAdmin",
+            "newaccount.users.undernet.org",  # a real answer this time
+            self.tree.music, "n",
+        ])
+
+        self.assertEqual(answers["ADMIN_HOSTMASKS"], ["*!*@newaccount.users.undernet.org"])
+        self.assertIn("replaces the 2 services hosts", self._last_output)
+
+    def test_a_wildcard_first_entry_is_not_offered_as_the_default(self):
+        """The smaller oddity in the same report: offering an invalid value
+        as the default meant pressing Enter tried to validate a string the
+        operator never typed, and printed a confusing refusal about it."""
+        self.set_config(ADMIN_HOSTMASKS=["*!*@*.home.net"])
+        answers, _hash = self._run_with_answers([
+            "MyBot", "", "#my-channel", "MyAdmin",
+            "",                               # blank: nothing to fall back to
+            self.tree.music, "n",
+        ])
+
+        self.assertNotIn("ADMIN_HOSTMASKS", answers)
+        self.assertNotIn("will not do", self._last_output)
+        self.assertNotIn("*.home.net]", self._last_output,
+                         "the invalid pattern was offered as the default")
+
     def test_a_blank_required_field_is_reprompted_not_accepted(self):
         """A genuinely fresh install (NICKNAME still unset - DCCoreTestCase's
         own baseline sets it to "DCCore" for every OTHER test, so this one

tests/test_the_console_guide_says_what_configure_does.py

@@ -45,8 +45,21 @@ def test_a_prompt_asks_for_the_services_host_right_after_admin_nick(self):
         self.assertIn('changes["ADMIN_HOSTMASKS"]', between)
 
     def test_a_blank_answer_writes_nothing(self):
+        """#891: a blank answer used to fall back to the first existing
+        entry and rewrite ADMIN_HOSTMASKS from it - collapsing a multi-host
+        list to just that one. The prompt's raw answer must reach
+        `if admin_host:` unchanged, with no `or default_host`/similar
+        fallback between the input() call and the write it guards."""
         code = read("configure.py")
-        self.assertIn('if admin_host:\n        changes["ADMIN_HOSTMASKS"]', code)
+        prompt_at = code.index('input(f"Your services host (blank to skip){suffix}: ")')
+        line_end = code.index("\n", prompt_at)
+        prompt_line = code[prompt_at:line_end]
+
+        self.assertNotIn(" or ", prompt_line,
+                         "a blank answer falls back to something instead of staying blank")
+        write_at = code.index('changes["ADMIN_HOSTMASKS"] = [f"*!*@{admin_host}"]')
+        guard = code[line_end:write_at]
+        self.assertIn("if admin_host:", guard)
 
 
 class TheGuideSaysSo(unittest.TestCase):