fix: guard cli requests against protocol mismatches (#1453)

refs #1435

Co-authored-by: akbash-bot <300245827+akbash-bot@users.noreply.github.com>
Co-authored-by: Can Celik <ogulcancelik@gmail.com>

5 files changed, 157 insertions, 4 deletions

diff --git a/src/cli.rs b/src/cli.rs
index 1ce262b4..e59be830 100644
--- a/src/cli.rs
+++ b/src/cli.rs
@@ -17,6 +17,7 @@ mod integration;
 mod notification;
 mod pane;
 mod plugin;
+mod protocol_guard;
 mod runtime;
 mod server;
 mod spec;
@@ -937,7 +938,9 @@ pub(super) fn wait_for_agent_change(
     timeout_message: &str,
 ) -> std::io::Result<i32> {
     let read_timeout = timeout_ms.map(Duration::from_millis);
-    let (ack, mut stream) = ApiClient::local()
+    let client = ApiClient::local();
+    ensure_server_protocol_compatible(&client, &request.id)?;
+    let (ack, mut stream) = client
         .subscribe_value(&request, read_timeout)
         .map_err(api_client_error_to_io)?;
     if let Err(err) = crate::api::client::parse_response_value(ack) {
@@ -990,11 +993,43 @@ pub(super) fn send_ok_request(method: Method) -> std::io::Result<i32> {
 }
 
 pub(super) fn send_request(request: &Request) -> std::io::Result<serde_json::Value> {
+    let client = ApiClient::local();
+    ensure_server_protocol_compatible(&client, &request.id)?;
+    client
+        .request_value(request)
+        .map_err(api_client_error_to_io)
+}
+
+pub(super) fn send_request_unchecked(request: &Request) -> std::io::Result<serde_json::Value> {
     ApiClient::local()
         .request_value(request)
         .map_err(api_client_error_to_io)
 }
 
+fn ensure_server_protocol_compatible(client: &ApiClient, request_id: &str) -> std::io::Result<()> {
+    let status = client.status().map_err(api_client_error_to_io)?;
+    let server_protocol = status
+        .protocol
+        .ok_or_else(|| std::io::Error::other("server ping did not include a protocol version"))?;
+    let Some(response) = protocol_guard::mismatch_response(
+        request_id,
+        server_protocol,
+        &crate::session::active_restart_after_update_guidance(),
+    ) else {
+        return Ok(());
+    };
+
+    eprintln!(
+        "{}",
+        serde_json::to_string(&response).map_err(std::io::Error::other)?
+    );
+    Err(protocol_guard::reported_error())
+}
+
+pub(crate) fn protocol_mismatch_was_reported(err: &std::io::Error) -> bool {
+    protocol_guard::was_reported(err)
+}
+
 fn api_timeout_error(err: &std::io::Error) -> bool {
     matches!(
         err.kind(),
diff --git a/src/cli/plugin.rs b/src/cli/plugin.rs
index 5cc528f4..68cf0282 100644
--- a/src/cli/plugin.rs
+++ b/src/cli/plugin.rs
@@ -932,6 +932,9 @@ fn register_installed_plugin(
                             ),
                         )));
                     }
+                    Err(unlink_err) if super::protocol_mismatch_was_reported(&unlink_err) => {
+                        return Err(InstallFailure::KeepCheckout(unlink_err));
+                    }
                     Err(unlink_err) => {
                         return Err(InstallFailure::KeepCheckout(std::io::Error::other(
                             format!(
diff --git a/src/cli/protocol_guard.rs b/src/cli/protocol_guard.rs
new file mode 100644
index 00000000..21302bbf
--- /dev/null
+++ b/src/cli/protocol_guard.rs
@@ -0,0 +1,110 @@
+use std::fmt;
+
+use crate::api::schema::{ErrorBody, ErrorResponse};
+
+#[derive(Debug)]
+pub(super) struct ProtocolMismatchReported;
+
+impl fmt::Display for ProtocolMismatchReported {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.write_str("protocol mismatch was already reported")
+    }
+}
+
+impl std::error::Error for ProtocolMismatchReported {}
+
+pub(super) fn mismatch_response(
+    request_id: &str,
+    server_protocol: u32,
+    restart_guidance: &str,
+) -> Option<ErrorResponse> {
+    let client_protocol = crate::protocol::PROTOCOL_VERSION;
+    if client_protocol == server_protocol {
+        return None;
+    }
+
+    let message = if client_protocol > server_protocol {
+        format!(
+            "client protocol {client_protocol} is newer than server protocol {server_protocol}; restart the Herdr server before using this command. {restart_guidance}"
+        )
+    } else {
+        format!(
+            "client protocol {client_protocol} is older than server protocol {server_protocol}; upgrade the Herdr client before using this command"
+        )
+    };
+
+    Some(ErrorResponse {
+        id: request_id.to_string(),
+        error: ErrorBody {
+            code: "protocol_mismatch".into(),
+            message,
+        },
+    })
+}
+
+pub(super) fn reported_error() -> std::io::Error {
+    std::io::Error::other(ProtocolMismatchReported)
+}
+
+pub(super) fn was_reported(err: &std::io::Error) -> bool {
+    err.get_ref()
+        .and_then(|source| source.downcast_ref::<ProtocolMismatchReported>())
+        .is_some()
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn matching_protocol_has_no_error() {
+        assert!(mismatch_response("req", crate::protocol::PROTOCOL_VERSION, "restart").is_none());
+    }
+
+    #[test]
+    fn older_server_error_preserves_request_id_and_guidance() {
+        let response = mismatch_response(
+            "cli:wait:agent-status",
+            crate::protocol::PROTOCOL_VERSION - 1,
+            "Run the session stop command, then restart.",
+        )
+        .unwrap();
+
+        assert_eq!(response.id, "cli:wait:agent-status");
+        assert_eq!(response.error.code, "protocol_mismatch");
+        assert!(response.error.message.contains(&format!(
+            "client protocol {}",
+            crate::protocol::PROTOCOL_VERSION
+        )));
+        assert!(response.error.message.contains(&format!(
+            "server protocol {}",
+            crate::protocol::PROTOCOL_VERSION - 1
+        )));
+        assert!(response.error.message.contains("restart"));
+    }
+
+    #[test]
+    fn newer_server_error_tells_user_to_upgrade_client() {
+        let response = mismatch_response(
+            "cli:pane:list",
+            crate::protocol::PROTOCOL_VERSION + 1,
+            "unused restart guidance",
+        )
+        .unwrap();
+
+        assert!(response
+            .error
+            .message
+            .contains("older than server protocol"));
+        assert!(response.error.message.contains("upgrade the Herdr client"));
+        assert!(!response.error.message.contains("unused restart guidance"));
+    }
+
+    #[test]
+    fn reported_error_is_recognizable_without_string_matching() {
+        assert!(was_reported(&reported_error()));
+        assert!(!was_reported(&std::io::Error::other(
+            "protocol mismatch was already reported"
+        )));
+    }
+}
diff --git a/src/cli/server.rs b/src/cli/server.rs
index 29775999..ff87cab1 100644
--- a/src/cli/server.rs
+++ b/src/cli/server.rs
@@ -201,7 +201,9 @@ fn server_live_handoff(args: &[String]) -> std::io::Result<i32> {
         return Ok(2);
     };
 
-    let response = super::send_request(&Request {
+    // Live handoff is itself a protocol-mismatch recovery path, so it must
+    // reach the running server without the normal CLI compatibility guard.
+    let response = super::send_request_unchecked(&Request {
         id: "cli:server:live-handoff".into(),
         method: Method::ServerLiveHandoff(params),
     })?;
diff --git a/src/main.rs b/src/main.rs
index 1e44b058..0649da9e 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -473,8 +473,11 @@ fn main() -> io::Result<()> {
         std::process::exit(2);
     }
 
-    if let cli::CommandOutcome::Handled(code) = cli::maybe_run(&args)? {
-        std::process::exit(code);
+    match cli::maybe_run(&args) {
+        Ok(cli::CommandOutcome::Handled(code)) => std::process::exit(code),
+        Ok(cli::CommandOutcome::NotCli) => {}
+        Err(err) if cli::protocol_mismatch_was_reported(&err) => std::process::exit(1),
+        Err(err) => return Err(err),
     }
 
     // Subcommands and flags (no TUI, no logging needed)