fix: require explicit workspace group close (#3206)

refs #2874

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

4 files changed, 85 insertions, 8 deletions

diff --git a/tests/api_ping.rs b/tests/api_ping.rs
index 917eb2d9..d6b253fb 100644
--- a/tests/api_ping.rs
+++ b/tests/api_ping.rs
@@ -304,7 +304,7 @@ fn ping_over_socket_returns_version() {
     assert_eq!(value["result"]["version"], env!("CARGO_PKG_VERSION"));
     // Intentionally hardcoded so wire protocol bumps require updating this test.
     // Changing this value means old clients/servers are no longer compatible.
-    assert_eq!(value["result"]["protocol"], 20);
+    assert_eq!(value["result"]["protocol"], 21);
 
     cleanup_spawned_herdr(child, base);
 }
diff --git a/tests/cli/sessions.rs b/tests/cli/sessions.rs
index d86de44a..1f7e3c0b 100644
--- a/tests/cli/sessions.rs
+++ b/tests/cli/sessions.rs
@@ -389,7 +389,7 @@ fn status_commands_report_client_and_server_versions() {
         "stdout: {full_stdout}"
     );
     assert!(
-        full_stdout.contains("  protocol: 20"),
+        full_stdout.contains("  protocol: 21"),
         "stdout: {full_stdout}"
     );
     assert!(full_stdout.contains("server:\n"), "stdout: {full_stdout}");
@@ -422,7 +422,7 @@ fn status_commands_report_client_and_server_versions() {
         "stdout: {server_stdout}"
     );
     assert!(
-        server_stdout.contains("protocol: 20"),
+        server_stdout.contains("protocol: 21"),
         "stdout: {server_stdout}"
     );
 
@@ -434,7 +434,7 @@ fn status_commands_report_client_and_server_versions() {
         "stdout: {client_stdout}"
     );
     assert!(
-        client_stdout.contains("protocol: 20"),
+        client_stdout.contains("protocol: 21"),
         "stdout: {client_stdout}"
     );
     assert!(
@@ -444,7 +444,7 @@ fn status_commands_report_client_and_server_versions() {
 
     let full_json = run_cli_json(&socket_path, &["status", "--json"]);
     assert_eq!(full_json["client"]["version"], env!("CARGO_PKG_VERSION"));
-    assert_eq!(full_json["client"]["protocol"], 20);
+    assert_eq!(full_json["client"]["protocol"], 21);
     assert_eq!(full_json["server"]["status"], "running");
     assert_eq!(full_json["server"]["running"], true);
     assert_eq!(full_json["server"]["compatible"], true);
@@ -458,12 +458,12 @@ fn status_commands_report_client_and_server_versions() {
     let server_json = run_cli_json(&socket_path, &["status", "server", "--json"]);
     assert_eq!(server_json["status"], "running");
     assert_eq!(server_json["version"], env!("CARGO_PKG_VERSION"));
-    assert_eq!(server_json["protocol"], 20);
+    assert_eq!(server_json["protocol"], 21);
     assert_eq!(server_json["compatible"], true);
 
     let client_json = run_cli_json(&socket_path, &["status", "client", "--json"]);
     assert_eq!(client_json["version"], env!("CARGO_PKG_VERSION"));
-    assert_eq!(client_json["protocol"], 20);
+    assert_eq!(client_json["protocol"], 21);
     assert!(client_json["binary"]
         .as_str()
         .is_some_and(|path| !path.is_empty()));
diff --git a/tests/cli/workspace.rs b/tests/cli/workspace.rs
index 4153f3b2..afa47c66 100644
--- a/tests/cli/workspace.rs
+++ b/tests/cli/workspace.rs
@@ -217,6 +217,83 @@ fn worktree_management_commands_work() {
 }
 
 #[test]
+fn workspace_close_requires_explicit_worktree_group_intent() {
+    let base = unique_test_dir();
+    let config_home = base.join("config");
+    let runtime_dir = base.join("runtime");
+    let socket_path = runtime_dir.join("herdr.sock");
+    let repo = base.join("repo");
+    let checkout = base.join("checkout");
+    create_committed_repo(&repo);
+
+    let herdr = spawn_herdr(&config_home, &runtime_dir, &socket_path);
+    wait_for_socket(&socket_path, Duration::from_secs(5));
+
+    let created = run_cli_json(
+        &socket_path,
+        &[
+            "worktree",
+            "create",
+            "--cwd",
+            repo.to_str().unwrap(),
+            "--branch",
+            "worktree/group-close",
+            "--path",
+            checkout.to_str().unwrap(),
+        ],
+    );
+    assert_eq!(created["result"]["type"], "worktree_created");
+    let workspaces = run_cli_json(&socket_path, &["workspace", "list"]);
+    let workspace_list = workspaces["result"]["workspaces"].as_array().unwrap();
+    let parent_workspace_id = workspace_list
+        .iter()
+        .find(|workspace| workspace["worktree"]["is_linked_worktree"] == false)
+        .and_then(|workspace| workspace["workspace_id"].as_str())
+        .unwrap()
+        .to_string();
+    let workspace_ids = workspace_list
+        .iter()
+        .map(|workspace| workspace["workspace_id"].as_str().unwrap().to_string())
+        .collect::<Vec<_>>();
+
+    let rejected = run_cli(&socket_path, &["workspace", "close", &parent_workspace_id]);
+    assert_eq!(rejected.status.code(), Some(1));
+    let error: serde_json::Value = serde_json::from_slice(&rejected.stderr).unwrap();
+    assert_eq!(error["error"]["code"], "workspace_group_close_required");
+    let after_rejection = run_cli_json(&socket_path, &["workspace", "list"]);
+    assert_eq!(
+        after_rejection["result"]["workspaces"]
+            .as_array()
+            .unwrap()
+            .iter()
+            .map(|workspace| workspace["workspace_id"].as_str().unwrap().to_string())
+            .collect::<Vec<_>>(),
+        workspace_ids
+    );
+
+    let closed = run_cli(
+        &socket_path,
+        &["workspace", "close", &parent_workspace_id, "--group"],
+    );
+    assert!(
+        closed.status.success(),
+        "stderr: {}",
+        String::from_utf8_lossy(&closed.stderr)
+    );
+    let after_close = run_cli_json(&socket_path, &["workspace", "list"]);
+    assert!(after_close["result"]["workspaces"]
+        .as_array()
+        .unwrap()
+        .is_empty());
+    assert!(
+        checkout.exists(),
+        "workspace close must not remove the checkout"
+    );
+
+    cleanup_spawned_herdr(herdr, base);
+}
+
+#[test]
 fn forced_worktree_remove_terminates_processes_inside_checkout() {
     let base = unique_test_dir();
     let config_home = base.join("config");
diff --git a/tests/support/mod.rs b/tests/support/mod.rs
index f0ed100b..f295dbbe 100644
--- a/tests/support/mod.rs
+++ b/tests/support/mod.rs
@@ -15,7 +15,7 @@ static INIT: Once = Once::new();
 static CLEANUP_GUARD: OnceLock<CleanupGuard> = OnceLock::new();
 const WATCHDOG_SCAN_INTERVAL: Duration = Duration::from_secs(1);
 const RUNTIME_OWNER_MARKER: &str = ".herdr-test-owner-pid";
-pub const CURRENT_PROTOCOL: u32 = 20;
+pub const CURRENT_PROTOCOL: u32 = 21;
 
 pub fn register_spawned_herdr_pid(pid: Option<u32>) {
     let Some(pid) = pid else {