From 895ca5d025fb63d9fab854ac4fffdcdd67ba6767 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Johannes=20K=C3=BChlewindt?=
 <johannes.kuehlewindt@gmail.com>
Date: Wed, 2 Jul 2025 16:36:47 +0200
Subject: [PATCH] cli: rad node connect using only NodeId

Allow the `rad node connect` command to use only a `NodeId` when connecting.

The `NodeId` is used to connect to known addresses, and will stop at the first
address that is connected to.
---
 crates/radicle-cli/src/commands/node.rs       | 49 ++++++++++++++++---
 .../radicle-cli/src/commands/node/control.rs  | 41 ++++++++++++++++
 crates/radicle-cli/tests/commands.rs          | 43 ++++++++++++++++
 3 files changed, 126 insertions(+), 7 deletions(-)

diff --git a/crates/radicle-cli/src/commands/node.rs b/crates/radicle-cli/src/commands/node.rs
index 44566c0c..db1be3fe 100644
--- a/crates/radicle-cli/src/commands/node.rs
+++ b/crates/radicle-cli/src/commands/node.rs
@@ -1,9 +1,11 @@
 use std::ffi::OsString;
 use std::path::PathBuf;
+use std::str::FromStr;
 use std::time;
 
 use anyhow::anyhow;
 
+use radicle::node::address::Store as AddressStore;
 use radicle::node::config::ConnectAddress;
 use radicle::node::routing::Store;
 use radicle::node::Handle as _;
@@ -35,7 +37,7 @@ Usage
     rad node stop [<option>...]
     rad node logs [-n <lines>]
     rad node debug [<option>...]
-    rad node connect <nid>@<addr> [<option>...]
+    rad node connect <nid>[@<addr>] [<option>...]
     rad node routing [--rid <rid>] [--nid <nid>] [--json] [<option>...]
     rad node inventory [--nid <nid>] [<option>...]
     rad node events [--timeout <secs>] [-n <count>] [<option>...]
@@ -75,9 +77,33 @@ pub struct Options {
     op: Operation,
 }
 
+/// Address used for the [`Operation::Connect`]
+pub enum Addr {
+    /// Fully-specified address of the form `<NID>@<Address>`
+    Peer(PeerAddr<NodeId, Address>),
+    /// Just the `NID`, to be used for address lookups.
+    Node(NodeId),
+}
+
+impl FromStr for Addr {
+    type Err = anyhow::Error;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        if s.contains("@") {
+            PeerAddr::from_str(s)
+                .map(Self::Peer)
+                .map_err(|e| anyhow!("expected <nid> or <nid>@<addr>: {e}"))
+        } else {
+            NodeId::from_str(s)
+                .map(Self::Node)
+                .map_err(|e| anyhow!("expected <nid> or <nid>@<addr>: {e}"))
+        }
+    }
+}
+
 pub enum Operation {
     Connect {
-        addr: PeerAddr<NodeId, Address>,
+        addr: Addr,
         timeout: time::Duration,
     },
     Config {
@@ -141,7 +167,7 @@ impl Args for Options {
         let mut nid: Option<NodeId> = None;
         let mut rid: Option<RepoId> = None;
         let mut json: bool = false;
-        let mut addr: Option<PeerAddr<NodeId, Address>> = None;
+        let mut addr: Option<Addr> = None;
         let mut lines: usize = 60;
         let mut count: usize = usize::MAX;
         let mut timeout = time::Duration::MAX;
@@ -224,7 +250,7 @@ impl Args for Options {
         let op = match op.unwrap_or_default() {
             OperationName::Connect => Operation::Connect {
                 addr: addr.ok_or_else(|| {
-                    anyhow!("an address of the form `<nid>@<host>:<port>` must be provided")
+                    anyhow!("an `<nid>` or an address of the form `<nid>@<host>:<port>` must be provided")
                 })?,
                 timeout,
             },
@@ -254,9 +280,18 @@ pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> {
     let mut node = Node::new(profile.socket());
 
     match options.op {
-        Operation::Connect { addr, timeout } => {
-            control::connect(&mut node, addr.id, addr.addr, timeout)?
-        }
+        Operation::Connect { addr, timeout } => match addr {
+            Addr::Peer(addr) => control::connect(&mut node, addr.id, addr.addr, timeout)?,
+            Addr::Node(nid) => {
+                let db = profile.database()?;
+                let addresses = db
+                    .addresses_of(&nid)?
+                    .into_iter()
+                    .map(|ka| ka.addr)
+                    .collect();
+                control::connect_many(&mut node, nid, addresses, timeout)?;
+            }
+        },
         Operation::Config { addresses } => {
             if addresses {
                 let cfg = node.config()?;
diff --git a/crates/radicle-cli/src/commands/node/control.rs b/crates/radicle-cli/src/commands/node/control.rs
index f75bd37d..453f4800 100644
--- a/crates/radicle-cli/src/commands/node/control.rs
+++ b/crates/radicle-cli/src/commands/node/control.rs
@@ -1,3 +1,4 @@
+use std::collections::HashMap;
 use std::ffi::OsString;
 use std::fs::{File, OpenOptions};
 use std::io::{BufRead, BufReader, Read, Seek, SeekFrom};
@@ -205,6 +206,46 @@ pub fn connect(
     Ok(())
 }
 
+pub fn connect_many(
+    node: &mut Node,
+    nid: NodeId,
+    addrs: Vec<Address>,
+    timeout: time::Duration,
+) -> anyhow::Result<()> {
+    let mut spinner = term::spinner("Connecting...");
+    let mut errors = HashMap::new();
+    for addr in addrs {
+        spinner.message(format!(
+            "Connecting to {}@{addr}...",
+            term::format::node(&nid)
+        ));
+        match node.connect(
+            nid,
+            addr.clone(),
+            node::ConnectOptions {
+                persistent: true,
+                timeout,
+            },
+        ) {
+            Ok(ConnectResult::Connected) => {
+                spinner.finish();
+                return Ok(());
+            }
+            Ok(ConnectResult::Disconnected { reason }) => {
+                errors.insert(addr, reason);
+            }
+            Err(err) => {
+                errors.insert(addr, err.to_string());
+            }
+        }
+    }
+    spinner.failed();
+    for (addr, err) in errors {
+        term::error(format!("Failed to connect to {addr}: {err}"));
+    }
+    Ok(())
+}
+
 pub fn status(node: &Node, profile: &Profile) -> anyhow::Result<()> {
     for warning in crate::warning::nodes_renamed(&profile.config) {
         term::warning(warning);
diff --git a/crates/radicle-cli/tests/commands.rs b/crates/radicle-cli/tests/commands.rs
index 0ad5b4ce..ee319594 100644
--- a/crates/radicle-cli/tests/commands.rs
+++ b/crates/radicle-cli/tests/commands.rs
@@ -958,6 +958,49 @@ fn rad_node_connect() {
     assert!(session.state.is_connected());
 }
 
+#[test]
+fn rad_node_connect_without_address() {
+    logger::init(log::Level::Debug);
+    let mut environment = Environment::new();
+    let mut alice = environment.node(Config::test(Alias::new("alice")));
+    let bob = environment.node(Config::test(Alias::new("bob")));
+    let working = tempfile::tempdir().unwrap();
+    let bob = bob.spawn();
+
+    alice
+        .db
+        .addresses_mut()
+        .insert(
+            &bob.id,
+            PROTOCOL_VERSION,
+            node::Features::SEED,
+            &Alias::new("bob"),
+            0,
+            &UserAgent::default(),
+            localtime::LocalTime::now().into(),
+            [node::KnownAddress::new(
+                node::Address::from(bob.addr),
+                node::address::Source::Imported,
+            )],
+        )
+        .unwrap();
+    let alice = alice.spawn();
+    alice
+        .rad(
+            "node",
+            &["connect", format!("{}", bob.id).as_str()],
+            working.path(),
+        )
+        .unwrap();
+
+    let sessions = alice.handle.sessions().unwrap();
+    let session = sessions.first().unwrap();
+
+    assert_eq!(session.nid, bob.id);
+    assert_eq!(session.addr, bob.addr.into());
+    assert!(session.state.is_connected());
+}
+
 #[test]
 fn rad_node() {
     let mut environment = Environment::new();