Repository: shell-pool/shpool (rust)

Issue:
Shpool should handle internal protocol skew more gracefully
Currently when there is breakage of shpool's internal protocol, we just break. We can and should do better.

To deal with this, the basic plan is to factor libshpool/src/protocol.rs out into a seperate crate, and have that crate's version reflect protocol compatibility. Within a semver compatible version (so x.* or 0.x.*), clients and daemons should be able to talk to one another. The protocol will gain a version exchange feature, and client commands will warn the user when incompatibile version skew is detected, suggesting that the user restart their daemon.

We could also keep this all inside libshpool and maintain a seperate manually updated version field, but I like the idea of using a dedicated crate and its version number.

Steps:
- [X] Move types to a seperate crate (#97)
- [x] Move methods to Ext traits for the types from the new crate
- [ ] Implement version negotiation logic and warnings

Pull request:
fix: add version negotiation warnings
This patch adds some version negotiation logic
and appropriate warnings to shpool. Shpool remains permissive about version mismatches, only issuing
warnings so that the user may continue to perform
any operations which still function correctly, but it now provides recommendations on how to
address the issue.

The protocol version is determined by the new
shpool-protocol crate.

Fixes #88

Patch:
diff --git a/Cargo.lock b/Cargo.lock
index 614b0bd8..4a623ed7 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -42,9 +42,9 @@ dependencies = [
 
 [[package]]
 name = "anstyle"
-version = "1.0.6"
+version = "1.0.8"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "8901269c6307e8d93993578286ac0edf7f195079ffff5ebdeea6a59ffb7e36bc"
+checksum = "1bec1de6f59aedf83baf9ff929c98f2ad654b97c9510f4e70cf6f661d49fd5b1"
 
 [[package]]
 name = "anstyle-parse"
@@ -180,7 +180,7 @@ dependencies = [
  "js-sys",
  "num-traits",
  "wasm-bindgen",
- "windows-targets 0.52.4",
+ "windows-targets 0.52.6",
 ]
 
 [[package]]
@@ -196,9 +196,9 @@ dependencies = [
 
 [[package]]
 name = "clap"
-version = "4.5.9"
+version = "4.5.16"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "64acc1846d54c1fe936a78dc189c34e28d3f5afc348403f28ecf53660b9b8462"
+checksum = "ed6719fffa43d0d87e5fd8caeab59be1554fb028cd30edc88fc4369b17971019"
 dependencies = [
  "clap_builder",
  "clap_derive",
@@ -206,9 +206,9 @@ dependencies = [
 
 [[package]]
 name = "clap_builder"
-version = "4.5.9"
+version = "4.5.15"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "6fb8393d67ba2e7bfaf28a23458e4e2b543cc73a99595511eb207fdb8aede942"
+checksum = "216aec2b177652e3846684cbfe25c9964d18ec45234f0f5da5157b207ed1aab6"
 dependencies = [
  "anstream",
  "anstyle",
@@ -218,9 +218,9 @@ dependencies = [
 
 [[package]]
 name = "clap_derive"
-version = "4.5.8"
+version = "4.5.13"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "2bac35c6dafb060fd4d275d9a4ffae97917c13a6327903a8be2153cd964f7085"
+checksum = "501d359d5f3dcaf6ecdeee48833ae73ec6e42723a1e52419c79abf9507eec0a0"
 dependencies = [
  "heck",
  "proc-macro2",
@@ -489,7 +489,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "0c2a198fb6b0eada2a8df47933734e6d35d350665a33a3593d7164fa52c75c19"
 dependencies = [
  "cfg-if",
- "windows-targets 0.52.4",
+ "windows-targets 0.48.5",
 ]
 
 [[package]]
@@ -763,9 +763,9 @@ dependencies = [
 
 [[package]]
 name = "regex"
-version = "1.10.5"
+version = "1.10.6"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "b91213439dad192326a0d7c6ee3955910425f441d7038e0d6933b0aec5c4517f"
+checksum = "4219d74c6b67a3654a9fbebc4b419e22126d13d2f3c4a07ee0cb61ff79a79619"
 dependencies = [
  "aho-corasick",
  "memchr",
@@ -926,7 +926,7 @@ dependencies = [
 
 [[package]]
 name = "shpool-protocol"
-version = "0.1.0"
+version = "0.2.0"
 dependencies = [
  "anyhow",
  "serde",
@@ -1020,14 +1020,15 @@ dependencies = [
 
 [[package]]
 name = "tempfile"
-version = "3.10.1"
+version = "3.12.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "85b77fafb263dd9d05cbeac119526425676db3784113aa9295c88498cbf8bff1"
+checksum = "04cbcdd0c794ebb0d4cf35e88edd2f7d2c4c3e9a5a6dab322839b321c6a87a64"
 dependencies = [
  "cfg-if",
  "fastrand",
+ "once_cell",
  "rustix",
- "windows-sys 0.52.0",
+ "windows-sys 0.59.0",
 ]
 
 [[package]]
@@ -1325,7 +1326,7 @@ version = "0.52.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "33ab640c8d7e35bf8ba19b884ba838ceb4fba93a4e8c65a9059d08afcfc683d9"
 dependencies = [
- "windows-targets 0.52.4",
+ "windows-targets 0.52.6",
 ]
 
 [[package]]
@@ -1343,7 +1344,16 @@ version = "0.52.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "282be5f36a8ce781fad8c8ae18fa3f9beff57ec1b52cb3de0789201425d9a33d"
 dependencies = [
- "windows-targets 0.52.4",
+ "windows-targets 0.52.6",
+]
+
+[[package]]
+name = "windows-sys"
+version = "0.59.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "1e38bc4d79ed67fd075bcc251a1c39b32a1776bbe92e5bef1f0bf1f8c531853b"
+dependencies = [
+ "windows-targets 0.52.6",
 ]
 
 [[package]]
@@ -1363,17 +1373,18 @@ dependencies = [
 
 [[package]]
 name = "windows-targets"
-version = "0.52.4"
+version = "0.52.6"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "7dd37b7e5ab9018759f893a1952c9420d060016fc19a472b4bb20d1bdd694d1b"
+checksum = "9b724f72796e036ab90c1021d4780d4d3d648aca59e491e6b98e725b84e99973"
 dependencies = [
- "windows_aarch64_gnullvm 0.52.4",
- "windows_aarch64_msvc 0.52.4",
- "windows_i686_gnu 0.52.4",
- "windows_i686_msvc 0.52.4",
- "windows_x86_64_gnu 0.52.4",
- "windows_x86_64_gnullvm 0.52.4",
- "windows_x86_64_msvc 0.52.4",
+ "windows_aarch64_gnullvm 0.52.6",
+ "windows_aarch64_msvc 0.52.6",
+ "windows_i686_gnu 0.52.6",
+ "windows_i686_gnullvm",
+ "windows_i686_msvc 0.52.6",
+ "windows_x86_64_gnu 0.52.6",
+ "windows_x86_64_gnullvm 0.52.6",
+ "windows_x86_64_msvc 0.52.6",
 ]
 
 [[package]]
@@ -1384,9 +1395,9 @@ checksum = "2b38e32f0abccf9987a4e3079dfb67dcd799fb61361e53e2882c3cbaf0d905d8"
 
 [[package]]
 name = "windows_aarch64_gnullvm"
-version = "0.52.4"
+version = "0.52.6"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "bcf46cf4c365c6f2d1cc93ce535f2c8b244591df96ceee75d8e83deb70a9cac9"
+checksum = "32a4622180e7a0ec044bb555404c800bc9fd9ec262ec147edd5989ccd0c02cd3"
 
 [[package]]
 name = "windows_aarch64_msvc"
@@ -1396,9 +1407,9 @@ checksum = "dc35310971f3b2dbbf3f0690a219f40e2d9afcf64f9ab7cc1be722937c26b4bc"
 
 [[package]]
 name = "windows_aarch64_msvc"
-version = "0.52.4"
+version = "0.52.6"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "da9f259dd3bcf6990b55bffd094c4f7235817ba4ceebde8e6d11cd0c5633b675"
+checksum = "09ec2a7bb152e2252b53fa7803150007879548bc709c039df7627cabbd05d469"
 
 [[package]]
 name = "windows_i686_gnu"
@@ -1408,9 +1419,15 @@ checksum = "a75915e7def60c94dcef72200b9a8e58e5091744960da64ec734a6c6e9b3743e"
 
 [[package]]
 name = "windows_i686_gnu"
-version = "0.52.4"
+version = "0.52.6"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "8e9b5ad5ab802e97eb8e295ac6720e509ee4c243f69d781394014ebfe8bbfa0b"
+
+[[package]]
+name = "windows_i686_gnullvm"
+version = "0.52.6"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "b474d8268f99e0995f25b9f095bc7434632601028cf86590aea5c8a5cb7801d3"
+checksum = "0eee52d38c090b3caa76c563b86c3a4bd71ef1a819287c19d586d7334ae8ed66"
 
 [[package]]
 name = "windows_i686_msvc"
@@ -1420,9 +1437,9 @@ checksum = "8f55c233f70c4b27f66c523580f78f1004e8b5a8b659e05a4eb49d4166cca406"
 
 [[package]]
 name = "windows_i686_msvc"
-version = "0.52.4"
+version = "0.52.6"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "1515e9a29e5bed743cb4415a9ecf5dfca648ce85ee42e15873c3cd8610ff8e02"
+checksum = "240948bc05c5e7c6dabba28bf89d89ffce3e303022809e73deaefe4f6ec56c66"
 
 [[package]]
 name = "windows_x86_64_gnu"
@@ -1432,9 +1449,9 @@ checksum = "53d40abd2583d23e4718fddf1ebec84dbff8381c07cae67ff7768bbf19c6718e"
 
 [[package]]
 name = "windows_x86_64_gnu"
-version = "0.52.4"
+version = "0.52.6"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "5eee091590e89cc02ad514ffe3ead9eb6b660aedca2183455434b93546371a03"
+checksum = "147a5c80aabfbf0c7d901cb5895d1de30ef2907eb21fbbab29ca94c5b08b1a78"
 
 [[package]]
 name = "windows_x86_64_gnullvm"
@@ -1444,9 +1461,9 @@ checksum = "0b7b52767868a23d5bab768e390dc5f5c55825b6d30b86c844ff2dc7414044cc"
 
 [[package]]
 name = "windows_x86_64_gnullvm"
-version = "0.52.4"
+version = "0.52.6"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "77ca79f2451b49fa9e2af39f0747fe999fcda4f5e241b2898624dca97a1f2177"
+checksum = "24d5b23dc417412679681396f2b49f3de8c1473deb516bd34410872eff51ed0d"
 
 [[package]]
 name = "windows_x86_64_msvc"
@@ -1456,9 +1473,9 @@ checksum = "ed94fce61571a4006852b7389a063ab983c02eb1bb37b47f8272ce92d06d9538"
 
 [[package]]
 name = "windows_x86_64_msvc"
-version = "0.52.4"
+version = "0.52.6"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "32b752e52a2da0ddfbdbcc6fceadfeede4c939ed16d13e648833a61dfb611ed8"
+checksum = "589f6da84c646204747d1270a2a5661ea66ed1cced2631d546fdfb155959f9ec"
 
 [[package]]
 name = "winnow"
diff --git a/libshpool/Cargo.toml b/libshpool/Cargo.toml
index 7cf86838..dece9e21 100644
--- a/libshpool/Cargo.toml
+++ b/libshpool/Cargo.toml
@@ -43,7 +43,7 @@ strip-ansi-escapes = "0.2.0" # cleaning up strings for pager display
 notify = "6" # watch config file for updates
 libproc = "0.14.8" # sniffing shells by examining the subprocess
 daemonize = "0.5" # autodaemonization
-shpool-protocol = { version = "0.1.0", path = "../shpool-protocol" } # client-server protocol
+shpool-protocol = { version = "0.2.0", path = "../shpool-protocol" } # client-server protocol
 
 # rusty wrapper for unix apis
 [dependencies.nix]
diff --git a/libshpool/src/attach.rs b/libshpool/src/attach.rs
index 5d285d3c..9ce58563 100644
--- a/libshpool/src/attach.rs
+++ b/libshpool/src/attach.rs
@@ -22,7 +22,7 @@ use shpool_protocol::{
 };
 use tracing::{error, info, warn};
 
-use super::{config, duration, protocol, test_hooks, tty::TtySizeExt as _};
+use super::{config, duration, protocol, protocol::ClientResult, test_hooks, tty::TtySizeExt as _};
 
 const MAX_FORCE_RETRIES: usize = 20;
 
@@ -188,7 +188,18 @@ fn do_attach(
 
 fn dial_client(socket: &PathBuf) -> anyhow::Result<protocol::Client> {
     match protocol::Client::new(socket) {
-        Ok(c) => Ok(c),
+        Ok(ClientResult::JustClient(c)) => Ok(c),
+        Ok(ClientResult::VersionMismatch { warning, client }) => {
+            eprintln!("warning: {}, try restarting your daemon", warning);
+            eprintln!("hit enter to continue anyway or ^C to exit");
+
+            let _ = io::stdin()
+                .lines()
+                .next()
+                .context("waiting for a continue through a version mismatch")?;
+
+            Ok(client)
+        }
         Err(err) => {
             let io_err = err.downcast::<io::Error>()?;
             if io_err.kind() == io::ErrorKind::NotFound {
@@ -239,7 +250,13 @@ impl SignalHandler {
 
     fn handle_sigwinch(&self) -> anyhow::Result<()> {
         info!("handle_sigwinch: enter");
-        let mut client = protocol::Client::new(&self.socket)?;
+        let mut client = match protocol::Client::new(&self.socket)? {
+            ClientResult::JustClient(c) => c,
+            // At this point, we've already warned the user and they
+            // chose to continue anyway, so we shouldn't bother them
+            // again.
+            ClientResult::VersionMismatch { client, .. } => client,
+        };
 
         let tty_size = TtySize::from_fd(0).context("getting tty size")?;
         info!("handle_sigwinch: tty_size={:?}", tty_size);
diff --git a/libshpool/src/daemon/server.rs b/libshpool/src/daemon/server.rs
index 8675f25e..bb71c254 100644
--- a/libshpool/src/daemon/server.rs
+++ b/libshpool/src/daemon/server.rs
@@ -35,6 +35,7 @@ use shpool_protocol::{
     AttachHeader, AttachReplyHeader, AttachStatus, ConnectHeader, DetachReply, DetachRequest,
     KillReply, KillRequest, ListReply, ResizeReply, Session, SessionMessageDetachReply,
     SessionMessageReply, SessionMessageRequest, SessionMessageRequestPayload, SessionStatus,
+    VersionHeader,
 };
 use tracing::{error, info, instrument, span, trace, warn, Level};
 
@@ -139,6 +140,22 @@ impl Server {
             .set_read_timeout(Some(consts::SOCK_STREAM_TIMEOUT))
             .context("setting read timout on inbound session")?;
 
+        // advertize our protocol version to the client so that it can
+        // warn about mismatches
+        protocol::encode_to(
+            &VersionHeader {
+                // We allow fake version to be injected for ease of testing.
+                // Otherwise we would have to resort to some heinous build
+                // contortions.
+                version: match env::var("SHPOOL_TEST__OVERRIDE_VERSION") {
+                    Ok(fake_version) => fake_version,
+                    Err(_) => String::from(shpool_protocol::VERSION),
+                },
+            },
+            &mut stream,
+        )
+        .context("writing version header")?;
+
         let header = parse_connect_header(&mut stream).context("parsing connect header")?;
 
         if let Err(err) = check_peer(&stream) {
diff --git a/libshpool/src/detach.rs b/libshpool/src/detach.rs
index 7bff257e..5b1d4039 100644
--- a/libshpool/src/detach.rs
+++ b/libshpool/src/detach.rs
@@ -17,14 +17,18 @@ use std::{io, path::Path};
 use anyhow::{anyhow, Context};
 use shpool_protocol::{ConnectHeader, DetachReply, DetachRequest};
 
-use crate::{common, protocol};
+use crate::{common, protocol, protocol::ClientResult};
 
 pub fn run<P>(mut sessions: Vec<String>, socket: P) -> anyhow::Result<()>
 where
     P: AsRef<Path>,
 {
     let mut client = match protocol::Client::new(socket) {
-        Ok(c) => c,
+        Ok(ClientResult::JustClient(c)) => c,
+        Ok(ClientResult::VersionMismatch { warning, client }) => {
+            eprintln!("warning: {}, try restarting your daemon", warning);
+            client
+        }
         Err(err) => {
             let io_err = err.downcast::<io::Error>()?;
             if io_err.kind() == io::ErrorKind::NotFound {
diff --git a/libshpool/src/kill.rs b/libshpool/src/kill.rs
index c9281f7c..379ddb86 100644
--- a/libshpool/src/kill.rs
+++ b/libshpool/src/kill.rs
@@ -17,14 +17,18 @@ use std::{io, path::Path};
 use anyhow::{anyhow, Context};
 use shpool_protocol::{ConnectHeader, KillReply, KillRequest};
 
-use crate::{common, protocol};
+use crate::{common, protocol, protocol::ClientResult};
 
 pub fn run<P>(mut sessions: Vec<String>, socket: P) -> anyhow::Result<()>
 where
     P: AsRef<Path>,
 {
     let mut client = match protocol::Client::new(socket) {
-        Ok(c) => c,
+        Ok(ClientResult::JustClient(c)) => c,
+        Ok(ClientResult::VersionMismatch { warning, client }) => {
+            eprintln!("warning: {}, try restarting your daemon", warning);
+            client
+        }
         Err(err) => {
             let io_err = err.downcast::<io::Error>()?;
             if io_err.kind() == io::ErrorKind::NotFound {
diff --git a/libshpool/src/list.rs b/libshpool/src/list.rs
index a1873e6e..411d9053 100644
--- a/libshpool/src/list.rs
+++ b/libshpool/src/list.rs
@@ -17,11 +17,15 @@ use std::{io, path::PathBuf, time};
 use anyhow::Context;
 use shpool_protocol::{ConnectHeader, ListReply};
 
-use crate::protocol;
+use crate::{protocol, protocol::ClientResult};
 
 pub fn run(socket: PathBuf) -> anyhow::Result<()> {
     let mut client = match protocol::Client::new(socket) {
-        Ok(c) => c,
+        Ok(ClientResult::JustClient(c)) => c,
+        Ok(ClientResult::VersionMismatch { warning, client }) => {
+            eprintln!("warning: {}, try restarting your daemon", warning);
+            client
+        }
         Err(err) => {
             let io_err = err.downcast::<io::Error>()?;
             if io_err.kind() == io::ErrorKind::NotFound {
diff --git a/libshpool/src/protocol.rs b/libshpool/src/protocol.rs
index 7cbb61d6..47ba7701 100644
--- a/libshpool/src/protocol.rs
+++ b/libshpool/src/protocol.rs
@@ -13,6 +13,7 @@
 // limitations under the License.
 
 use std::{
+    cmp,
     io::{self, Read, Write},
     os::unix::net::UnixStream,
     path::Path,
@@ -23,8 +24,8 @@ use std::{
 use anyhow::{anyhow, Context};
 use byteorder::{LittleEndian, ReadBytesExt as _, WriteBytesExt as _};
 use serde::{Deserialize, Serialize};
-use shpool_protocol::{Chunk, ChunkKind, ConnectHeader};
-use tracing::{debug, error, instrument, span, trace, warn, Level};
+use shpool_protocol::{Chunk, ChunkKind, ConnectHeader, VersionHeader};
+use tracing::{debug, error, info, instrument, span, trace, warn, Level};
 
 use super::{consts, tty};
 
@@ -120,19 +121,70 @@ impl<'data> ChunkExt<'data> for Chunk<'data> {
 }
 
 pub struct Client {
-    pub stream: UnixStream,
+    stream: UnixStream,
+}
+
+/// The result of creating a client, possibly with
+/// flagging some issues that need to be handled.
+pub enum ClientResult {
+    /// The created client, ready to go.
+    JustClient(Client),
+    /// There was a version mismatch between the client
+    /// process and the daemon process which ought to be
+    /// handled, though it is possible that some operations
+    /// will continue to work.
+    VersionMismatch {
+        /// A warning about a version mismatch that should be
+        /// displayed to the user.
+        warning: String,
+        /// The client, which may or may not work.
+        client: Client,
+    },
 }
 
 impl Client {
-    pub fn new<P: AsRef<Path>>(sock: P) -> anyhow::Result<Self> {
+    /// Create a new client
+    #[allow(clippy::new_ret_no_self)]
+    pub fn new<P: AsRef<Path>>(sock: P) -> anyhow::Result<ClientResult> {
         let stream = UnixStream::connect(sock).context("connecting to shpool")?;
-        Ok(Client { stream })
-    }
 
-    pub fn write_connect_header(&mut self, header: ConnectHeader) -> anyhow::Result<()> {
-        let serialize_stream = self.stream.try_clone().context("cloning stream for reply")?;
-        encode_to(&header, serialize_stream).context("writing reply")?;
+        let daemon_version: VersionHeader = match decode_from(&stream) {
+            Ok(v) => v,
+            Err(e) => {
+                warn!("error parsing VersionHeader: {:?}", e);
+                return Ok(ClientResult::VersionMismatch {
+                    warning: String::from("could not get daemon version"),
+                    client: Client { stream },
+                });
+            }
+        };
+        info!("read daemon version header: {:?}", daemon_version);
+
+        match Self::version_ord(shpool_protocol::VERSION, &daemon_version.version)
+            .context("comparing versions")?
+        {
+            cmp::Ordering::Equal => Ok(ClientResult::JustClient(Client { stream })),
+            cmp::Ordering::Less => Ok(ClientResult::VersionMismatch {
+                warning: format!(
+                    "client protocol (version {:?}) is older than daemon protocol (version {:?})",
+                    shpool_protocol::VERSION,
+                    daemon_version.version,
+                ),
+                client: Client { stream },
+            }),
+            cmp::Ordering::Greater => Ok(ClientResult::VersionMismatch {
+                warning: format!(
+                    "client protocol ({:?}) is newer than daemon protocol (version {:?})",
+                    shpool_protocol::VERSION,
+                    daemon_version.version,
+                ),
+                client: Client { stream },
+            }),
+        }
+    }
 
+    pub fn write_connect_header(&self, header: ConnectHeader) -> anyhow::Result<()> {
+        encode_to(&header, &self.stream).context("writing reply")?;
         Ok(())
     }
 
@@ -144,6 +196,43 @@ impl Client {
         Ok(reply)
     }
 
+    /// This is essentially just PartialOrd on client version strings
+    /// with more descriptive errors (since PartialOrd gives an option)
+    /// and without having to wrap in a newtype.
+    fn version_ord(client_version: &str, daemon_version: &str) -> anyhow::Result<cmp::Ordering> {
+        let client_parts = client_version
+            .split('.')
+            .map(|p| p.parse::<i64>())
+            .collect::<Result<Vec<_>, _>>()
+            .context("parsing client version")?;
+        if client_parts.len() != 3 {
+            return Err(anyhow!(
+                "parsing client version: got {} parts, want 3",
+                client_parts.len(),
+            ));
+        }
+
+        let daemon_parts = daemon_version
+            .split('.')
+            .map(|p| p.parse::<i64>())
+            .collect::<Result<Vec<_>, _>>()
+            .context("parsing daemon version")?;
+        if daemon_parts.len() != 3 {
+            return Err(anyhow!(
+                "parsing daemon version: got {} parts, want 3",
+                daemon_parts.len(),
+            ));
+        }
+
+        // pre 1.0 releases flag breaking changes with their
+        // minor version rather than major version.
+        if client_parts[0] == 0 && daemon_parts[0] == 0 {
+            return Ok(client_parts[1].cmp(&daemon_parts[1]));
+        }
+
+        Ok(client_parts[0].cmp(&daemon_parts[0]))
+    }
+
     /// pipe_bytes suffles bytes from std{in,out} to the unix
     /// socket and back again. It is the main loop of
     /// `shpool attach`.
@@ -317,4 +406,54 @@ mod test {
             assert_eq!(c, round_tripped);
         }
     }
+
+    #[test]
+    fn version_ordering_noerr() {
+        use std::cmp::Ordering;
+
+        let cases = vec![
+            ("1.0.0", "1.0.0", Ordering::Equal),
+            ("1.0.0", "1.0.1", Ordering::Equal),
+            ("1.0.0", "1.1.0", Ordering::Equal),
+            ("1.0.0", "1.1.1", Ordering::Equal),
+            ("1.0.0", "1.100.100", Ordering::Equal),
+            ("1.0.0", "2.0.0", Ordering::Less),
+            ("1.0.0", "2.8.0", Ordering::Less),
+            ("1.199.0", "2.8.0", Ordering::Less),
+            ("2.0.0", "1.0.0", Ordering::Greater),
+            ("0.1.0", "0.1.0", Ordering::Equal),
+            ("0.1.1", "0.1.0", Ordering::Equal),
+            ("0.1.1", "0.1.99", Ordering::Equal),
+            ("0.1.0", "0.2.0", Ordering::Less),
+            ("0.1.99", "0.2.0", Ordering::Less),
+            ("0.2.0", "0.1.0", Ordering::Greater),
+        ];
+
+        for (lhs, rhs, ordering) in cases {
+            let actual_ordering =
+                Client::version_ord(lhs, rhs).expect("version strings to have an ordering");
+            assert_eq!(actual_ordering, ordering);
+        }
+    }
+
+    #[test]
+    fn version_ordering_err() {
+        let cases = vec![
+            ("1.0.0", "1.0.0.0", "got 4 parts, want 3"),
+            ("1.0.0.0", "1.0.0", "got 4 parts, want 3"),
+            ("foobar", "1.0.0", "invalid digit found in string"),
+            ("1.foobar", "1.0.0", "invalid digit found in string"),
+        ];
+
+        for (lhs, rhs, err_substr) in cases {
+            if let Err(e) = Client::version_ord(lhs, rhs) {
+                eprintln!("ERR: {:?}", e);
+                eprintln!("EXPECTED SUBSTR: {}", err_substr);
+                let errstr = format!("{:?}", e);
+                assert!(errstr.contains(err_substr));
+            } else {
+                panic!("no error though we expected one");
+            }
+        }
+    }
 }
diff --git a/shpool-protocol/Cargo.toml b/shpool-protocol/Cargo.toml
index 069f57bd..43dcc831 100644
--- a/shpool-protocol/Cargo.toml
+++ b/shpool-protocol/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "shpool-protocol"
-version = "0.1.0"
+version = "0.2.0"
 edition = "2021"
 authors = ["Ethan Pailes <pailes@google.com>"]
 repository = "https://github.com/shell-pool/shpool"
diff --git a/shpool-protocol/src/lib.rs b/shpool-protocol/src/lib.rs
index a573ea35..604d7d93 100644
--- a/shpool-protocol/src/lib.rs
+++ b/shpool-protocol/src/lib.rs
@@ -19,7 +19,19 @@ use serde_derive::{Deserialize, Serialize};
 
 pub const VERSION: &str = env!("CARGO_PKG_VERSION");
 
-/// ConnectHeader is the blob of metadata that a client transmits when it
+/// The header used to advertize daemon version.
+///
+/// This header gets written by the daemon to every stream as
+/// soon as it is opened, which allows the client to compare
+/// version strings for protocol negotiation (basically just
+/// deciding if the user ought to be warned about mismatched
+/// versions).
+#[derive(Serialize, Deserialize, Debug)]
+pub struct VersionHeader {
+    pub version: String,
+}
+
+/// The blob of metadata that a client transmits when it
 /// first connects.
 ///
 /// It uses an enum to allow different connection types


Test patch:
diff --git a/shpool/tests/attach.rs b/shpool/tests/attach.rs
index 033a4449..efac6d16 100644
--- a/shpool/tests/attach.rs
+++ b/shpool/tests/attach.rs
@@ -1562,6 +1562,39 @@ fn autodaemonize() -> anyhow::Result<()> {
     })
 }
 
+#[test]
+#[timeout(30000)]
+fn version_mismatch_client_newer() -> anyhow::Result<()> {
+    support::dump_err(|| {
+        let mut daemon_proc = support::daemon::Proc::new(
+            "norc.toml",
+            DaemonArgs {
+                extra_env: vec![(
+                    String::from("SHPOOL_TEST__OVERRIDE_VERSION"),
+                    String::from("0.0.0"),
+                )],
+                ..DaemonArgs::default()
+            },
+        )
+        .context("starting daemon proc")?;
+        let mut attach_proc =
+            daemon_proc.attach("sh1", Default::default()).context("starting attach proc")?;
+        let mut line_matcher = attach_proc.line_matcher()?;
+        let mut stderr_line_matcher = attach_proc.stderr_line_matcher()?;
+
+        // we should see a warning prompting us
+        stderr_line_matcher.scan_until_re("is newer.*try restarting your daemon$")?;
+        stderr_line_matcher.scan_until_re("hit enter to continue.*$")?;
+        attach_proc.run_cmd("")?; // continue through it
+
+        // not really needed, just here to test the events system
+        attach_proc.run_cmd("echo hi")?;
+        line_matcher.scan_until_re("hi$")?;
+
+        Ok(())
+    })
+}
+
 #[ignore] // TODO: re-enable, this test if flaky
 #[test]
 fn up_arrow_no_crash() -> anyhow::Result<()> {
diff --git a/shpool/tests/detach.rs b/shpool/tests/detach.rs
index ff68b791..a587dd71 100644
--- a/shpool/tests/detach.rs
+++ b/shpool/tests/detach.rs
@@ -38,6 +38,42 @@ fn single_running() -> anyhow::Result<()> {
     })
 }
 
+#[test]
+#[timeout(30000)]
+fn version_mismatch_client_newer() -> anyhow::Result<()> {
+    support::dump_err(|| {
+        let mut daemon_proc = support::daemon::Proc::new(
+            "norc.toml",
+            DaemonArgs {
+                extra_env: vec![(
+                    String::from("SHPOOL_TEST__OVERRIDE_VERSION"),
+                    String::from("0.0.0"),
+                )],
+                ..DaemonArgs::default()
+            },
+        )
+        .context("starting daemon proc")?;
+
+        let waiter = daemon_proc.events.take().unwrap().waiter(["daemon-bidi-stream-enter"]);
+        let mut attach_proc =
+            daemon_proc.attach("sh1", Default::default()).context("starting attach proc")?;
+
+        // get past the version mismatch prompt
+        attach_proc.run_cmd("")?;
+
+        daemon_proc.events = Some(waiter.wait_final_event("daemon-bidi-stream-enter")?);
+
+        let out = daemon_proc.detach(vec![String::from("sh1")])?;
+        assert!(out.status.success());
+
+        let stderr = String::from_utf8_lossy(&out.stderr[..]);
+        assert!(stderr.contains("is newer"));
+        assert!(stderr.contains("try restarting"));
+
+        Ok(())
+    })
+}
+
 #[test]
 #[timeout(30000)]
 fn single_not_running() -> anyhow::Result<()> {
diff --git a/shpool/tests/kill.rs b/shpool/tests/kill.rs
index 74d9644a..e28269ac 100644
--- a/shpool/tests/kill.rs
+++ b/shpool/tests/kill.rs
@@ -51,6 +51,43 @@ fn empty() -> anyhow::Result<()> {
     })
 }
 
+#[test]
+#[timeout(30000)]
+fn version_mismatch_client_newer() -> anyhow::Result<()> {
+    support::dump_err(|| {
+        let mut daemon_proc = support::daemon::Proc::new(
+            "norc.toml",
+            DaemonArgs {
+                extra_env: vec![(
+                    String::from("SHPOOL_TEST__OVERRIDE_VERSION"),
+                    String::from("0.0.0"),
+                )],
+                ..DaemonArgs::default()
+            },
+        )
+        .context("starting daemon proc")?;
+
+        let waiter = daemon_proc.events.take().unwrap().waiter(["daemon-bidi-stream-enter"]);
+        let mut attach_proc =
+            daemon_proc.attach("sh1", Default::default()).context("starting attach proc")?;
+
+        // get past the version mismatch prompt
+        attach_proc.run_cmd("")?;
+
+        daemon_proc.events = Some(waiter.wait_final_event("daemon-bidi-stream-enter")?);
+
+        let out = daemon_proc.kill(vec![String::from("sh1")])?;
+        assert!(out.status.success());
+
+        let stderr = String::from_utf8_lossy(&out.stderr[..]);
+        println!("stderr: {}", stderr);
+        assert!(stderr.contains("is newer"));
+        assert!(stderr.contains("try restarting"));
+
+        Ok(())
+    })
+}
+
 #[test]
 #[timeout(30000)]
 fn single_attached() -> anyhow::Result<()> {
diff --git a/shpool/tests/list.rs b/shpool/tests/list.rs
index 642e170f..74c9ef07 100644
--- a/shpool/tests/list.rs
+++ b/shpool/tests/list.rs
@@ -31,6 +31,58 @@ fn empty() -> anyhow::Result<()> {
     })
 }
 
+#[test]
+#[timeout(30000)]
+fn version_mismatch_client_newer() -> anyhow::Result<()> {
+    support::dump_err(|| {
+        let mut daemon_proc = support::daemon::Proc::new(
+            "norc.toml",
+            DaemonArgs {
+                listen_events: false,
+                extra_env: vec![(
+                    String::from("SHPOOL_TEST__OVERRIDE_VERSION"),
+                    String::from("0.0.0"),
+                )],
+            },
+        )
+        .context("starting daemon proc")?;
+        let out = daemon_proc.list()?;
+        assert!(out.status.success(), "list proc did not exit successfully");
+
+        let stderr = String::from_utf8_lossy(&out.stderr[..]);
+        assert!(stderr.contains("is newer"));
+        assert!(stderr.contains("try restarting"));
+
+        Ok(())
+    })
+}
+
+#[test]
+#[timeout(30000)]
+fn version_mismatch_client_older() -> anyhow::Result<()> {
+    support::dump_err(|| {
+        let mut daemon_proc = support::daemon::Proc::new(
+            "norc.toml",
+            DaemonArgs {
+                listen_events: false,
+                extra_env: vec![(
+                    String::from("SHPOOL_TEST__OVERRIDE_VERSION"),
+                    String::from("99999.0.0"),
+                )],
+            },
+        )
+        .context("starting daemon proc")?;
+        let out = daemon_proc.list()?;
+        assert!(out.status.success(), "list proc did not exit successfully");
+
+        let stderr = String::from_utf8_lossy(&out.stderr[..]);
+        assert!(stderr.contains("is older"));
+        assert!(stderr.contains("try restarting"));
+
+        Ok(())
+    })
+}
+
 #[test]
 #[timeout(30000)]
 fn no_daemon() -> anyhow::Result<()> {