proto-p10: BX P merge case + silent variant for probe lookups

src/proto-p10.c

@@ -388,35 +388,60 @@ GetServerN(const char *numeric)
     }
 }
 
-struct userNode*
-GetUserN(const char *numeric) /* using numeric */
+/* Shared internal numeric-to-userNode lookup.  Wraps both the noisy
+ * GetUserN (callers expect success — a miss is bug-worthy) and the
+ * quiet GetUserN_silent (callers know the target can legitimately be
+ * absent — e.g., snoop/track on transient targets, BX P probing for
+ * the alias/primary pair). */
+static struct userNode *
+GetUserN_impl(const char *numeric, int quiet)
 {
     struct userNode *un;
     struct server *s;
     int n, slen, ulen;
 
     switch (strlen(numeric)) {
     default:
-        log_module(MAIN_LOG, LOG_WARNING, "GetUserN(%s): numeric too long!", numeric);
+        if (!quiet)
+            log_module(MAIN_LOG, LOG_WARNING, "GetUserN(%s): numeric too long!", numeric);
         return NULL;
     case 5: slen = 2; ulen = 3; break;
     case 4: slen = 1; ulen = 3; break;
     case 3: slen = 1; ulen = 2; break;
     case 2: case 1: case 0:
-        log_module(MAIN_LOG, LOG_WARNING, "GetUserN(%s): numeric too short!", numeric);
+        if (!quiet)
+            log_module(MAIN_LOG, LOG_WARNING, "GetUserN(%s): numeric too short!", numeric);
         return NULL;
     }
     if (!(s = servers_num[base64toint(numeric, slen)])) {
-        log_module(MAIN_LOG, LOG_WARNING, "GetUserN(%s): couldn't find server (len=%d)!", numeric, slen);
+        if (!quiet)
+            log_module(MAIN_LOG, LOG_WARNING, "GetUserN(%s): couldn't find server (len=%d)!", numeric, slen);
         return NULL;
     }
     n = base64toint(numeric+slen, ulen) & s->num_mask;
     if (!(un = s->users[n])) {
-        log_module(MAIN_LOG, LOG_WARNING, "GetUserN(%s) couldn't find user!", numeric);
+        if (!quiet)
+            log_module(MAIN_LOG, LOG_WARNING, "GetUserN(%s) couldn't find user!", numeric);
     }
     return un;
 }
 
+struct userNode*
+GetUserN(const char *numeric) /* using numeric */
+{
+    return GetUserN_impl(numeric, 0);
+}
+
+/* Same as GetUserN but suppresses all warnings on lookup miss.  Use
+ * for callers that legitimately probe numerics which may not exist
+ * (snoop / track tracking transient PRIVMSG/NOTICE targets, BX P
+ * lookup probes during alias reconciliation, etc.). */
+struct userNode*
+GetUserN_silent(const char *numeric)
+{
+    return GetUserN_impl(numeric, 1);
+}
+
 extern struct userNode *opserv;
 static void
 check_ctcp(struct userNode *user, struct userNode *bot, char *text, UNUSED_ARG(int server_qualified))
@@ -1735,22 +1760,113 @@ static CMD_FUNC(cmd_bouncer_transfer)
         if (argc < 6)
             return 0;
 
-        old_primary = GetUserN(argv[2]);
-        new_node = GetUserN(argv[3]);
+        /* Both lookups can legitimately miss — old_primary may have
+         * already been cleaned up by a prior event, new_node is
+         * normally absent (the swap path is the common case) but
+         * may exist for the in-place-conversion / merge case below.
+         * Use the silent variant so neither probe spams the snoop
+         * channel. */
+        old_primary = GetUserN_silent(argv[2]);
+        new_node = GetUserN_silent(argv[3]);
 
         if (!old_primary)
             return 1; /* Already gone, nothing to do */
 
-        /* Numeric swap: move old_primary's numeric routing to the new
-         * server/slot.  The userNode keeps its nick, clients dict entry,
-         * handle_info, channels — only the P10 numeric changes.
+        /* Two BX P shapes need handling:
+         *
+         * 1. **Numeric swap (new_node absent)** — classic promote-or-
+         *    transfer.  Move old_primary's numeric routing to the new
+         *    server/slot.  The userNode keeps its nick, clients dict
+         *    entry, handle_info, channels — only the P10 numeric changes.
+         *    This is the case the original handler was written for, and
+         *    is what fires when nefarious never bursts an alias to us.
          *
-         * Nefarious never bursts aliases as N tokens (they're introduced
-         * via BX C only), so X3 won't have a node for the alias numeric.
-         * If new_node somehow exists (shouldn't happen), log and ignore. */
+         * 2. **In-place conversion / merge (new_node exists with same
+         *    account)** — modern fork peers emit BX P for the case where
+         *    an N-introduced client (old) is being absorbed into an
+         *    existing primary (new), e.g. burst-ordering caused us to
+         *    receive N for the would-be-alias before its BX C.  Both
+         *    nodes exist on X3.  We merge: delete old_primary's
+         *    userNode (channels and dict entry cleaned up via DelUser,
+         *    no QUIT broadcast), keep new_node intact as the surviving
+         *    identity.  Gate strictly on same-handle to avoid
+         *    accidentally merging unrelated collisions.
+         *
+         * 3. **new_node exists but different/no handle** — genuinely
+         *    unexpected.  Keep the original "log and ignore" behaviour
+         *    so we don't silently corrupt state across an account
+         *    mismatch. */
         if (new_node) {
+            if (old_primary->handle_info
+                && old_primary->handle_info == new_node->handle_info) {
+                /* Merge: old absorbed into new.  Before deleting
+                 * old_primary, transfer its channel memberships onto
+                 * new_node.  This mirrors nefarious's own both-exist
+                 * swap semantics (upstream m_bouncer_transfer.c:88-113,
+                 * fork bouncer_session.c:7070-7078): the ghost's
+                 * channels must survive on the merged identity.
+                 *
+                 * Ordering is load-bearing: this MUST run before
+                 * DelUser().  DelUser's own cleanup loop
+                 * (DelChannelUser(..., NULL, 0)) lets an unregistered
+                 * channel self-destruct when its last member leaves
+                 * (see DelChannelUser's tail, hash.c). If old_primary
+                 * were deleted first while still holding memberships
+                 * new_node lacks, any unregistered channel where
+                 * old_primary was the only member in X3's view would
+                 * be destroyed here — even though the surviving
+                 * identity (and the rest of the network) is still in
+                 * it.  Transferring first means old_primary's channel
+                 * list is already empty by the time DelUser runs, so
+                 * that loop body never executes.
+                 *
+                 * Walk old_primary->channels the same way DelUser's
+                 * loop does: always pop the last element, since
+                 * AddChannelUser/DelChannelUser mutate both the
+                 * channel's member list and the user's channel list
+                 * out from under us. */
+                while (old_primary->channels.used > 0) {
+                    struct modeNode *mn = old_primary->channels.list[old_primary->channels.used - 1];
+                    struct chanNode *chan = mn->channel;
+
+                    if (!GetUserMode(chan, new_node)) {
+                        /* AddChannelUser() only fires irc_join() when
+                         * the joining user IsLocal() (i.e. an X3
+                         * service bot) — new_node here is always a
+                         * network user, so no JOIN hits the wire.  It
+                         * unconditionally runs call_join_funcs()
+                         * though, which fires the same on-join hooks
+                         * (chanserv presence/ban/oplevel bookkeeping)
+                         * a real join would.  The numeric-swap path
+                         * above never touches channel membership at
+                         * all, so there's no existing both-exist
+                         * precedent that's hook-free; there's no
+                         * lower-level "add to channel, no hooks"
+                         * primitive to reach for instead.  Correctness
+                         * of membership state takes priority, so we
+                         * accept the hook firing here as a known
+                         * side effect rather than leaving the ghost's
+                         * channels to be silently dropped. */
+                        struct modeNode *new_mn = AddChannelUser(new_node, chan);
+                        new_mn->modes = mn->modes;
+                        new_mn->oplevel = mn->oplevel;
+                    }
+
+                    /* No announce (reason NULL), same call shape
+                     * DelUser's own loop uses: this is internal
+                     * bookkeeping, not a real part. */
+                    DelChannelUser(old_primary, chan, NULL, 0);
+                }
+
+                /* DelUser with announce=0 suppresses both QUIT and KILL
+                 * emission — this is internal cleanup, the network
+                 * isn't supposed to see the alias's identity leave. */
+                DelUser(old_primary, NULL, 0, "Bouncer transfer");
+                return 1;
+            }
             log_module(MAIN_LOG, LOG_WARNING,
-                "BX P: new_node %s already exists as %s — ignoring promote",
+                "BX P: new_node %s already exists as %s with mismatched "
+                "handle — ignoring promote",
                 argv[3], new_node->nick);
             return 1;
         }

src/proto.h

@@ -84,6 +84,7 @@ struct cManagerNode
 #ifdef WITH_PROTOCOL_P10
 struct server* GetServerN(const char *numeric);
 struct userNode* GetUserN(const char *numeric);
+struct userNode* GetUserN_silent(const char *numeric);
 #endif
 
 /* Basic protocol parsing support. */