Persist only named/prewarmed sessions, not implicit ones

Implicit sessions (dosh host with no --session) get a random
term-<millis>-<pid> name you can never reattach to, so persisting them just
left unreattachable holder processes lingering across restarts. The server
now only uses a persistent holder for prewarmed or explicitly-named
sessions; implicit ones use the in-process shell that dies on restart and
reaps on disconnect, as before. Adds a shared
protocol::{generate,is}_implicit_session_name as the single source of truth
for the name shape, used by both client (generate) and server (detect).

Server-only behavior change; wire VERSION unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

src/bin/dosh-client.rs

@@ -37,7 +37,7 @@ use std::path::{Path, PathBuf};
 use std::process::{Command, Stdio};
 use std::sync::Arc;
 use std::sync::atomic::{AtomicU64, Ordering};
-use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH};
+use std::time::{Duration, Instant};
 use tokio::io::{AsyncReadExt, AsyncWriteExt};
 use tokio::net::{TcpListener, TcpStream, UdpSocket, UnixStream};
 use tokio::signal::unix::{SignalKind, signal};
@@ -1173,15 +1173,10 @@ fn select_session(requested: Option<&str>, _force_new: bool) -> String {
     if let Some(session) = requested {
         return session.to_string();
     }
-    unique_session_name()
-}
-
-fn unique_session_name() -> String {
-    let millis = SystemTime::now()
-        .duration_since(UNIX_EPOCH)
-        .unwrap_or_default()
-        .as_millis();
-    format!("term-{millis}-{}", std::process::id())
+    // Implicit, ephemeral session: the server recognizes this name shape
+    // (`protocol::is_implicit_session_name`) and does NOT persist it across
+    // restarts, since you can never reattach to a random name.
+    protocol::generate_implicit_session_name()
 }
 
 fn ssh_bootstrap(

src/bin/dosh-server.rs

@@ -472,6 +472,20 @@ impl ServerState {
         Ok(())
     }
 
+    /// Whether a session named `name` should be backed by a persistent holder.
+    ///
+    /// Persistence is only worthwhile for sessions a user can actually reattach
+    /// to by name: prewarmed sessions and explicitly-named ones (`--session`).
+    /// Implicit `term-<millis>-<pid>` names (from `dosh host` with no session)
+    /// are ephemeral — persisting them just leaves unreattachable holders
+    /// lingering across restarts, so they use the in-process (dies-on-restart)
+    /// shell that reaps normally on disconnect.
+    fn should_persist_session(&self, name: &str) -> bool {
+        self.config.persist_sessions
+            && (self.config.prewarm_sessions.iter().any(|s| s == name)
+                || !protocol::is_implicit_session_name(name))
+    }
+
     /// Spawn (or, in the persistent case, spawn-and-adopt) a PTY for a session.
     ///
     /// With `persist_sessions` on, the shell is launched in a detached holder
@@ -488,7 +502,7 @@ impl ServerState {
     ) -> Result<(PtyHandle, Option<StdUnixStream>, bool)> {
         let cols = cols.max(1);
         let rows = rows.max(1);
-        if self.config.persist_sessions {
+        if self.should_persist_session(name) {
             match self.spawn_persistent_pty(name, cols, rows, env) {
                 Ok(pair) => return Ok((pair.0, Some(pair.1), true)),
                 Err(err) => {
@@ -2911,6 +2925,33 @@ fn parse_env_pairs(raw: &[String]) -> Result<Vec<(String, String)>> {
 mod tests {
     use super::*;
 
+    #[test]
+    fn persists_named_and_prewarmed_but_not_implicit_sessions() {
+        let (pty_tx, _rx) = mpsc::unbounded_channel();
+        let config = ServerConfig {
+            persist_sessions: true,
+            prewarm_sessions: vec!["default".to_string()],
+            ..ServerConfig::default()
+        };
+        let state = ServerState::new(config, [0u8; 32], pty_tx);
+        assert!(state.should_persist_session("default")); // prewarmed
+        assert!(state.should_persist_session("work")); // explicitly named
+        assert!(!state.should_persist_session("term-1781470634216-76685")); // implicit
+    }
+
+    #[test]
+    fn persist_disabled_never_persists() {
+        let (pty_tx, _rx) = mpsc::unbounded_channel();
+        let config = ServerConfig {
+            persist_sessions: false,
+            prewarm_sessions: vec!["default".to_string()],
+            ..ServerConfig::default()
+        };
+        let state = ServerState::new(config, [0u8; 32], pty_tx);
+        assert!(!state.should_persist_session("default"));
+        assert!(!state.should_persist_session("work"));
+    }
+
     #[test]
     fn native_auth_rate_limiter_throttles_per_ip_and_refills() {
         let mut limiter = NativeAuthRateLimiter::new(3);

src/protocol.rs

@@ -3,6 +3,39 @@ use crate::crypto;
 use crate::native::{EnvVar, NativeAuthOk, NativeClientHello, NativeServerHello, NativeUserAuth};
 use anyhow::{Context, Result, bail};
 use serde::{Deserialize, Serialize};
+use std::time::{SystemTime, UNIX_EPOCH};
+
+/// Generate a name for an implicit, ephemeral terminal session — the kind
+/// created by `dosh host` with no `--session`. Single source of truth shared by
+/// the client (which generates it) and the server (which recognizes it via
+/// [`is_implicit_session_name`] to decide a session is NOT worth persisting).
+pub fn generate_implicit_session_name() -> String {
+    let millis = SystemTime::now()
+        .duration_since(UNIX_EPOCH)
+        .unwrap_or_default()
+        .as_millis();
+    format!("term-{millis}-{}", std::process::id())
+}
+
+/// Whether `name` is a client-generated implicit session name
+/// (`term-<digits>-<digits>`). Such sessions are ephemeral: the user can never
+/// reattach by name, so the server must not persist them across restarts.
+/// Explicitly-named (`--session work`) and prewarmed sessions are not implicit.
+pub fn is_implicit_session_name(name: &str) -> bool {
+    let Some(rest) = name.strip_prefix("term-") else {
+        return false;
+    };
+    let mut parts = rest.split('-');
+    match (parts.next(), parts.next(), parts.next()) {
+        (Some(millis), Some(pid), None) => {
+            !millis.is_empty()
+                && millis.bytes().all(|b| b.is_ascii_digit())
+                && !pid.is_empty()
+                && pid.bytes().all(|b| b.is_ascii_digit())
+        }
+        _ => false,
+    }
+}
 
 pub const MAGIC: &[u8; 4] = b"DOSH";
 // v3: added `ForwardingKind::Agent` (SSH-agent forwarding). The new variant rides
@@ -496,3 +529,41 @@ impl ReplayWindow {
         }
     }
 }
+
+#[cfg(test)]
+mod session_name_tests {
+    use super::{generate_implicit_session_name, is_implicit_session_name};
+
+    #[test]
+    fn generated_names_are_recognized_as_implicit() {
+        let name = generate_implicit_session_name();
+        assert!(is_implicit_session_name(&name), "generated: {name}");
+    }
+
+    #[test]
+    fn explicit_and_prewarm_names_are_not_implicit() {
+        for name in [
+            "default",
+            "work",
+            "logs",
+            "term",
+            "term-",
+            "term-abc-1",
+            "term-1-x",
+            "term-1",
+            "term-1-2-3",
+            "",
+        ] {
+            assert!(
+                !is_implicit_session_name(name),
+                "should not be implicit: {name:?}"
+            );
+        }
+    }
+
+    #[test]
+    fn implicit_shape_matches() {
+        assert!(is_implicit_session_name("term-1781470634216-76685"));
+        assert!(is_implicit_session_name("term-0-0"));
+    }
+}