Make joining a network idempotent and shut down cleanly on every path

Running `tsunagi up` twice with the same arguments failed with "network ...
is already active", and then dropped the iroh endpoint without closing it.

A configured network is activated automatically at startup, so the second
run found it already up. `join_network` is declarative — "be a member of
this network" — so joining one that is already active now succeeds and
changes nothing. `activate_network` stays strict for callers that
specifically want to know whether an inactive network was started.

The CLI now closes the agent on the error path too, and handles SIGTERM as
well as Ctrl-C, so a service manager stopping the agent gets the same clean
shutdown an interactive user does.

Also documents the two lookups people conflate: resolving one endpoint's
address is iroh's public pkarr/DNS service and works today, which is why
`--peer <endpoint-id>` needs no address; finding who is in a network is this
project's `NetworkDiscovery` and is still static bootstrap only. Notes in
the README and the threat model that `n0` and `direct` publish this
endpoint's addresses to a public third-party service.

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

README.md

@@ -99,11 +99,11 @@ wireguard: tsunkkcp43lmdje on fd15:1d9e:fa21:f201:…/64 mtu 1100, 1/1 tunnel(s)
 
 Notes:
 
-- Only one side needs `--peer`; the link is bidirectional. Peer discovery
+- Only one side needs `--peer`; the link is bidirectional.
-  beyond this manual bootstrap is future work.
 - The default `--transport n0` uses iroh's public address lookup and relays, so
   two machines behind NAT find each other. `--transport local` keeps everything
-  on the local network.
+  on the local network. See *How peers find each other* below — it is worth
+  understanding what gets published.
 - Without `CAP_NET_ADMIN`, add `--no-tun`: the mesh, the data links and the
   WireGuard handshakes all still run and are visible in the status output, only
   traffic does not reach the operating system. That is the quickest way to
@@ -179,6 +179,34 @@ async fn main() -> Result<()> {
 reaches the internet by accident. Opt into `DirectOnly` or `N0Defaults`
 explicitly.
 
+## How peers find each other
+
+Two different lookups are involved, and only one of them is this project's:
+
+**1. Resolving one endpoint's address — iroh's, and it works today.**
+With `--transport n0` or `--transport direct`, iroh publishes a signed record
+of this endpoint's addresses, keyed by its endpoint id, to Number 0's public
+service (`dns.iroh.link`, over pkarr and DNS) and resolves other endpoints the
+same way. That is why `--peer <endpoint-id>` works with no address attached:
+iroh looks it up. None of that code is ours.
+
+**2. Finding who is in a network — ours, and it is still manual.**
+`NetworkDiscovery` maps a secret-derived `DiscoveryKey` to a set of *candidate*
+members. Two backends exist: `StaticBootstrap` (what `--peer` feeds) and an
+in-memory one for tests. The planned Mainline DHT backend, which would let
+members find each other from the network secret alone, is **not implemented**.
+So today you bootstrap by passing one peer's id; after that the mesh is
+whatever those agents reach.
+
+What this means in practice:
+
+- With `n0` or `direct`, **your endpoint id and IP addresses are published to a
+  public third-party service.** They are not secret, and the network secret is
+  never published, but an observer of that service learns that your endpoint
+  exists and where it is. `--transport local` publishes nothing.
+- A relay, when one is needed, sees the volume and timing of your traffic — not
+  its contents.
+
 ## Storage
 
 Two physically separate SQLite files, placed wherever the library's

docs/threat-model.md

@@ -41,6 +41,12 @@ Read this before relying on anything here. The protocol is in
 - **Weak secrets.** This targets high-entropy secrets. There is no PAKE, so a
   short human passphrase can be guessed offline by anyone who can reach the
   handshake. Use `NetworkSecret::generate()`.
+- **Public address publication.** With `TransportPolicy::N0Defaults` or
+  `DirectOnly`, iroh publishes a signed record of this endpoint's addresses,
+  keyed by its endpoint id, to Number 0's public pkarr/DNS service, and
+  resolves peers through it. The network secret is never published and
+  membership cannot be inferred from a single record, but the endpoint's
+  existence and its addresses become public. `LocalOnly` publishes nothing.
 - **Addresses and metadata are observable.** Anyone able to watch the network
   sees addresses, timing and volume. Discovery backends see the
   `discovery_key` and the addresses published under it, which is enough to map

src/agent/mod.rs

@@ -242,10 +242,17 @@ impl Agent {
         self.inner.events.subscribe()
     }
 
-    /// Adds a network to the persistent configuration and activates it.
+    /// Makes this agent a member of a network, activating it.
     ///
     /// The same `(name, secret)` always produces the same [`NetworkId`], on
     /// every device.
+    ///
+    /// This is declarative and therefore **idempotent**: joining a network
+    /// that is already active succeeds and changes nothing. That matters
+    /// because a configured network is activated automatically at startup, so
+    /// running the same command twice must not be an error. Use
+    /// [`Agent::activate_network`] when you specifically want to know whether
+    /// an inactive network was started.
     pub async fn join_network(
         &self,
         name: &NetworkName,
@@ -257,11 +264,17 @@ impl Agent {
             .storage
             .upsert_network(network_id, name.clone(), secret.clone(), true)
             .await?;
-        self.activate_with_keys(keys).await?;
+        match self.activate_with_keys(keys).await {
-        Ok(network_id)
+            // Already a member of exactly this network space: nothing to do.
+            Ok(()) | Err(Error::NetworkAlreadyActive(_)) => Ok(network_id),
+            Err(err) => Err(err),
+        }
     }
 
     /// Activates a configured network that is currently inactive.
+    ///
+    /// Fails with [`Error::NetworkAlreadyActive`] if it is already running.
+    /// [`Agent::join_network`] is the forgiving version.
     pub async fn activate_network(&self, network_id: NetworkId) -> Result<()> {
         let stored = self
             .inner

src/bin/tsunagi.rs

@@ -71,13 +71,17 @@ impl PathArgs {
 }
 
 /// How much external connectivity machinery the endpoint may use.
+///
+/// `direct` and `n0` publish this endpoint's addresses, keyed by its endpoint
+/// id, to Number 0's public lookup service, and resolve peers through it.
+/// That is what makes `--peer <endpoint-id>` work without an address.
 #[derive(Debug, Clone, Copy, ValueEnum)]
 enum Transport {
-    /// Loopback and the local network only. No relays, no address lookup.
+    /// Loopback and the local network only. Publishes nothing.
     Local,
     /// Public address lookup, but no relays.
     Direct,
-    /// iroh's defaults: address lookup plus the public n0 relays.
+    /// iroh's defaults: public address lookup plus the public n0 relays.
     N0,
 }
 
@@ -378,8 +382,16 @@ async fn up(args: UpArgs) -> Result<(), Box<dyn std::error::Error>> {
     };
 
     let agent = Agent::spawn(config).await?;
+    // From here on every exit goes through `agent.shutdown()`, so the endpoint
+    // is never dropped without being closed.
     let mut events = agent.subscribe();
-    let network = agent.join_network(&name, &secret).await?;
+    let network = match agent.join_network(&name, &secret).await {
+        Ok(network) => network,
+        Err(err) => {
+            agent.shutdown().await;
+            return Err(err.into());
+        }
+    };
 
     println!("tsunagi is up");
     println!("  endpoint id  {}", agent.endpoint_id());
@@ -401,11 +413,8 @@ async fn up(args: UpArgs) -> Result<(), Box<dyn std::error::Error>> {
 
     loop {
         tokio::select! {
-            signal = tokio::signal::ctrl_c() => {
+            reason = stop_signal() => {
-                if let Err(err) = signal {
+                println!("\nstopping ({reason})...");
-                    eprintln!("cannot listen for Ctrl-C: {err}");
-                }
-                println!("\nstopping...");
                 break;
             }
             event = events.recv() => match event {
@@ -431,6 +440,34 @@ async fn up(args: UpArgs) -> Result<(), Box<dyn std::error::Error>> {
     Ok(())
 }
 
+/// Resolves when the process is asked to stop.
+///
+/// Both Ctrl-C and `SIGTERM` are handled, so a service manager stopping the
+/// agent gets the same clean shutdown an interactive user does.
+async fn stop_signal() -> &'static str {
+    #[cfg(unix)]
+    {
+        use tokio::signal::unix::{SignalKind, signal};
+        let mut terminate = match signal(SignalKind::terminate()) {
+            Ok(stream) => stream,
+            Err(err) => {
+                eprintln!("cannot listen for SIGTERM: {err}");
+                let _ = tokio::signal::ctrl_c().await;
+                return "interrupted";
+            }
+        };
+        tokio::select! {
+            _ = tokio::signal::ctrl_c() => "interrupted",
+            _ = terminate.recv() => "terminated",
+        }
+    }
+    #[cfg(not(unix))]
+    {
+        let _ = tokio::signal::ctrl_c().await;
+        "interrupted"
+    }
+}
+
 #[cfg(feature = "tun-device")]
 fn system_tun_factory() -> Result<Arc<dyn TunFactory>, Box<dyn std::error::Error>> {
     use tsunagi::dataplane::wireguard::SystemTunFactory;

src/discovery.rs

@@ -8,14 +8,17 @@
 //! A discovery backend must not carry control messages between agents, must not
 //! confirm authentication and must not mutate agent state directly.
 //!
-//! Two concerns are kept apart:
+//! Two concerns are kept apart, and only the first one is this module's:
 //!
 //! * *Finding members of a network* — [`NetworkDiscovery::resolve`], keyed by
-//!   the secret-derived [`DiscoveryKey`].
+//!   the secret-derived [`DiscoveryKey`]. That is what lives here, and today
-//! * *Resolving the address of one iroh endpoint* — an
+//!   it is [`StaticBootstrap`] plus a test backend; a DHT backend is future
-//!   [`iroh::EndpointAddr`] either already carries addresses, or iroh's own
+//!   work.
-//!   address lookup service must be enabled. Dialling a bare [`EndpointId`]
+//! * *Resolving the address of one iroh endpoint* — **iroh's job, not ours**.
-//!   with neither is expected to fail.
+//!   With [`crate::config::TransportPolicy::N0Defaults`] or `DirectOnly`, iroh
+//!   publishes and resolves endpoint addresses through Number 0's public
+//!   service, so dialling a bare [`EndpointId`] works. With `LocalOnly` there
+//!   is no lookup, and a candidate must carry addresses of its own.
 //!
 //! No empty result ever proves a network is empty. It only means "nobody found
 //! yet".

tests/restart.rs

@@ -78,6 +78,42 @@ async fn a_restarted_agent_keeps_its_identity_and_reconnects() {
     drop(dir);
 }
 
+#[tokio::test]
+async fn joining_a_network_twice_is_not_an_error() {
+    let discovery = SharedMemoryDiscovery::new();
+    let (name, secret) = network("idempotent-join");
+
+    let agent = TestAgent::spawn(&discovery).await.unwrap();
+
+    // Joining is declarative: saying it twice in one run must be fine.
+    let first = agent.agent.join_network(&name, &secret).await.unwrap();
+    let again = agent.agent.join_network(&name, &secret).await.unwrap();
+    assert_eq!(first, again);
+    assert_eq!(agent.agent.list_networks().await.unwrap().len(), 1);
+
+    // Activating explicitly is the strict version and does report it.
+    assert!(matches!(
+        agent.agent.activate_network(first).await,
+        Err(Error::NetworkAlreadyActive(_))
+    ));
+
+    // And after a restart, where the network came back up on its own, the
+    // same command must still succeed. This is what running the CLI twice
+    // does.
+    let dir = agent.stop().await;
+    let restarted = Agent::spawn(config_with(dir.path(), &discovery))
+        .await
+        .unwrap();
+    assert!(restarted.is_active(first).await, "auto-start brought it up");
+    let rejoined = restarted.join_network(&name, &secret).await.unwrap();
+    assert_eq!(rejoined, first);
+    assert!(restarted.network_status(first).await.is_ok());
+
+    restarted.shutdown().await;
+    drop(restarted);
+    drop(dir);
+}
+
 #[tokio::test]
 async fn readiness_does_not_wait_for_anyone_else() {
     let discovery = SharedMemoryDiscovery::new();