diff options
25 files changed, 465 insertions, 113 deletions
diff --git a/docs/next/api/herdr-api.schema.json b/docs/next/api/herdr-api.schema.json index f9642ffa..8ff90f20 100644 --- a/docs/next/api/herdr-api.schema.json +++ b/docs/next/api/herdr-api.schema.json @@ -1,6 +1,6 @@ { "$schema": "https://json-schema.org/draft/2020-12/schema", - "protocol": 20, + "protocol": 21, "schema_version": 1, "schemas": { "error_response": { @@ -4145,6 +4145,20 @@ ], "type": "string" }, + "WorkspaceCloseParams": { + "properties": { + "close_group": { + "type": "boolean" + }, + "workspace_id": { + "type": "string" + } + }, + "required": [ + "workspace_id" + ], + "type": "object" + }, "WorkspaceCreateParams": { "properties": { "cwd": { @@ -4722,7 +4736,7 @@ "type": "string" }, "params": { - "$ref": "#/schemas/request/$defs/WorkspaceTarget" + "$ref": "#/schemas/request/$defs/WorkspaceCloseParams" } }, "required": [ diff --git a/docs/next/website/src/content/docs/cli-reference.mdx b/docs/next/website/src/content/docs/cli-reference.mdx index a0abc541..87d89c1f 100644 --- a/docs/next/website/src/content/docs/cli-reference.mdx +++ b/docs/next/website/src/content/docs/cli-reference.mdx @@ -121,7 +121,7 @@ herdr workspace get <workspace_id> herdr workspace focus <workspace_id> herdr workspace rename <workspace_id> <label> herdr workspace report-metadata <workspace_id> --source ID [--token NAME=VALUE] [--clear-token NAME] [--seq N] [--ttl-ms N] -herdr workspace close <workspace_id> +herdr workspace close <workspace_id> [--group] ``` Create a workspace without stealing focus: @@ -143,7 +143,7 @@ herdr worktree remove --workspace ID [--force] Worktrees are normal Herdr workspaces with Git checkout provenance. `worktree create` creates a Git worktree checkout, opens it as a workspace, and groups it with the parent repo workspace. If `--branch` names an existing local branch, Herdr checks it out; otherwise it creates the branch from `--base` or `HEAD`. Without `--path`, Herdr creates the checkout under `<worktrees.directory>/<repo>/<branch-slug>`. -`workspace close` closes only Herdr state. To delete the checkout, run `worktree remove`. It runs `git worktree remove`, never deletes the branch, and requires `--force` when Git refuses a dirty checkout. +`workspace close` closes only Herdr state. Closing a primary workspace while linked-worktree workspaces are open requires `--group`; without it, the command leaves the group open and returns `workspace_group_close_required`. To delete the checkout, run `worktree remove`. It runs `git worktree remove`, never deletes the branch, and requires `--force` when Git refuses a dirty checkout. ## Tabs diff --git a/docs/next/website/src/content/docs/ja/cli-reference.mdx b/docs/next/website/src/content/docs/ja/cli-reference.mdx index 9c14c6d6..7cf905bd 100644 --- a/docs/next/website/src/content/docs/ja/cli-reference.mdx +++ b/docs/next/website/src/content/docs/ja/cli-reference.mdx @@ -117,7 +117,7 @@ herdr workspace get <workspace_id> herdr workspace focus <workspace_id> herdr workspace rename <workspace_id> <label> herdr workspace report-metadata <workspace_id> --source ID [--token NAME=VALUE] [--clear-token NAME] [--seq N] [--ttl-ms N] -herdr workspace close <workspace_id> +herdr workspace close <workspace_id> [--group] ``` フォーカスを奪わずにワークスペースを作成します: @@ -139,7 +139,7 @@ herdr worktree remove --workspace ID [--force] worktree は、Git チェックアウトの出自情報を持つ通常の Herdr ワークスペースです。`worktree create` は Git worktree のチェックアウトを作成し、ワークスペースとして開き、親リポジトリのワークスペースとグループ化します。`--branch` が既存のローカルブランチを指す場合はそれをチェックアウトし、そうでなければ `--base` または `HEAD` からブランチを作成します。`--path` がない場合、チェックアウトは `<worktrees.directory>/<repo>/<branch-slug>` の下に作成されます。 -`workspace close` は Herdr の状態だけを閉じます。`worktree remove` が明示的なチェックアウト削除の経路です。`git worktree remove` を実行し、ブランチは決して削除せず、Git がダーティなチェックアウトを拒否する場合は `--force` が必要です。 +`workspace close` は Herdr の状態だけを閉じます。リンクされた worktree ワークスペースが開いている間に親ワークスペースを閉じるには `--group` が必要です。指定しない場合、グループは開いたままになり `workspace_group_close_required` が返されます。`worktree remove` が明示的なチェックアウト削除の経路です。`git worktree remove` を実行し、ブランチは決して削除せず、Git がダーティなチェックアウトを拒否する場合は `--force` が必要です。 ## タブ diff --git a/docs/next/website/src/content/docs/ja/socket-api.mdx b/docs/next/website/src/content/docs/ja/socket-api.mdx index 0dac2e13..5bb4fea7 100644 --- a/docs/next/website/src/content/docs/ja/socket-api.mdx +++ b/docs/next/website/src/content/docs/ja/socket-api.mdx @@ -299,6 +299,8 @@ Worktree メソッドは Git チェックアウトを Herdr ワークスペー worktree コマンドはライフサイクルイベントも発行します。`worktree.create` は `workspace.created`、`tab.created`、`pane.created`、`worktree.created` を発行します。`worktree.open` は `worktree.opened` を発行し、新しい Herdr ワークスペースを開いた場合はワークスペース/タブ/ペインの作成イベントも発行します。`worktree.remove` は `worktree.removed` を発行し、リンクされたワークスペースがまだ開いている場合は `workspace.closed` も発行します。 +リンクされた worktree ワークスペースが開いている親ワークスペースを `workspace.close` で閉じるには、params に `"close_group": true` が必要です。明示的なグループクローズ指定がない場合は `workspace_group_close_required` を返します。明示的なグループクローズは、閉じた各ワークスペースについて `workspace.closed` イベントを 1 件ずつ発行します。 + ## エージェントビューのクエリ `agent.view.set` は、組み込みの Agents ビューに一つの一時的な宣言型プロジェクションを diff --git a/docs/next/website/src/content/docs/socket-api.mdx b/docs/next/website/src/content/docs/socket-api.mdx index 0cae833e..8a055a36 100644 --- a/docs/next/website/src/content/docs/socket-api.mdx +++ b/docs/next/website/src/content/docs/socket-api.mdx @@ -412,6 +412,8 @@ Use at most one of `workspace_id` or `cwd` for `worktree.list`, `worktree.create Worktree commands also emit lifecycle events. `worktree.create` emits `workspace.created`, `tab.created`, `pane.created`, and `worktree.created`. `worktree.open` emits `worktree.opened`, and it also emits workspace/tab/pane creation events when it opens a new Herdr workspace. `worktree.remove` emits `worktree.removed`; if the linked workspace is still open, it also emits `workspace.closed`. +`workspace.close` rejects closing a primary workspace while linked-worktree workspaces are open unless its params include `"close_group": true`, returning `workspace_group_close_required` when explicit group intent is missing. An explicit group close emits one `workspace.closed` event for each workspace it closes. + ## Agent view queries `agent.view.set` installs one transient declarative projection for the built-in diff --git a/docs/next/website/src/content/docs/zh-cn/cli-reference.mdx b/docs/next/website/src/content/docs/zh-cn/cli-reference.mdx index 496f4a6a..ade416ff 100644 --- a/docs/next/website/src/content/docs/zh-cn/cli-reference.mdx +++ b/docs/next/website/src/content/docs/zh-cn/cli-reference.mdx @@ -117,7 +117,7 @@ herdr workspace get <workspace_id> herdr workspace focus <workspace_id> herdr workspace rename <workspace_id> <label> herdr workspace report-metadata <workspace_id> --source ID [--token NAME=VALUE] [--clear-token NAME] [--seq N] [--ttl-ms N] -herdr workspace close <workspace_id> +herdr workspace close <workspace_id> [--group] ``` 不抢占焦点地创建工作区: @@ -139,7 +139,7 @@ herdr worktree remove --workspace ID [--force] worktree 是带有 Git 检出来源信息的普通 Herdr 工作区。`worktree create` 创建一个 Git worktree 检出,作为工作区打开,并与父仓库工作区分到一组。如果 `--branch` 指向已有的本地分支,Herdr 检出它;否则从 `--base` 或 `HEAD` 创建分支。没有 `--path` 时,Herdr 在 `<worktrees.directory>/<repo>/<branch-slug>` 下创建检出。 -`workspace close` 只关闭 Herdr 状态。`worktree remove` 是显式的检出删除路径;它运行 `git worktree remove`,从不删除分支,并在 Git 拒绝脏检出时要求 `--force`。 +`workspace close` 只关闭 Herdr 状态。当关联的 worktree 工作区仍然打开时，关闭父工作区需要 `--group`；否则命令会保留整个组并返回 `workspace_group_close_required`。`worktree remove` 是显式的检出删除路径;它运行 `git worktree remove`,从不删除分支,并在 Git 拒绝脏检出时要求 `--force`。 ## 标签页 diff --git a/docs/next/website/src/content/docs/zh-cn/socket-api.mdx b/docs/next/website/src/content/docs/zh-cn/socket-api.mdx index 6fb197f1..ace5ba70 100644 --- a/docs/next/website/src/content/docs/zh-cn/socket-api.mdx +++ b/docs/next/website/src/content/docs/zh-cn/socket-api.mdx @@ -299,6 +299,8 @@ Worktree 方法把 Git 检出作为 Herdr 工作区管理。`worktree.create` Worktree 命令也发出生命周期事件。`worktree.create` 发出 `workspace.created`、`tab.created`、`pane.created` 和 `worktree.created`。`worktree.open` 发出 `worktree.opened`,并在打开新的 Herdr 工作区时同时发出工作区/标签页/窗格创建事件。`worktree.remove` 发出 `worktree.removed`;如果关联的工作区仍然打开,还会发出 `workspace.closed`。 +当关联的 worktree 工作区仍然打开时，父工作区只有在 `workspace.close` 的 params 中包含 `"close_group": true` 时才会被关闭；缺少显式关闭组意图时会返回 `workspace_group_close_required`。显式关闭组时，每个被关闭的工作区都会发出一个 `workspace.closed` 事件。 + ## 智能体视图查询 `agent.view.set` 为内置 Agents 视图设置一个临时的声明式投影。智能体信息或当前 diff --git a/src/api/schema.rs b/src/api/schema.rs index 6f76e3d3..e06b701c 100644 --- a/src/api/schema.rs +++ b/src/api/schema.rs @@ -80,7 +80,7 @@ pub enum Method { #[serde(rename = "workspace.report_metadata")] WorkspaceReportMetadata(WorkspaceReportMetadataParams), #[serde(rename = "workspace.close")] - WorkspaceClose(WorkspaceTarget), + WorkspaceClose(WorkspaceCloseParams), #[serde(rename = "worktree.list")] WorktreeList(WorktreeListParams), #[serde(rename = "worktree.create")] diff --git a/src/api/schema/tests.rs b/src/api/schema/tests.rs index 5fe6dd27..5199038d 100644 --- a/src/api/schema/tests.rs +++ b/src/api/schema/tests.rs @@ -62,6 +62,34 @@ fn request_uses_dot_method_names() { } #[test] +fn workspace_close_group_intent_defaults_false_and_round_trips() { + let request: Request = serde_json::from_value(serde_json::json!({ + "id": "close", + "method": "workspace.close", + "params": { "workspace_id": "w1" } + })) + .unwrap(); + assert!(matches!( + request.method, + Method::WorkspaceClose(WorkspaceCloseParams { + close_group: false, + .. + }) + )); + + let explicit = Request { + id: "close-group".into(), + method: Method::WorkspaceClose(WorkspaceCloseParams { + workspace_id: "w1".into(), + close_group: true, + }), + }; + let json = serde_json::to_value(&explicit).unwrap(); + assert_eq!(json["params"]["close_group"], true); + assert_eq!(serde_json::from_value::<Request>(json).unwrap(), explicit); +} + +#[test] fn agent_start_and_prompt_requests_round_trip() { let start = Request { id: "start".into(), diff --git a/src/api/schema/workspaces.rs b/src/api/schema/workspaces.rs index f9ad4ced..bfc63252 100644 --- a/src/api/schema/workspaces.rs +++ b/src/api/schema/workspaces.rs @@ -17,6 +17,13 @@ pub struct WorkspaceCreateParams { } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, schemars::JsonSchema)] +pub struct WorkspaceCloseParams { + pub workspace_id: String, + #[serde(default, skip_serializing_if = "super::is_false")] + pub close_group: bool, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, schemars::JsonSchema)] pub struct WorkspaceRenameParams { pub workspace_id: String, pub label: String, diff --git a/src/app/actions.rs b/src/app/actions.rs index e3fc2639..07f7278b 100644 --- a/src/app/actions.rs +++ b/src/app/actions.rs @@ -1677,24 +1677,7 @@ impl AppState { self.selection = None; self.selection_autoscroll = None; self.mark_session_dirty(); - let close_indices = self - .workspaces - .get(self.selected) - .and_then(|ws| ws.worktree_space()) - .filter(|space| !space.is_linked_worktree) - .map(|space| { - self.workspaces - .iter() - .enumerate() - .filter_map(|(idx, ws)| { - ws.worktree_space() - .is_some_and(|member| member.key == space.key) - .then_some(idx) - }) - .collect::<Vec<_>>() - }) - .filter(|indices| indices.len() >= 2) - .unwrap_or_else(|| vec![self.selected]); + let close_indices = self.workspace_close_indices(self.selected); let mut terminal_ids = Vec::new(); let mut pane_ids = Vec::new(); @@ -1984,31 +1967,55 @@ impl AppState { self.apply_pane_zoom(ws_idx, pane_id, PaneZoomCommand::Toggle); } - pub(crate) fn workspace_close_would_close_worktree_group(&self, ws_idx: usize) -> bool { + pub(crate) fn workspace_close_indices(&self, ws_idx: usize) -> Vec<usize> { self.workspaces .get(ws_idx) .and_then(|ws| ws.worktree_space()) .filter(|space| !space.is_linked_worktree) - .is_some_and(|space| { + .map(|space| { self.workspaces .iter() - .filter(|ws| { + .enumerate() + .filter_map(|(idx, ws)| { ws.worktree_space() .is_some_and(|member| member.key == space.key) + .then_some(idx) }) - .count() - >= 2 + .collect::<Vec<_>>() }) + .filter(|indices| indices.len() >= 2) + .unwrap_or_else(|| vec![ws_idx]) + } + + pub(crate) fn workspace_close_would_close_worktree_group(&self, ws_idx: usize) -> bool { + self.workspace_close_indices(ws_idx).len() >= 2 + } + + pub(crate) fn begin_workspace_close_confirmation(&mut self, ws_idx: usize) -> bool { + let Some(workspace_id) = self + .workspaces + .get(ws_idx) + .map(|workspace| workspace.id.clone()) + else { + return false; + }; + self.selected = ws_idx; + self.confirm_close_workspace_id = Some(workspace_id); + self.mode = Mode::ConfirmClose; + true + } + + pub(crate) fn take_confirmed_workspace_close_index(&mut self) -> Option<usize> { + let workspace_id = self.confirm_close_workspace_id.take()?; + self.workspaces + .iter() + .position(|workspace| workspace.id == workspace_id) } pub(crate) fn confirm_implicit_worktree_group_close(&mut self, ws_idx: usize) -> bool { - if self.confirm_close && self.workspace_close_would_close_worktree_group(ws_idx) { - self.selected = ws_idx; - self.mode = Mode::ConfirmClose; - true - } else { - false - } + self.confirm_close + && self.workspace_close_would_close_worktree_group(ws_idx) + && self.begin_workspace_close_confirmation(ws_idx) } #[cfg(test)] diff --git a/src/app/api/workspaces.rs b/src/app/api/workspaces.rs index cf50d3ad..d9e689e3 100644 --- a/src/app/api/workspaces.rs +++ b/src/app/api/workspaces.rs @@ -1,8 +1,8 @@ use std::path::PathBuf; use crate::api::schema::{ - EventData, EventEnvelope, EventKind, ResponseResult, WorkspaceCreateParams, - WorkspaceMoveBlockParams, WorkspaceMoveParams, WorkspaceRenameParams, + EventData, EventEnvelope, EventKind, ResponseResult, WorkspaceCloseParams, + WorkspaceCreateParams, WorkspaceMoveBlockParams, WorkspaceMoveParams, WorkspaceRenameParams, WorkspaceReportMetadataParams, WorkspaceTarget, }; use crate::app::App; @@ -295,37 +295,46 @@ impl App { encode_success(id, ResponseResult::Ok {}) } - pub(super) fn handle_workspace_close(&mut self, id: String, target: WorkspaceTarget) -> String { - let Some(index) = self.parse_workspace_id(&target.workspace_id) else { - return workspace_not_found(id, &target.workspace_id); + pub(super) fn handle_workspace_close( + &mut self, + id: String, + params: WorkspaceCloseParams, + ) -> String { + let Some(index) = self.parse_workspace_id(¶ms.workspace_id) else { + return workspace_not_found(id, ¶ms.workspace_id); }; if self.state.workspaces.get(index).is_none() { - return workspace_not_found(id, &target.workspace_id); + return workspace_not_found(id, ¶ms.workspace_id); } - let workspace_id = self.public_workspace_id(index); - let workspace = self.workspace_info(index); - let pane_ids = self - .state - .workspaces - .get(index) - .map(|ws| { - ws.tabs - .iter() - .flat_map(|tab| tab.layout.pane_ids()) - .collect::<Vec<_>>() + let close_indices = self.state.workspace_close_indices(index); + if close_indices.len() >= 2 && !params.close_group { + return encode_error( + id, + "workspace_group_close_required", + "workspace has linked worktree workspaces; use --group (close_group=true in the API) to close the group", + ); + } + let closed_workspaces = close_indices + .iter() + .map(|index| { + ( + self.public_workspace_id(*index), + self.workspace_info(*index), + ) }) - .unwrap_or_default(); + .collect::<Vec<_>>(); self.state.selected = index; self.state.close_selected_workspace(); - self.state.remove_plugin_pane_records(pane_ids); self.shutdown_detached_terminal_runtimes(); - self.emit_event(EventEnvelope { - event: EventKind::WorkspaceClosed, - data: EventData::WorkspaceClosed { - workspace_id, - workspace: Some(workspace), - }, - }); + for (workspace_id, workspace) in closed_workspaces { + self.emit_event(EventEnvelope { + event: EventKind::WorkspaceClosed, + data: EventData::WorkspaceClosed { + workspace_id, + workspace: Some(workspace), + }, + }); + } encode_success(id, ResponseResult::Ok {}) } @@ -455,21 +464,149 @@ mod tests { app } + fn app_with_worktree_group() -> App { + let mut app = app_with_linked_worktree(); + let mut parent = Workspace::test_new("parent"); + parent.worktree_space = Some(crate::workspace::WorktreeSpaceMembership { + key: "repo-key".into(), + label: "herdr".into(), + repo_root: "/repo/herdr".into(), + checkout_path: "/repo/herdr".into(), + is_linked_worktree: false, + }); + app.state.workspaces.insert(0, parent); + app.state.active = Some(1); + app.state.selected = 1; + app.state.mode = crate::app::Mode::Terminal; + app + } + + #[test] + fn api_workspace_close_parent_group_requires_explicit_group_intent() { + for confirm_close in [true, false] { + let mut app = app_with_worktree_group(); + app.state.confirm_close = confirm_close; + let parent_id = app.public_workspace_id(0); + let workspace_ids = app + .state + .workspaces + .iter() + .map(|workspace| workspace.id.clone()) + .collect::<Vec<_>>(); + + let request: crate::api::schema::Request = serde_json::from_value(serde_json::json!({ + "id": "req", + "method": "workspace.close", + "params": { "workspace_id": parent_id } + })) + .unwrap(); + let response = app.handle_api_request(request); + + let response: serde_json::Value = serde_json::from_str(&response).unwrap(); + assert_eq!(response["error"]["code"], "workspace_group_close_required"); + assert!(app.event_hub.events_after(0).is_empty()); + assert_eq!(app.state.mode, crate::app::Mode::Terminal); + assert_eq!(app.state.active, Some(1)); + assert_eq!(app.state.selected, 1); + assert_eq!( + app.state + .workspaces + .iter() + .map(|workspace| workspace.id.clone()) + .collect::<Vec<_>>(), + workspace_ids + ); + } + } + + #[test] + fn api_workspace_close_noncontiguous_group_preserves_adversarial_identity_state() { + let mut app = app_with_worktree_group(); + let parent = app.state.workspaces.remove(0); + let linked = app.state.workspaces.remove(0); + app.state = crate::app::state::AppState::test_with_adversarial_identity_state(); + let survivor_id = app.state.workspaces[0].id.clone(); + app.state.workspaces.insert(0, parent); + app.state.workspaces.push(linked); + app.state.active = Some(1); + app.state.selected = 1; + app.state.mode = crate::app::Mode::Terminal; + app.state.ensure_test_terminals(); + let closed_pane_ids = [0, 2].map(|index| app.state.workspaces[index].tabs[0].root_pane); + let closed_terminal_ids = [0, 2].map(|index| { + app.state + .terminal_id_for_pane(index, app.state.workspaces[index].tabs[0].root_pane) + .expect("closed workspace pane has a terminal") + }); + for pane_id in closed_pane_ids { + app.state.plugin_panes.insert( + pane_id, + crate::app::state::PluginPaneRecord { + plugin_id: "example.pane".into(), + entrypoint: "board".into(), + }, + ); + } + app.state.assert_invariants_for_test(); + + let parent_id = app.public_workspace_id(0); + let closed = [0, 2] + .into_iter() + .map(|index| (app.public_workspace_id(index), app.workspace_info(index))) + .collect::<Vec<_>>(); + + let response = app.handle_workspace_close( + "req".into(), + WorkspaceCloseParams { + workspace_id: parent_id, + close_group: true, + }, + ); + + let success: SuccessResponse = serde_json::from_str(&response).unwrap(); + assert_eq!(success.id, "req"); + assert_eq!(app.state.workspaces.len(), 1); + assert_eq!(app.state.workspaces[0].id, survivor_id); + for terminal_id in closed_terminal_ids { + assert!(!app.state.terminals.contains_key(&terminal_id)); + } + for pane_id in closed_pane_ids { + assert!(!app.state.plugin_panes.contains_key(&pane_id)); + } + assert!(app.state.terminal_runtime_shutdowns.is_empty()); + app.state.assert_invariants_for_test(); + let events = app.event_hub.events_after(0); + assert_eq!(events.len(), closed.len()); + for ((_, event), (workspace_id, workspace)) in events.iter().zip(closed) { + assert!(matches!(event.event, EventKind::WorkspaceClosed)); + assert!(matches!( + &event.data, + EventData::WorkspaceClosed { + workspace_id: closed_id, + workspace: Some(closed_workspace), + } if closed_id == &workspace_id && closed_workspace == &workspace + )); + } + } + #[test] fn api_workspace_close_closes_linked_worktree_workspace_only() { - let mut app = app_with_linked_worktree(); + let mut app = app_with_worktree_group(); + let linked_id = app.public_workspace_id(1); let response = app.handle_workspace_close( "req".into(), - WorkspaceTarget { - workspace_id: app.state.workspaces[0].id.clone(), + WorkspaceCloseParams { + workspace_id: linked_id, + close_group: true, }, ); let success: SuccessResponse = serde_json::from_str(&response).unwrap(); assert_eq!(success.id, "req"); assert_eq!(app.state.request_remove_linked_worktree, None); - assert!(app.state.workspaces.is_empty()); + assert_eq!(app.state.workspaces.len(), 1); + assert_eq!(app.state.workspaces[0].display_name(), "parent"); } #[test] @@ -482,8 +619,9 @@ mod tests { let response = app.handle_workspace_close( "req".into(), - WorkspaceTarget { + WorkspaceCloseParams { workspace_id: workspace_id.clone(), + close_group: false, }, ); diff --git a/src/app/input/modal.rs b/src/app/input/modal.rs index 343fa99c..e710459b 100644 --- a/src/app/input/modal.rs +++ b/src/app/input/modal.rs @@ -733,12 +733,15 @@ pub(crate) fn handle_resize_key(state: &mut AppState, raw_key: TerminalKey) { } pub(super) fn open_confirm_close(state: &mut AppState) { - state.mode = Mode::ConfirmClose; + state.begin_workspace_close_confirmation(state.selected); } #[cfg(test)] pub(super) fn confirm_close_accept(state: &mut AppState) { - state.close_selected_workspace(); + if let Some(ws_idx) = state.take_confirmed_workspace_close_index() { + state.selected = ws_idx; + state.close_selected_workspace(); + } if state.workspaces.is_empty() { state.mode = Mode::Navigate; } else { @@ -747,6 +750,7 @@ pub(super) fn confirm_close_accept(state: &mut AppState) { } pub(super) fn confirm_close_cancel(state: &mut AppState) { + state.confirm_close_workspace_id = None; state.mode = Mode::Navigate; } @@ -1113,9 +1117,8 @@ impl App { } pub(super) fn confirm_close_accept_via_api(&mut self) { - let ws_idx = self.state.selected; - if ws_idx < self.state.workspaces.len() { - self.close_workspace_idx_via_api(ws_idx); + if let Some(ws_idx) = self.state.take_confirmed_workspace_close_index() { + self.close_workspace_idx_with_group_via_api(ws_idx); } self.state.mode = if self.state.active.is_some() { Mode::Terminal @@ -1245,7 +1248,7 @@ impl App { if self.state.confirm_close { open_confirm_close(&mut self.state); } else { - self.close_workspace_idx_via_api(ws_idx); + self.close_workspace_idx_with_group_via_api(ws_idx); self.state.mode = Mode::Navigate; } } @@ -2139,8 +2142,8 @@ mod tests { #[test] fn confirm_close_keyboard_actions_are_direct_not_focused() { let mut state = state_with_workspaces(&["a", "b"]); - state.mode = Mode::ConfirmClose; state.selected = 1; + open_confirm_close(&mut state); handle_confirm_close_key( &mut state, @@ -2149,7 +2152,7 @@ mod tests { assert_eq!(state.mode, Mode::Navigate); assert_eq!(state.workspaces.len(), 2); - state.mode = Mode::ConfirmClose; + open_confirm_close(&mut state); handle_confirm_close_key( &mut state, KeyEvent::new(KeyCode::Enter, KeyModifiers::empty()), @@ -2160,7 +2163,6 @@ mod tests { #[test] fn confirm_close_for_linked_worktree_closes_workspace_only() { let mut state = state_with_workspaces(&["main", "issue"]); - state.mode = Mode::ConfirmClose; state.selected = 1; state.workspaces[1].worktree_space = Some(crate::workspace::WorktreeSpaceMembership { key: "repo-key".into(), @@ -2170,6 +2172,7 @@ mod tests { is_linked_worktree: true, }); + open_confirm_close(&mut state); handle_confirm_close_key( &mut state, KeyEvent::new(KeyCode::Enter, KeyModifiers::empty()), @@ -2305,6 +2308,50 @@ mod tests { } #[test] + fn api_confirm_close_accept_closes_parent_worktree_group() { + let mut app = app_with_test_workspaces(&["main", "issue"]); + mark_worktree_space_member(&mut app.state, 0, "repo-key"); + mark_worktree_space_member(&mut app.state, 1, "repo-key"); + app.state.selected = 0; + open_confirm_close(&mut app.state); + + app.handle_confirm_close_key_via_api(KeyEvent::new(KeyCode::Enter, KeyModifiers::empty())); + + assert!(app.state.workspaces.is_empty()); + assert_eq!(app.state.mode, Mode::Navigate); + assert_eq!(app.event_hub.events_after(0).len(), 2); + } + + #[test] + fn api_confirm_close_accept_keeps_the_original_workspace_target() { + let mut app = app_with_test_workspaces(&["main", "issue", "other"]); + mark_worktree_space_member(&mut app.state, 0, "repo-key"); + mark_worktree_space_member(&mut app.state, 1, "repo-key"); + app.state.selected = 0; + open_confirm_close(&mut app.state); + + app.focus_workspace_idx_via_api(2); + assert_eq!(app.state.selected, 2); + assert_eq!(app.state.mode, Mode::ConfirmClose); + + app.handle_confirm_close_key_via_api(KeyEvent::new(KeyCode::Enter, KeyModifiers::empty())); + + assert_eq!(app.state.workspaces.len(), 1); + assert_eq!(app.state.workspaces[0].display_name(), "other"); + assert_eq!( + app.event_hub + .events_after(0) + .iter() + .filter(|(_, event)| matches!( + event.event, + crate::api::schema::EventKind::WorkspaceClosed + )) + .count(), + 2 + ); + } + + #[test] fn api_context_menu_close_tab_last_parent_group_workspace_keeps_confirmation_mode() { let mut app = app_with_test_workspaces(&["main", "issue"]); mark_worktree_space_member(&mut app.state, 0, "repo-key"); diff --git a/src/app/input/mouse.rs b/src/app/input/mouse.rs index 3c6c86f4..bf82126b 100644 --- a/src/app/input/mouse.rs +++ b/src/app/input/mouse.rs @@ -3394,7 +3394,7 @@ mod tests { app.state.workspaces = vec![Workspace::test_new("a"), Workspace::test_new("b")]; app.state.active = Some(0); app.state.selected = 1; - app.state.mode = Mode::ConfirmClose; + app.state.begin_workspace_close_confirmation(1); let popup = app.state.confirm_close_rect(); let inner = Rect::new( diff --git a/src/app/input/navigate.rs b/src/app/input/navigate.rs index 8e3d7d6e..517e267d 100644 --- a/src/app/input/navigate.rs +++ b/src/app/input/navigate.rs @@ -214,7 +214,7 @@ impl App { if self.state.confirm_close { super::modal::open_confirm_close(&mut self.state); } else { - self.close_workspace_idx_via_api(ws_idx); + self.close_workspace_idx_with_group_via_api(ws_idx); leave_navigate_mode(&mut self.state); } } @@ -443,9 +443,9 @@ impl App { self.runtime_workspace_focus("tui.workspace.focus", workspace_id); } - pub(crate) fn close_workspace_idx_via_api(&mut self, ws_idx: usize) { + pub(crate) fn close_workspace_idx_with_group_via_api(&mut self, ws_idx: usize) { let workspace_id = self.public_workspace_id(ws_idx); - self.runtime_workspace_close("tui.workspace.close", workspace_id); + self.runtime_workspace_close_group("tui.workspace.close", workspace_id); } pub(crate) fn move_workspace_via_api(&mut self, source_ws_idx: usize, insert_idx: usize) { @@ -489,7 +489,7 @@ impl App { if self.state.confirm_implicit_worktree_group_close(ws_idx) { return true; } - self.close_workspace_idx_via_api(ws_idx); + self.close_workspace_idx_with_group_via_api(ws_idx); return false; } let tab_idx = self.state.workspaces[ws_idx].active_tab_index(); @@ -3518,6 +3518,23 @@ navigate_pane_down = "ctrl+j" } #[test] + fn tui_close_parent_group_closes_immediately_when_confirmation_disabled() { + let mut app = app_with_test_workspaces(&["main", "issue"]); + mark_worktree_space_member(&mut app.state, 0, "repo-key"); + mark_worktree_space_member(&mut app.state, 1, "repo-key"); + app.state.active = Some(0); + app.state.selected = 0; + app.state.mode = Mode::Navigate; + app.state.confirm_close = false; + + app.execute_tui_navigate_action(NavigateAction::CloseWorkspace, ActionContext::Navigate); + + assert!(app.state.workspaces.is_empty()); + assert_eq!(app.state.mode, Mode::Navigate); + assert_eq!(app.event_hub.events_after(0).len(), 2); + } + + #[test] fn prefix_close_pane_last_parent_group_pane_opens_confirmation() { let mut state = state_with_workspaces(&["main", "issue"]); mark_worktree_space_member(&mut state, 0, "repo-key"); diff --git a/src/app/mod.rs b/src/app/mod.rs index ff286c3f..c28f62da 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -718,6 +718,7 @@ impl App { host_mouse_pixels: None, session_dirty: false, terminal_runtime_shutdowns: Vec::new(), + confirm_close_workspace_id: None, }; state.terminals = restored_terminals; diff --git a/src/app/runtime_mutations.rs b/src/app/runtime_mutations.rs index e9b3fde1..d3d767e7 100644 --- a/src/app/runtime_mutations.rs +++ b/src/app/runtime_mutations.rs @@ -2,8 +2,9 @@ use crate::api::schema::{ EmptyParams, LayoutSetSplitRatioParams, Method, PaneFocusDirectionParams, PaneInputSetParams, PaneRenameParams, PaneResizeParams, PaneSplitParams, PaneSwapParams, PaneTarget, PaneZoomParams, TabCreateParams, TabMoveParams, TabRenameParams, TabTarget, - WorkspaceCreateParams, WorkspaceMoveBlockParams, WorkspaceMoveParams, WorkspaceRenameParams, - WorkspaceTarget, WorktreeCreateParams, WorktreeOpenParams, WorktreeRemoveParams, + WorkspaceCloseParams, WorkspaceCreateParams, WorkspaceMoveBlockParams, WorkspaceMoveParams, + WorkspaceRenameParams, WorkspaceTarget, WorktreeCreateParams, WorktreeOpenParams, + WorktreeRemoveParams, }; use super::App; @@ -61,12 +62,18 @@ impl App { self.dispatch_runtime_mutation(id, Method::WorkspaceMoveBlock(params)) } - pub(crate) fn runtime_workspace_close( + pub(crate) fn runtime_workspace_close_group( &mut self, id: &'static str, workspace_id: String, ) -> String { - self.dispatch_runtime_mutation(id, Method::WorkspaceClose(WorkspaceTarget { workspace_id })) + self.dispatch_runtime_mutation( + id, + Method::WorkspaceClose(WorkspaceCloseParams { + workspace_id, + close_group: true, + }), + ) } pub(crate) fn runtime_tab_create( diff --git a/src/app/state.rs b/src/app/state.rs index 21ee8899..782b8ae5 100644 --- a/src/app/state.rs +++ b/src/app/state.rs @@ -1445,6 +1445,8 @@ pub struct AppState { pub(crate) previous_pane_focus: Option<PaneFocusTarget>, pub selected: usize, pub mode: Mode, + /// Stable workspace identity captured when the close confirmation opens. + pub(crate) confirm_close_workspace_id: Option<String>, pub should_quit: bool, /// In monolithic --no-session mode, detach exits the app because there is no server to detach from. pub detach_exits: bool, @@ -1836,6 +1838,7 @@ impl AppState { previous_pane_focus: None, selected: 0, mode: Mode::Navigate, + confirm_close_workspace_id: None, should_quit: false, detach_exits: false, detach_requested: false, diff --git a/src/cli/runtime.rs b/src/cli/runtime.rs index 16d2d8b3..793ca24e 100644 --- a/src/cli/runtime.rs +++ b/src/cli/runtime.rs @@ -2,8 +2,8 @@ use crate::api::schema::{ EmptyParams, Method, PaneFocusDirectionParams, PaneInputSetParams, PaneMoveParams, PaneRenameParams, PaneResizeParams, PaneSplitParams, PaneSwapParams, PaneTarget, PaneZoomParams, Request, TabCreateParams, TabListParams, TabRenameParams, TabTarget, - WorkspaceCreateParams, WorkspaceRenameParams, WorkspaceTarget, WorktreeCreateParams, - WorktreeListParams, WorktreeOpenParams, WorktreeRemoveParams, + WorkspaceCloseParams, WorkspaceCreateParams, WorkspaceRenameParams, WorkspaceTarget, + WorktreeCreateParams, WorktreeListParams, WorktreeOpenParams, WorktreeRemoveParams, }; fn print_method_response(id: &'static str, method: Method) -> std::io::Result<i32> { @@ -42,11 +42,8 @@ pub(super) fn workspace_rename(params: WorkspaceRenameParams) -> std::io::Result print_method_response("cli:workspace:rename", Method::WorkspaceRename(params)) } -pub(super) fn workspace_close(workspace_id: String) -> std::io::Result<i32> { - print_method_response( - "cli:workspace:close", - Method::WorkspaceClose(WorkspaceTarget { workspace_id }), - ) +pub(super) fn workspace_close(params: WorkspaceCloseParams) -> std::io::Result<i32> { + print_method_response("cli:workspace:close", Method::WorkspaceClose(params)) } pub(super) fn tab_list(params: TabListParams) -> std::io::Result<i32> { diff --git a/src/cli/workspace.rs b/src/cli/workspace.rs index 525fc971..c24c9b73 100644 --- a/src/cli/workspace.rs +++ b/src/cli/workspace.rs @@ -225,16 +225,19 @@ fn workspace_report_metadata(args: &[String]) -> std::io::Result<i32> { } fn workspace_close(args: &[String]) -> std::io::Result<i32> { - let Some(raw_workspace_id) = args.first() else { - eprintln!("usage: herdr workspace close <workspace_id>"); - return Ok(2); + let (raw_workspace_id, close_group) = match args { + [workspace_id] => (workspace_id, false), + [workspace_id, flag] if flag == "--group" => (workspace_id, true), + _ => { + eprintln!("usage: herdr workspace close <workspace_id> [--group]"); + return Ok(2); + } }; - if args.len() != 1 { - eprintln!("usage: herdr workspace close <workspace_id>"); - return Ok(2); - } - super::runtime::workspace_close(super::normalize_workspace_id(raw_workspace_id)) + super::runtime::workspace_close(crate::api::schema::WorkspaceCloseParams { + workspace_id: super::normalize_workspace_id(raw_workspace_id), + close_group, + }) } fn print_workspace_help() { @@ -245,5 +248,5 @@ fn print_workspace_help() { eprintln!(" herdr workspace focus <workspace_id>"); eprintln!(" herdr workspace rename <workspace_id> <label>"); eprintln!(" herdr workspace report-metadata <workspace_id> --source ID [--token NAME=VALUE] [--clear-token NAME] [--seq N] [--ttl-ms N]"); - eprintln!(" herdr workspace close <workspace_id>"); + eprintln!(" herdr workspace close <workspace_id> [--group]"); } diff --git a/src/protocol/wire.rs b/src/protocol/wire.rs index 4a445361..3680a557 100644 --- a/src/protocol/wire.rs +++ b/src/protocol/wire.rs @@ -13,7 +13,7 @@ use serde::{Deserialize, Serialize}; // --------------------------------------------------------------------------- /// Current protocol version. Bumped when wire format changes incompatibly. -pub const PROTOCOL_VERSION: u32 = 20; +pub const PROTOCOL_VERSION: u32 = 21; /// Maximum allowed frame payload size (2 MB). Frames larger than this are /// rejected to prevent denial-of-service via oversized length prefixes. 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 { |