From 4936f34cd900ab4b2abfa37193929f775fa60962 Mon Sep 17 00:00:00 2001 From: limityan Date: Thu, 30 Jul 2026 17:35:03 +0800 Subject: [PATCH] feat(cli): support session deletion in Shared TUI --- .../agent-runtime-deployment-design.md | 39 ++++- .../rules/source/forbidden-rules.mjs | 4 +- scripts/core-boundaries/self-test.mjs | 1 + src/apps/cli/src/actions.rs | 19 ++- src/apps/cli/src/agent/runtime_client.rs | 102 ++++++++---- src/apps/cli/src/modes/chat.rs | 29 ++-- src/apps/cli/src/modes/chat/commands.rs | 20 +-- src/apps/cli/src/modes/chat/input.rs | 12 +- src/apps/cli/src/modes/chat/run.rs | 8 +- src/apps/cli/src/modes/chat/selection.rs | 94 ++++++++--- src/apps/cli/src/modes/chat/sessions.rs | 73 +++++---- src/apps/cli/src/modes/chat/tests.rs | 70 ++++++-- src/apps/cli/src/shared_runtime.rs | 86 ++++++++-- src/apps/cli/src/ui/startup.rs | 9 +- .../adapters/agent-runtime-ipc/AGENTS-CN.md | 2 +- .../adapters/agent-runtime-ipc/AGENTS.md | 9 +- .../agent-runtime-ipc/src/operation.rs | 150 ++++++++++++++++-- .../agent-runtime-ipc/src/protocol.rs | 3 +- .../adapters/agent-runtime-ipc/src/server.rs | 72 +++------ .../agent-runtime-ipc/src/session_lease.rs | 14 ++ .../src/tests/protocol_contracts.rs | 65 +++++++- .../src/tests/shared_controller.rs | 97 +++++++++++ 22 files changed, 752 insertions(+), 226 deletions(-) diff --git a/docs/architecture/agent-runtime-deployment-design.md b/docs/architecture/agent-runtime-deployment-design.md index 1a42c9841d..5199463a3d 100644 --- a/docs/architecture/agent-runtime-deployment-design.md +++ b/docs/architecture/agent-runtime-deployment-design.md @@ -42,8 +42,8 @@ flowchart TB | Session 写入 | BitFun Runtime 的持久化 Session 由 `SessionManager` 管理；同一存储位置中的同一 Session 同时只允许一个本机进程写入，list/view 等只读操作不受影响 | | 当前 HTTP Server | 只提供 health/info/WebSocket 外壳，未装配 Agent Runtime，因此不取得 workspace ownership；`bootstrap.rs` 仅保持 agent-enabled composition 的一致边界，不由当前入口启动 | | Shared local IPC | 未发布的本机协议已有 discovery、实例锁、严格握手、Session 控制权、有界事件流和 cleanup；唯一 consumer 是第一方交互式 TUI adapter | -| Shared TUI | `bitfun --shared` / `bitfun chat --shared` 可列出、创建、恢复和重命名当前 Session，读取 transcript，切换当前 Session 的 Agent mode/model，通过 `/reload [skills|instructions]` 刷新声明式上下文，提交/取消 Turn，处理 Permission 和 UserInput；默认仍是 Embedded | -| Shared GUI/Headless/ACP/SDK Host/Remote | 未交付，也不会由 `--shared` 隐式启用；Replay、Observer、Controller transfer、Session delete/fork 同样不在当前协议中 | +| Shared TUI | `bitfun --shared` / `bitfun chat --shared` 可列出、创建、恢复 Session，删除未被控制的空闲非当前 Session，重命名当前 Session，读取 transcript，切换当前 Session 的 Agent mode/model，通过 `/reload [skills|instructions]` 刷新声明式上下文，提交/取消 Turn，处理 Permission 和 UserInput；默认仍是 Embedded | +| Shared GUI/Headless/ACP/SDK Host/Remote | 未交付，也不会由 `--shared` 隐式启用；Replay、Observer、Controller transfer、Session archive/fork 同样不在当前协议中 | 因此当前交付的是一条窄的、显式启用的 Shared TUI deployment，不是通用本机 Server。具体 `EventQueue` 仍由 Core 产品装配；IPC 只把当前 TUI 必需的强类型操作和事件映射到同一个 Runtime owner，没有事件重放或公开协议承诺。 @@ -196,7 +196,9 @@ sequenceDiagram S-->>C: initialized(health + interactive_tui) C->>S: create or restore Session S-->>C: Session control + Session facts - C->>S: rename, update, or reload current Session context + C->>S: rename or update current Session + C->>S: delete idle non-current Session + C->>S: reload current Session context C->>S: submit/cancel Turn or answer Permission/UserInput S-->>C: Session-filtered authoritative events else invalid @@ -204,11 +206,11 @@ sequenceDiagram end ``` -当前私有协议（v6）只覆盖 TUI 已有用户旅程需要的窄操作： +当前私有协议（v7）只覆盖 TUI 已有用户旅程需要的窄操作： | 已支持 | 明确不支持 | |---|---| -| Health、Session list/create、原子 restore（含 transcript 与 pending Permission）、当前 Session rename、Agent mode/model update、声明式上下文 reload | Session delete/fork、跨 workspace attach、transcript 分页、模型目录/默认值和 Agent/Subagent 管理 | +| Health、Session list/create、原子 restore（含 transcript 与 pending Permission）、删除未被控制的空闲 Session、当前 Session rename、Agent mode/model update、声明式上下文 reload | Session archive/fork、跨 workspace attach、transcript 分页、模型目录/默认值和 Agent/Subagent 管理 | | Turn submit/cancel | replay、cursor、resume event stream | | pending/respond Permission、submit UserInput answers | observer、controller transfer、多 Session multiplex | | 连接断开清理、Session-filtered events | detach/observer/controller transfer、SDK callbacks、GUI/Remote/Peer/ACP/Headless wire | @@ -225,9 +227,9 @@ sequenceDiagram - JSON frame 使用 4-byte 长度前缀；request 在发送前执行 128 KiB 上限（覆盖 TUI 已有的 64 KiB 粘贴输入及类型化信封），response/event 在序列化时执行 8 MiB 上限。超限返回类型化错误，不能进行无界分配；超过该上限的历史 Session 暂由 Embedded TUI 打开，不在本阶段引入分页协议； - 未认证连接也计入有界 connection budget，单个客户端不能无限制造 server task； - 未知 frame/operation 信封字段、未知 operation、错误身份和不兼容版本 fail closed；复用的 Runtime DTO 按其既有反序列化契约处理字段； -- 一个连接最多控制一个 Session、同时最多提交一个活动 Turn；一个 Session 同时只有一个 controller。create/restore 在完整结果通过大小检查后才原子切换控制权，失败时保留原 Session。活动 Turn 期间不能切换 Session，也不能修改其名称、Agent mode 或 model。 +- 一个连接最多控制一个 Session、同时最多提交一个活动 Turn；一个 Session 同时只有一个 controller。create/restore 在完整结果通过大小检查后才原子切换控制权，失败时保留原 Session。活动 Turn 期间不能切换 Session，也不能修改其名称、Agent mode 或 model；删除只作用于非当前且未被任何连接控制的 Session。 - Submit 使用调用方已有的 `turn_id` 标识不确定结果；若提交超时，返回 `outcome_unknown`、关闭连接并按该 ID 取消。断连取消只有得到确认后才释放 Session 控制权；无法确认时继续隔离该 Session，直到 Runtime 进程退出。 -- Session rename 和 Agent mode/model update 复用既有 Runtime 端口和校验，Runtime 对最终更新保持权威并拒绝无效值。它们都是有副作用操作；发送前编码或 frame 上限失败表示请求未执行，连接仍可使用。rename 写入失败时恢复旧 metadata：确认恢复后返回明确失败，无法确认时返回 `outcome_unknown`。Shared Client 在请求写入后响应超时或丢失连接时也返回 `outcome_unknown` 并断开连接。两种情况都不自动重试；用户恢复 Session 并核对当前值后再决定是否重试。模式与模型目录仍是同版本第一方产品事实，不加入 IPC。 +- Session delete/rename 和 Agent mode/model update 复用既有 Runtime 端口和校验，Runtime 对最终结果保持权威并拒绝无效目标。它们都是有副作用操作；发送前编码或 frame 上限失败表示请求未执行，连接仍可使用。rename 写入失败时恢复旧 metadata：确认恢复后返回明确失败，无法确认时返回 `outcome_unknown`。Shared Client 在请求写入后响应超时或丢失连接时也返回 `outcome_unknown` 并断开连接。两种情况都不自动重试：rename 由用户恢复 Session 并核对当前值；delete 由用户重新打开 `/sessions` 核对目标是否仍存在。模式与模型目录仍是同版本第一方产品事实，不加入 IPC。 - 声明式上下文 reload 只失效当前 Session 的 instructions 缓存，并按目标复用 Skill Registry 刷新；它可在活动 Turn 中执行但不改写该 Turn，generation 保护保证下一条消息重建上下文。它不引入 watcher、热替换或第二套 Runtime owner。 - Shared TUI 的模型选择器复用 Client 已有的只读产品配置来显示同版本模型目录；它只把选中的 model ID 通过 `update current Session model` 交给 Runtime。Client 不持有 Session 写入权，也不通过 IPC 管理模型目录或默认值。 - Agent 事件流 lag/closed 后 fail closed；Permission lag 先从 Runtime 权威 pending 集合重建，重建失败或流关闭时取消当前 Turn 并退出。路由到父 Session 的嵌套 Permission 与 AskUserQuestion 复用现有 TUI 交互，不新增第二套 UI 状态。 @@ -341,6 +343,27 @@ sequenceDiagram Embedded 和 Shared 最终调用同一 `AgentRuntime::rename_session`。Runtime 只有在确认旧名称已保留时才返回明确失败；持久化恢复无法确认时，两种部署都返回 `outcome_unknown`。Shared 还会在请求已发送但权威响应丢失时返回该结果并关闭连接。用户恢复 Session、检查当前名称后再决定是否重试。 +### 5.4 Scenario (+1) · Delete an idle Session + +```mermaid +sequenceDiagram + participant U as User + participant T as TUI adapter + participant C as CLI Runtime client + participant R as Agent Runtime + + U->>T: /sessions then Ctrl+D + T->>T: reject current or active target + T->>C: delete_session(id) + C->>R: direct call or one Shared frame + R->>R: existing delete owner + R-->>C: applied / failed / outcome_unknown + C-->>T: typed result + T-->>U: remove only after applied +``` + +Embedded 和 Shared 最终调用同一 `AgentRuntime::delete_session`。Shared Server 只在请求方没有活动 Turn、目标 Session 未被任何 Client 控制时调用 Runtime owner；`session_in_use` 和 `not_found` 保持结构化错误。TUI 复用现有单个 Session 异步任务槽位，不阻塞事件循环，也不自动重试结果不确定的删除。 + ## 6. 隔离和生命周期原则 实例身份与 ownership key 分工不同： @@ -374,7 +397,7 @@ Session/Turn、事件恢复、Permission/UserInput、Controller、配置管理 |---|---| | 当前 consumer | 仅第一方交互式 TUI adapter；不自动包含 GUI、Headless CLI、Remote 或 SDK Host | | 稳定测试合同 | 本机 endpoint、initialize-first、128 KiB request / 8 MiB response-event 上限、连接上限、owner-checked cleanup、原子 Session controller 切换、单连接单活动 Turn、事件流失效后 fail closed、断连取消、30 秒空闲退出 | -| 当前业务范围 | Session/Turn/transcript、当前 Session name/Agent mode/model、Permission/UserInput 的 TUI 必需子集；任何新增操作都需要真实 consumer 和 owner 等价测试 | +| 当前业务范围 | Session list/create/restore/delete、Turn/transcript、当前 Session name/Agent mode/model、Permission/UserInput 的 TUI 必需子集；任何新增操作都需要真实 consumer 和 owner 等价测试 | | 协议地位 | crate 保持 `publish = false`；这是 workspace 内私有协议，不是 Agent SDK 或远程兼容承诺 | 架构守卫只允许 CLI 消费该 crate；IPC 可以复用稳定的 Event、Product Domain 与 Runtime Port DTO，但禁止依赖 Runtime 实现、SDK Host、services、Tauri 或远程网络 transport。 diff --git a/scripts/core-boundaries/rules/source/forbidden-rules.mjs b/scripts/core-boundaries/rules/source/forbidden-rules.mjs index 537a71ed9b..231b64edf5 100644 --- a/scripts/core-boundaries/rules/source/forbidden-rules.mjs +++ b/scripts/core-boundaries/rules/source/forbidden-rules.mjs @@ -6,9 +6,9 @@ export const forbiddenContentRules = [ reason: 'agent-runtime-ipc operation scope is frozen to the reviewed Shared TUI slice', patterns: [ { - regex: /^\s+(?!(?:Health|ListSessions|CreateSession|RestoreSession|RenameSession|UpdateSessionMode|UpdateSessionModel|ReloadSessionContext|SubmitTurn|CancelTurn|PendingPermissions|RespondPermission|SubmitUserAnswers|Unit|Sessions|SessionCreated|SessionRestored|TurnAccepted|TurnCancelled|Self|AgentContextReloadRequest|AgentDialogTurnRequest|AgentSessionCreateRequest|AgentSessionCreateResult|AgentSessionListRequest|AgentSessionModeUpdateRequest|AgentSessionModelUpdateRequest|AgentSessionSummary|AgentTurnCancellationRequest|AgentTurnCancellationResult|SessionTranscript)\b)[A-Z][A-Za-z0-9_]*\b/, + regex: /^\s+(?!(?:Health|ListSessions|CreateSession|RestoreSession|DeleteSession|RenameSession|UpdateSessionMode|UpdateSessionModel|ReloadSessionContext|SubmitTurn|CancelTurn|PendingPermissions|RespondPermission|SubmitUserAnswers|Unit|Sessions|SessionCreated|SessionRestored|TurnAccepted|TurnCancelled|None|CurrentController|AttachExisting|UncontrolledTarget|Self|RuntimeIpcSessionRequirement|RuntimeIpcOperationRules|AgentContextReloadRequest|AgentDialogTurnRequest|AgentSessionCreateRequest|AgentSessionCreateResult|AgentSessionListRequest|AgentSessionModeUpdateRequest|AgentSessionModelUpdateRequest|AgentSessionSummary|AgentTurnCancellationRequest|AgentTurnCancellationResult|SessionTranscript)\b)[A-Z][A-Za-z0-9_]*\b/, message: - 'agent-runtime-ipc may not add replay, observer, controller-transfer, deletion, fork, or other operations beyond the reviewed Shared TUI slice', + 'agent-runtime-ipc may not add archive, replay, observer, controller-transfer, fork, or other operations beyond the reviewed Shared TUI slice', }, ], }, diff --git a/scripts/core-boundaries/self-test.mjs b/scripts/core-boundaries/self-test.mjs index 793658e5ce..356439d141 100644 --- a/scripts/core-boundaries/self-test.mjs +++ b/scripts/core-boundaries/self-test.mjs @@ -4858,6 +4858,7 @@ export function runManifestParserSelfTest({ 'ManageAgents', ].every((name) => runtimeIpcOperationPattern.test(` ${name},`)) || runtimeIpcOperationPattern.test(' Health,') || + runtimeIpcOperationPattern.test(' DeleteSession {') || runtimeIpcOperationPattern.test(' RenameSession {') || runtimeIpcOperationPattern.test(' UpdateSessionMode {') || runtimeIpcOperationPattern.test(' UpdateSessionModel {') || diff --git a/src/apps/cli/src/actions.rs b/src/apps/cli/src/actions.rs index 607aed8168..013ac0252a 100644 --- a/src/apps/cli/src/actions.rs +++ b/src/apps/cli/src/actions.rs @@ -115,7 +115,7 @@ pub(crate) enum ActionHandler { pub(crate) const SHARED_TUI_EMBEDDED_HANDOFF: &str = "Exit all Shared TUI clients, wait up to 30 seconds for their Runtime to stop, then use default Embedded `bitfun chat`"; pub(crate) const SHARED_TUI_HELP_NOTE: &str = - "Shared TUI: start with `bitfun chat --shared`. Multiple TUI processes reuse one workspace Runtime, while each TUI controls at most one Session and each Session has one controller. Use `/rename ` to rename the current Session, `/agent`, Tab, or Shift+Tab to change its Agent mode, `/models` to change its model, and `/reload [skills|instructions]` to refresh declarative context for the next message. Model configuration, Agent/Subagent management, MCP, extension, account-sync, usage, and other management remain Embedded. Exit all Shared TUI clients and wait up to 30 seconds before returning to default Embedded `bitfun chat`."; + "Shared TUI: start with `bitfun chat --shared`. Multiple TUI processes reuse one workspace Runtime, while each TUI controls at most one Session and each Session has one controller. Use `/sessions` and Ctrl+D to delete an idle, non-current Session; use `/rename ` to rename the current Session, `/agent`, Tab, or Shift+Tab to change its Agent mode, `/models` to change its model, and `/reload [skills|instructions]` to refresh declarative context for the next message. Model configuration, Agent/Subagent management, MCP, extension, account-sync, usage, and other management remain Embedded. Exit all Shared TUI clients and wait up to 30 seconds before returning to default Embedded `bitfun chat`."; impl ActionHandler { pub(crate) const fn available_in_shared_tui(self, context: ActionContext) -> bool { @@ -1840,10 +1840,27 @@ mod tests { assert!(SHARED_TUI_HELP_NOTE.contains("`/models`")); assert!(SHARED_TUI_HELP_NOTE.contains("`/rename `")); assert!(SHARED_TUI_HELP_NOTE.contains("`/reload [skills|instructions]`")); + assert!(SHARED_TUI_HELP_NOTE.contains("Ctrl+D")); + assert!(SHARED_TUI_HELP_NOTE.contains("idle, non-current Session")); assert!(SHARED_TUI_HELP_NOTE.contains("Agent/Subagent management")); assert!(SHARED_TUI_HELP_NOTE.contains("remain Embedded")); } + #[test] + fn shared_startup_session_list_keeps_the_supported_delete_action() { + let source = include_str!("ui/startup.rs").replace("\r\n", "\n"); + let selector = source + .split_once("fn show_session_selector(&mut self)") + .expect("startup session selector") + .1 + .split_once("fn show_model_selector(&mut self)") + .expect("startup session selector boundary") + .0; + + assert!(selector.contains(".show(session_items, None, true)")); + assert!(!selector.contains("Session deletion is unavailable in Shared TUI")); + } + #[test] fn rename_is_an_idle_current_session_chat_action() { let action = action_by_id("rename_session", ActionContext::Chat) diff --git a/src/apps/cli/src/agent/runtime_client.rs b/src/apps/cli/src/agent/runtime_client.rs index 49d1cda851..4e6037b2e0 100644 --- a/src/apps/cli/src/agent/runtime_client.rs +++ b/src/apps/cli/src/agent/runtime_client.rs @@ -122,20 +122,20 @@ fn session_migration_notices( } #[derive(Debug)] -pub(crate) struct SessionUpdateError { +pub(crate) struct SessionOperationError { message: String, outcome_unknown: bool, } -impl fmt::Display for SessionUpdateError { +impl fmt::Display for SessionOperationError { fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { formatter.write_str(&self.message) } } -impl std::error::Error for SessionUpdateError {} +impl std::error::Error for SessionOperationError {} -impl SessionUpdateError { +impl SessionOperationError { fn runtime(error: RuntimeError) -> Self { let outcome_unknown = matches!( &error, @@ -606,23 +606,37 @@ impl CliAgentRuntimeClient { .await } - pub(crate) async fn delete_session(&self, session_id: &str) -> Result<()> { - self.embedded_runtime("deleting sessions")? - .delete_session(AgentSessionDeleteRequest { - workspace_path: self.project_workspace_path_string(), - session_id: session_id.to_string(), - remote_connection_id: None, - remote_ssh_host: None, - }) - .await - .map_err(|error| anyhow::anyhow!(error.into_message())) + pub(crate) async fn delete_session( + &self, + session_id: &str, + ) -> std::result::Result<(), SessionOperationError> { + match &self.backend { + CliAgentRuntimeBackend::Embedded(runtime) => runtime + .delete_session(AgentSessionDeleteRequest { + workspace_path: self.project_workspace_path_string(), + session_id: session_id.to_string(), + remote_connection_id: None, + remote_ssh_host: None, + }) + .await + .map_err(SessionOperationError::runtime), + CliAgentRuntimeBackend::Shared(client) => { + let result = client + .request(RuntimeIpcOperation::DeleteSession { + session_id: session_id.to_string(), + }) + .await + .map_err(SessionOperationError::shared)?; + expect_unit(result, "delete_session").map_err(SessionOperationError::unexpected) + } + } } pub(crate) async fn update_session_model( &self, session_id: &str, model_id: &str, - ) -> std::result::Result<(), SessionUpdateError> { + ) -> std::result::Result<(), SessionOperationError> { let request = AgentSessionModelUpdateRequest { session_id: session_id.to_string(), model_id: model_id.to_string(), @@ -631,13 +645,14 @@ impl CliAgentRuntimeClient { CliAgentRuntimeBackend::Embedded(runtime) => runtime .update_session_model(request) .await - .map_err(SessionUpdateError::runtime), + .map_err(SessionOperationError::runtime), CliAgentRuntimeBackend::Shared(client) => { let result = client .request(RuntimeIpcOperation::UpdateSessionModel { request }) .await - .map_err(SessionUpdateError::shared)?; - expect_unit(result, "update_session_model").map_err(SessionUpdateError::unexpected) + .map_err(SessionOperationError::shared)?; + expect_unit(result, "update_session_model") + .map_err(SessionOperationError::unexpected) } } } @@ -646,7 +661,7 @@ impl CliAgentRuntimeClient { &self, session_id: &str, session_name: &str, - ) -> std::result::Result<(), SessionUpdateError> { + ) -> std::result::Result<(), SessionOperationError> { match &self.backend { CliAgentRuntimeBackend::Embedded(runtime) => { let request = AgentSessionRenameRequest { @@ -659,7 +674,7 @@ impl CliAgentRuntimeClient { runtime .rename_session(request) .await - .map_err(SessionUpdateError::runtime) + .map_err(SessionOperationError::runtime) } CliAgentRuntimeBackend::Shared(client) => { let result = client @@ -670,8 +685,8 @@ impl CliAgentRuntimeClient { }, }) .await - .map_err(SessionUpdateError::shared)?; - expect_unit(result, "rename_session").map_err(SessionUpdateError::unexpected) + .map_err(SessionOperationError::shared)?; + expect_unit(result, "rename_session").map_err(SessionOperationError::unexpected) } } } @@ -680,7 +695,7 @@ impl CliAgentRuntimeClient { &self, session_id: &str, mode_id: &str, - ) -> std::result::Result<(), SessionUpdateError> { + ) -> std::result::Result<(), SessionOperationError> { let request = AgentSessionModeUpdateRequest { session_id: session_id.to_string(), mode_id: mode_id.to_string(), @@ -689,13 +704,14 @@ impl CliAgentRuntimeClient { CliAgentRuntimeBackend::Embedded(runtime) => runtime .update_session_mode(request) .await - .map_err(SessionUpdateError::runtime), + .map_err(SessionOperationError::runtime), CliAgentRuntimeBackend::Shared(client) => { let result = client .request(RuntimeIpcOperation::UpdateSessionMode { request }) .await - .map_err(SessionUpdateError::shared)?; - expect_unit(result, "update_session_mode").map_err(SessionUpdateError::unexpected) + .map_err(SessionOperationError::shared)?; + expect_unit(result, "update_session_mode") + .map_err(SessionOperationError::unexpected) } } } @@ -1327,7 +1343,7 @@ mod tests { use super::{ project_routed_permission_event, session_migration_notices, shared_disconnect_message, shared_restore_error, validated_session_summary, CliWorkspacePaths, SessionMigrationNotice, - SessionUpdateError, + SessionOperationError, }; use bitfun_agent_runtime_ipc::RuntimeIpcStreamInvalidationReason; @@ -1344,7 +1360,7 @@ mod tests { #[test] fn shared_session_update_preserves_unknown_outcome_as_a_typed_fact() { - let error = SessionUpdateError::shared(RuntimeIpcClientError::Remote(RuntimeIpcError { + let error = SessionOperationError::shared(RuntimeIpcClientError::Remote(RuntimeIpcError { code: RuntimeIpcErrorCode::OutcomeUnknown, message: "inspect authoritative state before retrying".to_string(), })); @@ -1357,21 +1373,21 @@ mod tests { RuntimeIpcClientError::Disconnected, RuntimeIpcClientError::UnexpectedResponse, ] { - assert!(SessionUpdateError::shared(transport_error).outcome_unknown()); + assert!(SessionOperationError::shared(transport_error).outcome_unknown()); } assert!( - SessionUpdateError::unexpected(anyhow::anyhow!("unexpected response shape")) + SessionOperationError::unexpected(anyhow::anyhow!("unexpected response shape")) .outcome_unknown() ); assert!( - !SessionUpdateError::shared(RuntimeIpcClientError::Remote(RuntimeIpcError { + !SessionOperationError::shared(RuntimeIpcClientError::Remote(RuntimeIpcError { code: RuntimeIpcErrorCode::InvalidRequest, message: "unknown mode".to_string(), },)) .outcome_unknown() ); assert!( - !SessionUpdateError::shared(RuntimeIpcClientError::RequestEncoding( + !SessionOperationError::shared(RuntimeIpcClientError::RequestEncoding( bitfun_agent_runtime_ipc::RuntimeIpcIoError::FrameTooLarge { size: 129, max_bytes: 128, @@ -1383,7 +1399,7 @@ mod tests { #[test] fn embedded_runtime_unknown_outcome_is_preserved() { - let error = SessionUpdateError::runtime(RuntimeError::Port(PortError::new( + let error = SessionOperationError::runtime(RuntimeError::Port(PortError::new( PortErrorKind::OutcomeUnknown, "inspect authoritative state", ))); @@ -1475,6 +1491,26 @@ mod tests { assert!(!rename.contains("serde_json::from_value")); } + #[test] + fn session_delete_uses_direct_runtime_or_private_shared_ipc() { + let source = include_str!("runtime_client.rs").replace("\r\n", "\n"); + let delete = source + .split_once("pub(crate) async fn delete_session(") + .expect("delete method") + .1 + .split_once("pub(crate) async fn update_session_model(") + .expect("delete method boundary") + .0; + + assert!(delete.contains("CliAgentRuntimeBackend::Embedded(runtime)")); + assert!(delete.contains(".delete_session(AgentSessionDeleteRequest {")); + assert!(delete.contains("CliAgentRuntimeBackend::Shared(client)")); + assert!(delete.contains("RuntimeIpcOperation::DeleteSession")); + assert!(!delete.contains("embedded_runtime")); + assert!(!delete.contains("serde_json::to_value")); + assert!(!delete.contains("serde_json::from_value")); + } + #[test] fn mode_updates_use_the_runtime_sdk_without_the_core_compatibility_facade() { let source = include_str!("runtime_client.rs").replace("\r\n", "\n"); diff --git a/src/apps/cli/src/modes/chat.rs b/src/apps/cli/src/modes/chat.rs index 373cbce8be..fc587f4d09 100644 --- a/src/apps/cli/src/modes/chat.rs +++ b/src/apps/cli/src/modes/chat.rs @@ -29,7 +29,7 @@ use crate::actions::{ ActionState, ResolvedKeymap, SHARED_TUI_EMBEDDED_HANDOFF, SHARED_TUI_HELP_NOTE, }; use crate::agent::context_reload_client::CliContextReloadClient; -use crate::agent::runtime_client::{CliAgentRuntimeClient, SessionUpdateError}; +use crate::agent::runtime_client::{CliAgentRuntimeClient, SessionOperationError}; use crate::chat_state::ChatState; use crate::config::CliConfig; use crate::ui::agent_selector::{AgentItem, AgentSelectorAction}; @@ -168,7 +168,7 @@ enum PendingMcpTask { }, } -enum PendingSessionUpdateKind { +enum PendingSessionOperationKind { Mode { mode_id: String, }, @@ -179,14 +179,18 @@ enum PendingSessionUpdateKind { Rename { session_name: String, }, + Delete { + session_name: String, + }, } -impl PendingSessionUpdateKind { +impl PendingSessionOperationKind { fn name(&self) -> &'static str { match self { Self::Mode { .. } => "agent mode", Self::Model { .. } => "model", Self::Rename { .. } => "name", + Self::Delete { .. } => "deletion", } } @@ -195,21 +199,22 @@ impl PendingSessionUpdateKind { Self::Mode { mode_id } => mode_id, Self::Model { model_id, .. } => model_id, Self::Rename { session_name } => session_name, + Self::Delete { session_name } => session_name, } } } -struct PendingSessionUpdate { +struct PendingSessionOperation { session_id: String, - kind: PendingSessionUpdateKind, + kind: PendingSessionOperationKind, started_at: Instant, slow_notice_shown: bool, exit_warning_shown: bool, - handle: tokio::task::JoinHandle>, + handle: tokio::task::JoinHandle>, } -const SESSION_UPDATE_SLOW_NOTICE: Duration = Duration::from_secs(15); -const SHARED_TUI_CHAT_STATUS: &str = "Shared TUI preview: this view controls sessions, turns, the current Session name, current Session Agent mode, current Session model, and declarative context via /reload [skills|instructions]; model management remains Embedded, along with local extension, MCP, account-sync, and Agent/Subagent management."; +const SESSION_OPERATION_SLOW_NOTICE: Duration = Duration::from_secs(15); +const SHARED_TUI_CHAT_STATUS: &str = "Shared TUI preview: this view controls sessions, including deleting an idle Session, turns, the current Session name, current Session Agent mode, current Session model, and declarative context via /reload [skills|instructions]; model management remains Embedded, along with local extension, MCP, account-sync, and Agent/Subagent management."; #[derive(Default)] struct NonKeyEventOutcome { @@ -248,9 +253,9 @@ pub(crate) struct ChatMode { pending_mcp_op: Option, /// Running MCP tasks (non-blocking, polled in main loop) pending_mcp_tasks: Vec, - /// One durable current-Session update in flight. The event loop remains responsive - /// while the runtime owner writes session metadata. - pending_session_update: Option, + /// One Session operation in flight. The event loop remains responsive while + /// the Runtime owner updates or deletes Session state. + pending_session_operation: Option, /// One explicit native slash-menu choice waiting for its parameterized submission. selected_native_command_once: Option, external_source_snapshot: Option, @@ -308,7 +313,7 @@ impl ChatMode { initial_prompt: None, pending_mcp_op: None, pending_mcp_tasks: Vec::new(), - pending_session_update: None, + pending_session_operation: None, selected_native_command_once: None, external_source_snapshot: None, external_source_conflict_choices: BTreeMap::new(), diff --git a/src/apps/cli/src/modes/chat/commands.rs b/src/apps/cli/src/modes/chat/commands.rs index 68f1698392..83ff6eba20 100644 --- a/src/apps/cli/src/modes/chat/commands.rs +++ b/src/apps/cli/src/modes/chat/commands.rs @@ -32,7 +32,7 @@ fn parse_reload_invocation( None } -fn pending_session_update_blocks_runtime_action( +fn pending_session_operation_blocks_runtime_action( shared_tui: bool, pending_for_current_session: bool, handler: ActionHandler, @@ -825,16 +825,16 @@ impl ChatMode { return Ok(None); } let pending_for_current_session = self - .pending_session_update + .pending_session_operation .as_ref() .is_some_and(|pending| pending.session_id == chat_state.core_session_id); - if pending_session_update_blocks_runtime_action( + if pending_session_operation_blocks_runtime_action( self.agent.is_shared(), pending_for_current_session, action.handler, ) { chat_view.set_status(Some(format!( - "Waiting for the current session update to finish before using {}.", + "Waiting for the pending Session operation to finish before using {}.", action.name ))); return Ok(None); @@ -1040,9 +1040,9 @@ impl ChatMode { chat_view.set_status(Some("Usage: /rename ".to_string())); return Ok(None); }; - if self.pending_session_update.is_some() { + if self.pending_session_operation.is_some() { chat_view.set_status(Some( - "A current session update is already in progress. Please wait.".to_string(), + "A Session operation is already in progress. Please wait.".to_string(), )); return Ok(None); } @@ -1063,9 +1063,9 @@ impl ChatMode { .rename_session(&task_session_id, &task_session_name) .await }); - self.pending_session_update = Some(PendingSessionUpdate { + self.pending_session_operation = Some(PendingSessionOperation { session_id, - kind: PendingSessionUpdateKind::Rename { session_name }, + kind: PendingSessionOperationKind::Rename { session_name }, started_at: Instant::now(), slow_notice_shown: false, exit_warning_shown: false, @@ -1095,12 +1095,12 @@ impl ChatMode { self.selected_native_command_once = None; } let pending_for_current_session = self - .pending_session_update + .pending_session_operation .as_ref() .is_some_and(|pending| pending.session_id == chat_state.core_session_id); if session_update_blocks_typed_submission(pending_for_current_session, trimmed) { chat_view.set_status(Some( - "Waiting for the current session update to finish before sending.".to_string(), + "Waiting for the pending Session operation to finish before sending.".to_string(), )); return Ok(None); } diff --git a/src/apps/cli/src/modes/chat/input.rs b/src/apps/cli/src/modes/chat/input.rs index e653f18633..c606081bdb 100644 --- a/src/apps/cli/src/modes/chat/input.rs +++ b/src/apps/cli/src/modes/chat/input.rs @@ -406,17 +406,17 @@ impl ChatMode { ChatExitReason::SwitchSession(_) | ChatExitReason::NewSession ) && shared_session_change_is_blocked( this.agent.is_shared(), - this.pending_session_update.is_some(), + this.pending_session_operation.is_some(), ) { chat_view.set_status(Some( - "Wait for the current session update to finish before changing sessions." + "Wait for the pending Session operation to finish before changing sessions." .to_string(), )); return; } match reason { ChatExitReason::SwitchSession(new_session_id) => { - if let Some(pending) = this.pending_session_update.as_mut() { + if let Some(pending) = this.pending_session_operation.as_mut() { pending.exit_warning_shown = false; } match this.switch_to_session( @@ -434,7 +434,7 @@ impl ChatMode { } } ChatExitReason::NewSession => { - if let Some(pending) = this.pending_session_update.as_mut() { + if let Some(pending) = this.pending_session_operation.as_mut() { pending.exit_warning_shown = false; } match this.create_new_session(session_id, chat_state, chat_view, rt_handle) { @@ -447,11 +447,11 @@ impl ChatMode { } } ChatExitReason::Quit => { - if let Some(pending) = this.pending_session_update.as_mut() { + if let Some(pending) = this.pending_session_operation.as_mut() { if !pending.exit_warning_shown { pending.exit_warning_shown = true; chat_view.set_status(Some( - "Exit requested. Waiting for the current session update to finish; exit again to leave now. The update may not be saved, and the next restore will use the last successfully persisted session state." + "Exit requested. Waiting for the pending Session operation to finish; exit again to leave now. Its outcome may be unknown until the Session list is inspected again." .to_string(), )); return; diff --git a/src/apps/cli/src/modes/chat/run.rs b/src/apps/cli/src/modes/chat/run.rs index 62626467bf..f602e6b1ad 100644 --- a/src/apps/cli/src/modes/chat/run.rs +++ b/src/apps/cli/src/modes/chat/run.rs @@ -254,7 +254,7 @@ impl ChatMode { ); chat_view.set_agent_mode_switch_allowed(session_update_allowed( chat_state.is_processing, - self.pending_session_update.is_some(), + self.pending_session_operation.is_some(), )); // Keep spinner animation smooth without forcing full redraw every loop. @@ -274,7 +274,11 @@ impl ChatMode { if self.poll_mcp_task_completion(&mut chat_view, &mut chat_state, &rt_handle) { needs_redraw = true; } - match self.poll_session_update_completion(&mut chat_view, &mut chat_state, &rt_handle) { + match self.poll_session_operation_completion( + &mut chat_view, + &mut chat_state, + &rt_handle, + ) { SessionUpdatePollOutcome::NoChange => {} SessionUpdatePollOutcome::Redraw => needs_redraw = true, SessionUpdatePollOutcome::ExitAfterSave => { diff --git a/src/apps/cli/src/modes/chat/selection.rs b/src/apps/cli/src/modes/chat/selection.rs index 53730d78a7..74c7f6a9d9 100644 --- a/src/apps/cli/src/modes/chat/selection.rs +++ b/src/apps/cli/src/modes/chat/selection.rs @@ -29,6 +29,27 @@ fn previous_session_update_status( } } +fn session_delete_feedback( + session_name: &str, + outcome: &SessionUpdateApplyOutcome, +) -> (bool, String) { + match outcome { + SessionUpdateApplyOutcome::Applied => { + (true, format!("Session deleted: {session_name}")) + } + SessionUpdateApplyOutcome::SessionUpdateFailed(error) => ( + false, + format!("Failed to delete session {session_name}: {error}"), + ), + SessionUpdateApplyOutcome::OutcomeUnknown(error) => ( + false, + format!( + "Session deletion for {session_name} has an unknown outcome: {error}. This TUI is closing; reopen it and inspect /sessions before retrying." + ), + ), + } +} + fn session_update_completion_should_exit(exit_requested: bool, applied: bool) -> bool { exit_requested && applied } @@ -396,7 +417,7 @@ impl ChatMode { ) { if !session_update_allowed( chat_state.is_processing, - self.pending_session_update.is_some(), + self.pending_session_operation.is_some(), ) { chat_view.set_status(Some(session_update_unavailable_message( "Agent mode", @@ -562,7 +583,7 @@ impl ChatMode { } if !session_update_allowed( chat_state.is_processing, - self.pending_session_update.is_some(), + self.pending_session_operation.is_some(), ) { chat_view.set_status(Some(session_update_unavailable_message( "Model", @@ -582,9 +603,9 @@ impl ChatMode { .update_session_model(&task_session_id, &task_model_id) .await }); - self.pending_session_update = Some(PendingSessionUpdate { + self.pending_session_operation = Some(PendingSessionOperation { session_id, - kind: PendingSessionUpdateKind::Model { + kind: PendingSessionOperationKind::Model { model_id: selected_id, display_name: selected_display_name, }, @@ -625,7 +646,7 @@ impl ChatMode { let allow_mode_switch = session_update_allowed( chat_state.is_processing, - self.pending_session_update.is_some(), + self.pending_session_operation.is_some(), ); if self.agent.is_shared() { chat_view.show_agent_modes_only( @@ -654,7 +675,7 @@ impl ChatMode { AgentSelectorAction::SwitchMode(selected) => { if !session_update_allowed( chat_state.is_processing, - self.pending_session_update.is_some(), + self.pending_session_operation.is_some(), ) { chat_view.set_status(Some(session_update_unavailable_message( "Agent mode", @@ -687,9 +708,9 @@ impl ChatMode { return; } - if self.pending_session_update.is_some() { + if self.pending_session_operation.is_some() { chat_view.set_status(Some( - "A current session update is already in progress. Please wait.".to_string(), + "A Session operation is already in progress. Please wait.".to_string(), )); return; } @@ -705,9 +726,9 @@ impl ChatMode { .update_session_mode(&task_session_id, &task_mode_id) .await }); - self.pending_session_update = Some(PendingSessionUpdate { + self.pending_session_operation = Some(PendingSessionOperation { session_id, - kind: PendingSessionUpdateKind::Mode { mode_id }, + kind: PendingSessionOperationKind::Mode { mode_id }, started_at: Instant::now(), slow_notice_shown: false, exit_warning_shown: false, @@ -715,25 +736,25 @@ impl ChatMode { }); } - fn poll_session_update_completion( + fn poll_session_operation_completion( &mut self, chat_view: &mut ChatView, chat_state: &mut ChatState, rt_handle: &tokio::runtime::Handle, ) -> SessionUpdatePollOutcome { - let Some(pending) = self.pending_session_update.as_mut() else { + let Some(pending) = self.pending_session_operation.as_mut() else { return SessionUpdatePollOutcome::NoChange; }; if !pending.handle.is_finished() { if !pending.slow_notice_shown - && pending.started_at.elapsed() >= SESSION_UPDATE_SLOW_NOTICE + && pending.started_at.elapsed() >= SESSION_OPERATION_SLOW_NOTICE { pending.slow_notice_shown = true; if !pending.exit_warning_shown { let message = if self.agent.is_shared() { - "The current session update is still being saved. You can keep editing; changing sessions and sending wait for the result." + "The Session operation is still running. You can keep editing; changing sessions and sending wait for the result." } else { - "The current session update is still being saved. You can edit or switch sessions; sending in this session waits." + "The Session operation is still running. You can edit or switch sessions; sending in the affected Session waits." }; chat_view.set_status(Some(message.to_string())); } @@ -742,9 +763,9 @@ impl ChatMode { return SessionUpdatePollOutcome::NoChange; } let pending = self - .pending_session_update + .pending_session_operation .take() - .expect("finished session update should remain present"); + .expect("finished session operation should remain present"); let outcome = match tokio::task::block_in_place(|| rt_handle.block_on(pending.handle)) { Ok(Ok(())) => SessionUpdateApplyOutcome::Applied, Ok(Err(error)) if error.outcome_unknown() => { @@ -752,10 +773,34 @@ impl ChatMode { } Ok(Err(error)) => SessionUpdateApplyOutcome::SessionUpdateFailed(error.to_string()), Err(error) => SessionUpdateApplyOutcome::SessionUpdateFailed(format!( - "session update task failed: {error}" + "session operation task failed: {error}" )), }; let unknown_outcome = matches!(&outcome, SessionUpdateApplyOutcome::OutcomeUnknown(_)); + if let PendingSessionOperationKind::Delete { session_name } = &pending.kind { + let (remove_item, status) = session_delete_feedback(session_name, &outcome); + if remove_item { + chat_view.session_selector_remove_item(&pending.session_id); + tracing::info!("Deleted session: {}", pending.session_id); + } else { + tracing::error!( + session_id = %pending.session_id, + outcome = if unknown_outcome { "unknown" } else { "failed" }, + "Session deletion was not confirmed" + ); + } + chat_view.set_status(Some(status.clone())); + if unknown_outcome { + return SessionUpdatePollOutcome::ExitAfterUnknownOutcome(status); + } + chat_view.reshow_session_selector(); + return if session_update_completion_should_exit(pending.exit_warning_shown, remove_item) + { + SessionUpdatePollOutcome::ExitAfterSave + } else { + SessionUpdatePollOutcome::Redraw + }; + } if chat_state.core_session_id != pending.session_id { if let SessionUpdateApplyOutcome::SessionUpdateFailed(error) = &outcome { tracing::error!( @@ -778,16 +823,19 @@ impl ChatMode { return SessionUpdatePollOutcome::Redraw; } let applied = match &pending.kind { - PendingSessionUpdateKind::Mode { mode_id } => { + PendingSessionOperationKind::Mode { mode_id } => { apply_agent_mode_feedback(&mut self.agent_type, chat_state, mode_id, outcome) } - PendingSessionUpdateKind::Model { + PendingSessionOperationKind::Model { model_id, display_name, } => apply_model_selection_feedback(chat_state, display_name, model_id, outcome), - PendingSessionUpdateKind::Rename { session_name } => { + PendingSessionOperationKind::Rename { session_name } => { apply_session_rename_feedback(chat_state, session_name, outcome) } + PendingSessionOperationKind::Delete { .. } => { + unreachable!("session deletion is handled before current-session feedback") + } }; if applied { chat_view.set_status(Some(format!( @@ -813,7 +861,7 @@ impl ChatMode { if applied && matches!( &pending.kind, - PendingSessionUpdateKind::Mode { mode_id } if mode_id == "HarmonyOSDev" + PendingSessionOperationKind::Mode { mode_id } if mode_id == "HarmonyOSDev" ) { let deveco_home = std::env::var("DEVECO_HOME").ok(); @@ -846,7 +894,7 @@ fn session_update_unavailable_message(setting_name: &str, is_processing: bool) - if is_processing { format!("{setting_name} cannot be changed during the current turn.") } else { - "A current session update is already in progress. Please wait.".to_string() + "A Session operation is already in progress. Please wait.".to_string() } } diff --git a/src/apps/cli/src/modes/chat/sessions.rs b/src/apps/cli/src/modes/chat/sessions.rs index e2d120f7bb..3cae71d3da 100644 --- a/src/apps/cli/src/modes/chat/sessions.rs +++ b/src/apps/cli/src/modes/chat/sessions.rs @@ -119,12 +119,12 @@ impl ChatMode { rt_handle: &tokio::runtime::Handle, ) { if self - .pending_session_update + .pending_session_operation .as_ref() .is_some_and(|pending| pending.session_id == chat_state.core_session_id) { chat_view.set_status(Some( - "Waiting for the current session update to finish before sending.".to_string(), + "Waiting for the pending Session operation to finish before sending.".to_string(), )); return; } @@ -213,47 +213,66 @@ impl ChatMode { chat_view.show_session_selector( session_items, Some(current_session_id), - !self.agent.is_shared(), + session_delete_allowed( + false, + self.agent.is_shared(), + chat_state.is_processing, + self.pending_session_operation.is_some(), + ), ); } /// Handle session deletion from the session selector fn handle_session_delete( - &self, + &mut self, item: &SessionItem, chat_view: &mut ChatView, chat_state: &mut ChatState, rt_handle: &tokio::runtime::Handle, ) { - if self.agent.is_shared() { - chat_view.set_status(Some(format!( - "Session deletion is unavailable in Shared TUI preview. {SHARED_TUI_EMBEDDED_HANDOFF}; then run `bitfun sessions delete`" - ))); + let deleting_current_session = item.session_id == chat_state.core_session_id; + if deleting_current_session { + chat_view.set_status(Some("Cannot delete the active session".to_string())); return; } - // Prevent deleting the currently active session - if item.session_id == chat_state.core_session_id { - chat_view.set_status(Some("Cannot delete the active session".to_string())); + if !session_delete_allowed( + deleting_current_session, + self.agent.is_shared(), + chat_state.is_processing, + self.pending_session_operation.is_some(), + ) { + let message = if self.pending_session_operation.is_some() { + "Another Session operation is already in progress. Please wait." + } else { + "Session deletion cannot start during the current Turn in Shared TUI." + }; + chat_view.set_status(Some(message.to_string())); return; } let agent = self.agent.clone(); - let sid = item.session_id.clone(); - - let result = tokio::task::block_in_place(|| { - rt_handle.block_on(async { agent.delete_session(&sid).await }) + let session_id = item.session_id.clone(); + let task_session_id = session_id.clone(); + let session_name = item.session_name.clone(); + chat_view.hide_session_selector(); + chat_view.set_status(Some(format!("Deleting session {session_name}..."))); + let handle = rt_handle.spawn(async move { agent.delete_session(&task_session_id).await }); + self.pending_session_operation = Some(PendingSessionOperation { + session_id, + kind: PendingSessionOperationKind::Delete { session_name }, + started_at: Instant::now(), + slow_notice_shown: false, + exit_warning_shown: false, + handle, }); - - match result { - Ok(()) => { - chat_view.session_selector_remove_item(&item.session_id); - chat_view.set_status(Some(format!("Session deleted: {}", item.session_name))); - tracing::info!("Deleted session: {}", item.session_id); - } - Err(e) => { - chat_view.set_status(Some(format!("Failed to delete session: {}", e))); - tracing::error!("Failed to delete session: {}", e); - } - } } } + +fn session_delete_allowed( + deleting_current_session: bool, + shared_tui: bool, + current_turn_active: bool, + operation_pending: bool, +) -> bool { + !deleting_current_session && !operation_pending && (!shared_tui || !current_turn_active) +} diff --git a/src/apps/cli/src/modes/chat/tests.rs b/src/apps/cli/src/modes/chat/tests.rs index 2691dbfde2..77eba10a0d 100644 --- a/src/apps/cli/src/modes/chat/tests.rs +++ b/src/apps/cli/src/modes/chat/tests.rs @@ -19,10 +19,11 @@ mod tests { native_command_reconfirmation_is_required, native_hook_help_text, parse_external_agent_review_action, parse_external_control_action, parse_external_tool_review_action, parse_hook_management_action, parse_reload_invocation, - parse_reload_target, pending_session_update_blocks_runtime_action, + parse_reload_target, pending_session_operation_blocks_runtime_action, previous_session_update_status, render_external_hook_catalog, render_native_hook_overview, requested_session_name, retain_selected_native_command_for_input, selected_command_prefill, - session_command_help_note, session_update_allowed, session_update_blocks_typed_submission, + session_command_help_note, session_delete_allowed, session_delete_feedback, + session_update_allowed, session_update_blocks_typed_submission, session_update_completion_should_exit, shared_session_change_is_blocked, CommandRoute, ExternalAgentReviewAction, ExternalControlUiAction, ExternalSourceConflictPreferences, ExternalToolReviewAction, HookManagementAction, SessionUpdateApplyOutcome, @@ -1588,44 +1589,93 @@ mod tests { } #[test] - fn pending_session_update_routes_commands_to_their_action_guards() { + fn shared_session_delete_is_available_only_when_idle_and_no_operation_is_pending() { + assert!(session_delete_allowed(false, true, false, false)); + assert!(!session_delete_allowed(true, true, false, false)); + assert!(!session_delete_allowed(false, true, true, false)); + assert!(!session_delete_allowed(false, true, false, true)); + + // Embedded deletion keeps its existing ability to delete another + // Session while the current Session is running a Turn. + assert!(session_delete_allowed(false, false, true, false)); + } + + #[test] + fn session_delete_removes_the_item_only_after_runtime_confirmation() { + let (remove, status) = session_delete_feedback( + "Old session", + &SessionUpdateApplyOutcome::SessionUpdateFailed("session in use".to_string()), + ); + assert!(!remove); + assert!(status.contains("session in use")); + + let (remove, status) = + session_delete_feedback("Old session", &SessionUpdateApplyOutcome::Applied); + assert!(remove); + assert_eq!(status, "Session deleted: Old session"); + + let (remove, status) = session_delete_feedback( + "Old session", + &SessionUpdateApplyOutcome::OutcomeUnknown("request timed out".to_string()), + ); + assert!(!remove); + assert!(status.contains("unknown outcome")); + assert!(status.contains("closing")); + } + + #[test] + fn chat_session_delete_reuses_the_existing_async_session_slot() { + let source = include_str!("sessions.rs").replace("\r\n", "\n"); + let delete = source + .split_once("fn handle_session_delete(") + .expect("delete handler") + .1; + + assert!(delete.contains("rt_handle.spawn")); + assert!(delete.contains("pending_session_operation = Some(PendingSessionOperation")); + assert!(!delete.contains("block_in_place")); + assert!(!source.contains("PendingSessionDelete")); + } + + #[test] + fn pending_session_operation_routes_commands_to_their_action_guards() { assert!(session_update_blocks_typed_submission(true, "continue")); assert!(!session_update_blocks_typed_submission(true, "/new")); assert!(!session_update_blocks_typed_submission(true, "/sessions")); assert!(!session_update_blocks_typed_submission(true, "/exit")); assert!(!session_update_blocks_typed_submission(false, "continue")); - assert!(pending_session_update_blocks_runtime_action( + assert!(pending_session_operation_blocks_runtime_action( true, true, ActionHandler::Sessions, )); - assert!(pending_session_update_blocks_runtime_action( + assert!(pending_session_operation_blocks_runtime_action( true, true, ActionHandler::Init, )); - assert!(pending_session_update_blocks_runtime_action( + assert!(pending_session_operation_blocks_runtime_action( true, true, ActionHandler::RenameSession, )); - assert!(!pending_session_update_blocks_runtime_action( + assert!(!pending_session_operation_blocks_runtime_action( true, true, ActionHandler::Exit, )); - assert!(!pending_session_update_blocks_runtime_action( + assert!(!pending_session_operation_blocks_runtime_action( true, true, ActionHandler::OpenAgentSelector, )); - assert!(!pending_session_update_blocks_runtime_action( + assert!(!pending_session_operation_blocks_runtime_action( false, true, ActionHandler::Sessions, )); - assert!(!pending_session_update_blocks_runtime_action( + assert!(!pending_session_operation_blocks_runtime_action( true, false, ActionHandler::Sessions, diff --git a/src/apps/cli/src/shared_runtime.rs b/src/apps/cli/src/shared_runtime.rs index dcd6feeee1..933f7dca2c 100644 --- a/src/apps/cli/src/shared_runtime.rs +++ b/src/apps/cli/src/shared_runtime.rs @@ -1,9 +1,9 @@ use anyhow::{anyhow, Context, Result}; use async_trait::async_trait; use bitfun_agent_runtime::sdk::{ - AgentRuntime, AgentSessionRenameRequest, AgentSessionRestoreRequest, AgentUserAnswersRequest, - DialogSubmitOutcome, PermissionRequest, PermissionRequestEvent, PortErrorKind, RuntimeError, - SessionTranscriptRequest, + AgentRuntime, AgentSessionDeleteRequest, AgentSessionRenameRequest, AgentSessionRestoreRequest, + AgentUserAnswersRequest, DialogSubmitOutcome, PermissionRequest, PermissionRequestEvent, + PortErrorKind, RuntimeError, SessionTranscriptRequest, }; use bitfun_agent_runtime_ipc::{ DiscoveryStore, RuntimeInstanceIdentity, RuntimeIpcClient, RuntimeIpcError, @@ -265,6 +265,10 @@ impl RuntimeIpcRequestHandler for SharedRuntimeHandler { pending_permissions, }) } + RuntimeIpcOperation::DeleteSession { session_id } => { + delete_owned_session(&self.runtime, &self.workspace, session_id).await?; + Ok(RuntimeIpcOperationResult::Unit) + } RuntimeIpcOperation::UpdateSessionMode { request } => { self.runtime .update_session_mode(request) @@ -422,6 +426,22 @@ async fn rename_owned_session( .map_err(runtime_ipc_error) } +async fn delete_owned_session( + runtime: &AgentRuntime, + workspace: &Path, + session_id: String, +) -> std::result::Result<(), RuntimeIpcError> { + runtime + .delete_session(AgentSessionDeleteRequest { + workspace_path: workspace.to_string_lossy().to_string(), + session_id, + remote_connection_id: None, + remote_ssh_host: None, + }) + .await + .map_err(runtime_ipc_error) +} + fn subscribe_session_events( events: &SessionEventSenders, available: &watch::Sender, @@ -982,6 +1002,9 @@ fn runtime_ipc_error(error: RuntimeError) -> RuntimeIpcError { RuntimeError::Port(port_error) if port_error.kind == PortErrorKind::OutcomeUnknown => { RuntimeIpcErrorCode::OutcomeUnknown } + RuntimeError::Port(port_error) if port_error.kind == PortErrorKind::NotFound => { + RuntimeIpcErrorCode::NotFound + } _ => RuntimeIpcErrorCode::Unavailable, }; RuntimeIpcError { @@ -1010,11 +1033,11 @@ fn core_ipc_error(error: bitfun_core::util::errors::BitFunError) -> RuntimeIpcEr #[cfg(test)] mod tests { use super::{ - await_permission_route, connect_existing, index_user_question, invalidate_event_stream, - owned_session_rename_request, permission_event_session, permission_targets_session, - project_subagent_link_route, project_user_question_route, publish_event, - rename_owned_session, route_agent_event, runtime_ipc_error, subscribe_session_events, - SessionEventSenders, EVENT_BUFFER, + await_permission_route, connect_existing, delete_owned_session, index_user_question, + invalidate_event_stream, owned_session_rename_request, permission_event_session, + permission_targets_session, project_subagent_link_route, project_user_question_route, + publish_event, rename_owned_session, route_agent_event, runtime_ipc_error, + subscribe_session_events, SessionEventSenders, EVENT_BUFFER, }; use bitfun_agent_runtime::sdk::{ AgentRuntimeBuilder, AgentSessionCreateRequest, AgentSessionCreateResult, @@ -1034,6 +1057,7 @@ mod tests { #[derive(Default)] struct RecordingSessionPort { + delete_requests: Mutex>, rename_requests: Mutex>, } @@ -1077,7 +1101,8 @@ mod tests { Ok(Vec::new()) } - async fn delete_session(&self, _request: AgentSessionDeleteRequest) -> PortResult<()> { + async fn delete_session(&self, request: AgentSessionDeleteRequest) -> PortResult<()> { + self.delete_requests.lock().unwrap().push(request); Ok(()) } @@ -1130,6 +1155,16 @@ mod tests { assert_eq!(error.code, RuntimeIpcErrorCode::OutcomeUnknown); } + #[test] + fn missing_runtime_sessions_keep_their_ipc_error_category() { + let error = runtime_ipc_error(RuntimeError::Port(PortError::new( + PortErrorKind::NotFound, + "Session not found: session-1", + ))); + + assert_eq!(error.code, RuntimeIpcErrorCode::NotFound); + } + #[test] fn shared_rename_uses_the_server_workspace_and_no_remote_identity() { let request = owned_session_rename_request( @@ -1188,6 +1223,39 @@ mod tests { ); } + #[tokio::test] + async fn embedded_and_shared_delete_reach_the_same_runtime_owner() { + let workspace = tempfile::tempdir().expect("workspace"); + let canonical_workspace = dunce::canonicalize(workspace.path()).expect("workspace path"); + let workspace_path = canonical_workspace.to_string_lossy().to_string(); + let port = Arc::new(RecordingSessionPort::default()); + let runtime = AgentRuntimeBuilder::new() + .with_submission_port(port.clone()) + .with_session_management_port(port.clone()) + .build() + .expect("runtime"); + let expected = AgentSessionDeleteRequest { + workspace_path, + session_id: "session-2".to_string(), + remote_connection_id: None, + remote_ssh_host: None, + }; + + runtime + .delete_session(expected.clone()) + .await + .expect("embedded delete"); + + delete_owned_session(&runtime, &canonical_workspace, "session-2".to_string()) + .await + .expect("shared delete"); + + assert_eq!( + port.delete_requests.lock().unwrap().as_slice(), + &[expected.clone(), expected] + ); + } + #[tokio::test] async fn existing_runtime_connection_errors_are_not_hidden_as_absence() { let root = tempfile::tempdir().unwrap(); diff --git a/src/apps/cli/src/ui/startup.rs b/src/apps/cli/src/ui/startup.rs index eaf67c6639..b2a98450fc 100644 --- a/src/apps/cli/src/ui/startup.rs +++ b/src/apps/cli/src/ui/startup.rs @@ -1397,17 +1397,10 @@ impl StartupPage { }) .collect(); - self.session_selector - .show(session_items, None, !self.agent.is_shared()); + self.session_selector.show(session_items, None, true); } fn handle_session_delete(&mut self, item: &SessionItem) { - if self.agent.is_shared() { - self.status = Some(format!( - "Session deletion is unavailable in Shared TUI preview. {SHARED_TUI_EMBEDDED_HANDOFF}; then run `bitfun sessions delete`" - )); - return; - } let agent = Arc::clone(&self.agent); let sid = item.session_id.clone(); diff --git a/src/crates/adapters/agent-runtime-ipc/AGENTS-CN.md b/src/crates/adapters/agent-runtime-ipc/AGENTS-CN.md index 819139dd59..b4ee8c78e5 100644 --- a/src/crates/adapters/agent-runtime-ipc/AGENTS-CN.md +++ b/src/crates/adapters/agent-runtime-ipc/AGENTS-CN.md @@ -15,7 +15,7 @@ ## 边界 - 只导出 CLI adapter 实际使用的 workspace-private API，且 crate 不得发布，也不得把 wire 作为 SDK 合同。 -- 封闭 operation 范围为 Health、Session list/create/restore（restore 结果包含 transcript）、当前 Session rename 和 Agent mode/model update、声明式上下文 reload、Turn submit/cancel、pending/respond Permission 和 UserInput answers。上下文 reload 可在活动 Turn 中执行，不改写该 Turn，并通过缓存保护保证下一条消息重新读取已失效的 instructions。断连 cleanup 属于内部生命周期，不是 detach operation。模型目录和默认值仍是 wire 之外的产品配置；禁止顺带加入 delete、fork、replay、observer、controller transfer、Tool/MCP/Hook 管理或其他产品配置。 +- 封闭 operation 范围为 Health、Session list/create/restore/delete（restore 结果包含 transcript）、当前 Session rename 和 Agent mode/model update、声明式上下文 reload、Turn submit/cancel、pending/respond Permission 和 UserInput answers。delete 只允许作用于未被任何 Client 控制的空闲 Session。上下文 reload 可在活动 Turn 中执行，不改写该 Turn，并通过缓存保护保证下一条消息重新读取已失效的 instructions。断连 cleanup 属于内部生命周期，不是 detach operation。模型目录和默认值仍是 wire 之外的产品配置；禁止顺带加入 archive、fork、replay、observer、controller transfer、Tool/MCP/Hook 管理或其他产品配置。 - 可以复用稳定 Event、Product Domain 和 Runtime Port DTO。禁止依赖 `bitfun-core`、Agent Runtime 实现、SDK Host、services、Tauri、terminal、tool runtime 或远程 transport。 - 只使用 Windows Named Pipe 或 Unix Domain Socket；禁止 TCP、HTTP、WebSocket、浏览器访问或远程 fallback。 - 这是本机同用户隔离，不是沙箱。未来产品 composition 必须提供当前用户私有 runtime 目录。 diff --git a/src/crates/adapters/agent-runtime-ipc/AGENTS.md b/src/crates/adapters/agent-runtime-ipc/AGENTS.md index bb92c2e91f..8a79ddb0be 100644 --- a/src/crates/adapters/agent-runtime-ipc/AGENTS.md +++ b/src/crates/adapters/agent-runtime-ipc/AGENTS.md @@ -22,10 +22,11 @@ session controller leases, event delivery, connection bounds, and cleanup. It is - Export only the exact workspace-private API needed by the CLI adapter. Do not publish this crate or expose its wire as an SDK contract. -- The closed operation budget is Health, Session list/create/restore (including transcript), current-Session rename and Agent mode/model update, - declarative context reload, Turn submit/cancel, pending/respond Permission, and UserInput answers. Context reload may run during an active - Turn, does not rewrite that Turn, and guards the cache so the next message reads invalidated instructions. Disconnect cleanup is internal lifecycle, not a detach operation. - Model catalogs and defaults remain product configuration outside this wire. Do not add delete, fork, replay, observer, +- The closed operation budget is Health, Session list/create/restore/delete (including transcript on restore), current-Session rename and Agent mode/model update, + declarative context reload, Turn submit/cancel, pending/respond Permission, and UserInput answers. Delete is limited to an idle Session not controlled by any client. + Context reload may run during an active Turn, does not rewrite that Turn, and guards the cache so the next message reads invalidated instructions. + Disconnect cleanup is internal lifecycle, not a detach operation. + Model catalogs and defaults remain product configuration outside this wire. Do not add archive, fork, replay, observer, controller transfer, Tool/MCP/Hook management, or other product configuration incidentally. - Stable Event, Product Domain, and Runtime Port DTOs may be reused. Do not depend on `bitfun-core`, Agent Runtime implementations, SDK Host, services, diff --git a/src/crates/adapters/agent-runtime-ipc/src/operation.rs b/src/crates/adapters/agent-runtime-ipc/src/operation.rs index 4c138f1d15..bf31b4492f 100644 --- a/src/crates/adapters/agent-runtime-ipc/src/operation.rs +++ b/src/crates/adapters/agent-runtime-ipc/src/operation.rs @@ -47,6 +47,9 @@ pub enum RuntimeIpcOperation { RestoreSession { request: RuntimeSessionRestoreRequest, }, + DeleteSession { + session_id: String, + }, UpdateSessionMode { request: AgentSessionModeUpdateRequest, }, @@ -82,6 +85,7 @@ impl RuntimeIpcOperation { pub fn session_id(&self) -> Option<&str> { match self { Self::RestoreSession { request } => Some(&request.session_id), + Self::DeleteSession { session_id } => Some(session_id), Self::UpdateSessionMode { request } => Some(&request.session_id), Self::UpdateSessionModel { request } => Some(&request.session_id), Self::RenameSession { request } => Some(&request.session_id), @@ -95,19 +99,70 @@ impl RuntimeIpcOperation { } } - pub fn requires_controller(&self) -> bool { - matches!( - self, + pub(crate) fn rules(&self) -> RuntimeIpcOperationRules { + use RuntimeIpcSessionRequirement::{ + AttachExisting, CurrentController, None, UncontrolledTarget, + }; + + match self { + Self::Health | Self::ListSessions { .. } => { + RuntimeIpcOperationRules::new(None, false, false, false) + } + Self::CreateSession { .. } => RuntimeIpcOperationRules::new(None, true, true, true), + Self::RestoreSession { .. } => { + RuntimeIpcOperationRules::new(AttachExisting, true, true, true) + } + Self::DeleteSession { .. } => { + RuntimeIpcOperationRules::new(UncontrolledTarget, true, true, true) + } Self::UpdateSessionMode { .. } - | Self::UpdateSessionModel { .. } - | Self::RenameSession { .. } - | Self::ReloadSessionContext { .. } - | Self::SubmitTurn { .. } - | Self::CancelTurn { .. } - | Self::PendingPermissions { .. } - | Self::RespondPermission { .. } - | Self::SubmitUserAnswers { .. } - ) + | Self::UpdateSessionModel { .. } + | Self::RenameSession { .. } + | Self::SubmitTurn { .. } => { + RuntimeIpcOperationRules::new(CurrentController, true, false, true) + } + Self::ReloadSessionContext { .. } + | Self::CancelTurn { .. } + | Self::RespondPermission { .. } + | Self::SubmitUserAnswers { .. } => { + RuntimeIpcOperationRules::new(CurrentController, false, false, true) + } + Self::PendingPermissions { .. } => { + RuntimeIpcOperationRules::new(CurrentController, false, false, false) + } + } + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum RuntimeIpcSessionRequirement { + None, + CurrentController, + AttachExisting, + UncontrolledTarget, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) struct RuntimeIpcOperationRules { + pub(crate) session_requirement: RuntimeIpcSessionRequirement, + pub(crate) requires_idle: bool, + pub(crate) serializes_session_selection: bool, + pub(crate) side_effecting: bool, +} + +impl RuntimeIpcOperationRules { + const fn new( + session_requirement: RuntimeIpcSessionRequirement, + requires_idle: bool, + serializes_session_selection: bool, + side_effecting: bool, + ) -> Self { + Self { + session_requirement, + requires_idle, + serializes_session_selection, + side_effecting, + } } } @@ -146,3 +201,74 @@ pub enum RuntimeIpcOperationResult { requests: Vec, }, } + +#[cfg(test)] +mod tests { + use super::{RuntimeIpcOperation, RuntimeIpcSessionRequirement, RuntimeSessionRestoreRequest}; + use bitfun_runtime_ports::{AgentContextReloadRequest, AgentContextReloadTarget}; + + #[test] + fn delete_rules_are_fail_closed_for_shared_session_selection() { + let rules = RuntimeIpcOperation::DeleteSession { + session_id: "session-2".to_string(), + } + .rules(); + + assert_eq!( + rules.session_requirement, + RuntimeIpcSessionRequirement::UncontrolledTarget + ); + assert!(rules.requires_idle); + assert!(rules.serializes_session_selection); + assert!(rules.side_effecting); + } + + #[test] + fn reload_rules_preserve_active_turn_semantics_after_rule_consolidation() { + let rules = RuntimeIpcOperation::ReloadSessionContext { + request: AgentContextReloadRequest { + session_id: "session-1".to_string(), + target: AgentContextReloadTarget::All, + }, + } + .rules(); + + assert_eq!( + rules.session_requirement, + RuntimeIpcSessionRequirement::CurrentController + ); + assert!(!rules.requires_idle); + assert!(!rules.serializes_session_selection); + assert!(rules.side_effecting); + } + + #[test] + fn restore_and_pending_permission_rules_preserve_existing_behavior() { + let restore = RuntimeIpcOperation::RestoreSession { + request: RuntimeSessionRestoreRequest { + workspace_path: "D:/workspace/project".to_string(), + session_id: "session-2".to_string(), + }, + } + .rules(); + assert_eq!( + restore.session_requirement, + RuntimeIpcSessionRequirement::AttachExisting + ); + assert!(restore.requires_idle); + assert!(restore.serializes_session_selection); + assert!(restore.side_effecting); + + let pending = RuntimeIpcOperation::PendingPermissions { + session_id: "session-1".to_string(), + } + .rules(); + assert_eq!( + pending.session_requirement, + RuntimeIpcSessionRequirement::CurrentController + ); + assert!(!pending.requires_idle); + assert!(!pending.serializes_session_selection); + assert!(!pending.side_effecting); + } +} diff --git a/src/crates/adapters/agent-runtime-ipc/src/protocol.rs b/src/crates/adapters/agent-runtime-ipc/src/protocol.rs index 4dd1dd536e..8036245015 100644 --- a/src/crates/adapters/agent-runtime-ipc/src/protocol.rs +++ b/src/crates/adapters/agent-runtime-ipc/src/protocol.rs @@ -5,7 +5,7 @@ use crate::{RuntimeIpcOperation, RuntimeIpcOperationResult}; use bitfun_events::AgenticEventEnvelope; use bitfun_product_domains::tool_permissions::PermissionRequestEvent; -pub const PROTOCOL_VERSION: u32 = 6; +pub const PROTOCOL_VERSION: u32 = 7; #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] #[serde(tag = "type", rename_all = "snake_case", deny_unknown_fields)] @@ -129,6 +129,7 @@ pub enum RuntimeIpcErrorCode { IncompatibleProtocol, WrongInstance, FrameTooLarge, + NotFound, SessionInUse, ControllerRequired, SessionMismatch, diff --git a/src/crates/adapters/agent-runtime-ipc/src/server.rs b/src/crates/adapters/agent-runtime-ipc/src/server.rs index c0c59d4230..41d3d9b81a 100644 --- a/src/crates/adapters/agent-runtime-ipc/src/server.rs +++ b/src/crates/adapters/agent-runtime-ipc/src/server.rs @@ -1,3 +1,4 @@ +use crate::operation::RuntimeIpcSessionRequirement; use crate::{ read_frame, serialize_frame_with_limit, write_frame_with_limit, write_serialized_frame_with_limit, DiscoveryRecord, DiscoveryStore, InitializeResult, @@ -411,35 +412,23 @@ async fn run_initialized_connection( continue; } - if active_turn_id.is_some() - && matches!( - operation, - RuntimeIpcOperation::SubmitTurn { .. } - | RuntimeIpcOperation::RestoreSession { .. } - | RuntimeIpcOperation::CreateSession { .. } - | RuntimeIpcOperation::UpdateSessionMode { .. } - | RuntimeIpcOperation::UpdateSessionModel { .. } - | RuntimeIpcOperation::RenameSession { .. } - ) - { + let rules = operation.rules(); + if active_turn_id.is_some() && rules.requires_idle { send_error( stream, config.request_timeout, Some(request_id), RuntimeIpcErrorCode::SessionInUse, - "finish or cancel the active turn before changing the controlled session, name, agent mode, or model", + "finish or cancel the active turn before starting this session operation", ) .await?; continue; } // Serialize attachment so a newly visible Session cannot be claimed - // before its generated ID returns to the creating connection. - let _attachment_guard = if matches!( - operation, - RuntimeIpcOperation::CreateSession { .. } - | RuntimeIpcOperation::RestoreSession { .. } - ) { + // before its generated ID returns to the creating connection, or + // deleted while another connection is attaching it. + let _attachment_guard = if rules.serializes_session_selection { Some(config.attachment_gate.lock().await) } else { None @@ -476,7 +465,7 @@ async fn run_initialized_connection( } _ => None, }; - let side_effecting = operation_has_side_effects(&operation); + let side_effecting = rules.side_effecting; let result = tokio::time::timeout(config.request_timeout, handler.execute(operation)).await; let result = match result { @@ -671,42 +660,27 @@ async fn wait_until_unavailable(availability: Option<&mut watch::Receiver> } } -fn operation_has_side_effects(operation: &RuntimeIpcOperation) -> bool { - matches!( - operation, - RuntimeIpcOperation::CreateSession { .. } - | RuntimeIpcOperation::RestoreSession { .. } - | RuntimeIpcOperation::UpdateSessionMode { .. } - | RuntimeIpcOperation::UpdateSessionModel { .. } - | RuntimeIpcOperation::RenameSession { .. } - | RuntimeIpcOperation::ReloadSessionContext { .. } - | RuntimeIpcOperation::SubmitTurn { .. } - | RuntimeIpcOperation::CancelTurn { .. } - | RuntimeIpcOperation::RespondPermission { .. } - | RuntimeIpcOperation::SubmitUserAnswers { .. } - ) -} - fn prepare_operation( config: &ConnectionConfig, connection_id: &str, operation: &RuntimeIpcOperation, ) -> Result { - if matches!(operation, RuntimeIpcOperation::CreateSession { .. }) { - return Ok(LeaseTransition::Unchanged); - } - - if let RuntimeIpcOperation::RestoreSession { request } = operation { - return config.leases.switch(connection_id, &request.session_id); - } - - if operation.requires_controller() { - config.leases.validate( + let session_id = operation.session_id(); + match operation.rules().session_requirement { + RuntimeIpcSessionRequirement::None => {} + RuntimeIpcSessionRequirement::CurrentController => config.leases.validate( connection_id, - operation - .session_id() - .expect("controller operations are session scoped"), - )?; + session_id.expect("controller operations are session scoped"), + )?, + RuntimeIpcSessionRequirement::AttachExisting => { + return config.leases.switch( + connection_id, + session_id.expect("attachment operations are session scoped"), + ); + } + RuntimeIpcSessionRequirement::UncontrolledTarget => config.leases.validate_uncontrolled( + session_id.expect("uncontrolled-target operations are session scoped"), + )?, } Ok(LeaseTransition::Unchanged) } diff --git a/src/crates/adapters/agent-runtime-ipc/src/session_lease.rs b/src/crates/adapters/agent-runtime-ipc/src/session_lease.rs index f01067a050..5b07663477 100644 --- a/src/crates/adapters/agent-runtime-ipc/src/session_lease.rs +++ b/src/crates/adapters/agent-runtime-ipc/src/session_lease.rs @@ -112,6 +112,20 @@ impl RuntimeSessionLeases { } } + pub(crate) fn validate_uncontrolled(&self, session_id: &str) -> Result<(), RuntimeIpcError> { + let state = self + .state + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + if state.by_session.contains_key(session_id) { + return Err(error( + RuntimeIpcErrorCode::SessionInUse, + "session already has an active Shared TUI controller", + )); + } + Ok(()) + } + pub(crate) fn release_connection(&self, connection_id: &str) -> Option { let mut state = self .state diff --git a/src/crates/adapters/agent-runtime-ipc/src/tests/protocol_contracts.rs b/src/crates/adapters/agent-runtime-ipc/src/tests/protocol_contracts.rs index 59afd1d21b..5977c7b2d1 100644 --- a/src/crates/adapters/agent-runtime-ipc/src/tests/protocol_contracts.rs +++ b/src/crates/adapters/agent-runtime-ipc/src/tests/protocol_contracts.rs @@ -1,7 +1,8 @@ +use crate::operation::RuntimeIpcSessionRequirement; use crate::{ - serialize_frame_with_limit, InitializeRequest, RuntimeIpcFrame, RuntimeIpcOperation, - RuntimeSessionRenameRequest, RuntimeUserAnswersRequest, MAX_REQUEST_FRAME_BYTES, - PROTOCOL_VERSION, + serialize_frame_with_limit, InitializeRequest, RuntimeIpcError, RuntimeIpcErrorCode, + RuntimeIpcFrame, RuntimeIpcOperation, RuntimeSessionRenameRequest, RuntimeUserAnswersRequest, + MAX_REQUEST_FRAME_BYTES, PROTOCOL_VERSION, }; use bitfun_product_domains::tool_permissions::PermissionReply; @@ -85,7 +86,10 @@ fn protocol_round_trips_the_reviewed_session_mode_operation() { assert_eq!(decoded, operation); assert_eq!(decoded.session_id(), Some("session-1")); - assert!(decoded.requires_controller()); + assert_eq!( + decoded.rules().session_requirement, + RuntimeIpcSessionRequirement::CurrentController + ); } #[test] @@ -106,12 +110,15 @@ fn protocol_round_trips_the_reviewed_session_model_operation() { assert_eq!(decoded, operation); assert_eq!(decoded.session_id(), Some("session-1")); - assert!(decoded.requires_controller()); + assert_eq!( + decoded.rules().session_requirement, + RuntimeIpcSessionRequirement::CurrentController + ); } #[test] fn protocol_round_trips_the_current_session_rename_operation() { - assert_eq!(PROTOCOL_VERSION, 6); + assert_eq!(PROTOCOL_VERSION, 7); let operation = RuntimeIpcOperation::RenameSession { request: RuntimeSessionRenameRequest { @@ -136,7 +143,42 @@ fn protocol_round_trips_the_current_session_rename_operation() { assert_eq!(decoded, operation); assert_eq!(decoded.session_id(), Some("session-1")); - assert!(decoded.requires_controller()); + assert_eq!( + decoded.rules().session_requirement, + RuntimeIpcSessionRequirement::CurrentController + ); +} + +#[test] +fn protocol_round_trips_session_delete_and_not_found() { + let operation = RuntimeIpcOperation::DeleteSession { + session_id: "session-2".to_string(), + }; + let encoded = serde_json::to_value(&operation).expect("serialize session delete"); + assert_eq!( + encoded, + json!({ + "operation": "delete_session", + "sessionId": "session-2" + }) + ); + let decoded: RuntimeIpcOperation = + serde_json::from_value(encoded).expect("deserialize session delete"); + assert_eq!(decoded, operation); + assert_eq!(decoded.session_id(), Some("session-2")); + + let frame = RuntimeIpcFrame::Error { + request_id: Some(7), + error: RuntimeIpcError { + code: RuntimeIpcErrorCode::NotFound, + message: "session not found".to_string(), + }, + }; + let encoded = serde_json::to_value(&frame).expect("serialize not-found error"); + assert_eq!(encoded["error"]["code"], "not_found"); + let decoded: RuntimeIpcFrame = + serde_json::from_value(encoded).expect("deserialize not-found error"); + assert_eq!(decoded, frame); } #[test] @@ -157,7 +199,14 @@ fn protocol_round_trips_context_reload_as_a_controller_operation() { assert_eq!(decoded, operation); assert_eq!(decoded.session_id(), Some("session-1")); - assert!(decoded.requires_controller()); + let rules = decoded.rules(); + assert_eq!( + rules.session_requirement, + RuntimeIpcSessionRequirement::CurrentController + ); + assert!(!rules.requires_idle); + assert!(!rules.serializes_session_selection); + assert!(rules.side_effecting); } #[test] diff --git a/src/crates/adapters/agent-runtime-ipc/src/tests/shared_controller.rs b/src/crates/adapters/agent-runtime-ipc/src/tests/shared_controller.rs index ee725d480a..e248516e8b 100644 --- a/src/crates/adapters/agent-runtime-ipc/src/tests/shared_controller.rs +++ b/src/crates/adapters/agent-runtime-ipc/src/tests/shared_controller.rs @@ -86,6 +86,7 @@ struct FakeHandler { mode_delay: Option, model_delay: Option, rename_delay: Option, + delete_delay: Option, submit_delay: Option, settle_cancel: bool, events: broadcast::Sender, @@ -109,6 +110,7 @@ impl Default for FakeHandler { mode_delay: None, model_delay: None, rename_delay: None, + delete_delay: None, submit_delay: None, settle_cancel: true, events, @@ -194,6 +196,11 @@ impl RuntimeIpcRequestHandler for FakeHandler { tokio::time::sleep(delay).await; } } + if matches!(operation, RuntimeIpcOperation::DeleteSession { .. }) { + if let Some(delay) = self.delete_delay { + tokio::time::sleep(delay).await; + } + } match operation { RuntimeIpcOperation::RestoreSession { request } => Ok(restored(&request.session_id)), RuntimeIpcOperation::SubmitTurn { request } => { @@ -533,6 +540,12 @@ fn rename_operation(session_id: &str, session_name: &str) -> RuntimeIpcOperation } } +fn delete_operation(session_id: &str) -> RuntimeIpcOperation { + RuntimeIpcOperation::DeleteSession { + session_id: session_id.to_string(), + } +} + fn server_config() -> RuntimeIpcServerConfig { RuntimeIpcServerConfig { server_version: "shared-controller-test".to_string(), @@ -986,6 +999,90 @@ async fn rename_requires_the_controlled_idle_session() { server.finish().await; } +#[tokio::test] +async fn delete_requires_an_uncontrolled_target_and_an_idle_connection() { + let handler = Arc::new(FakeHandler::default()); + let server = TestServer::start(server_config(), handler.clone()).await; + let mut first = server.connect("delete-controller").await; + let mut second = server.connect("delete-other").await; + + expect_response( + &mut first, + 2, + restore_operation(server.workspace.path(), "session-a"), + ) + .await; + expect_error( + &mut first, + 3, + delete_operation("session-a"), + RuntimeIpcErrorCode::SessionInUse, + ) + .await; + expect_error( + &mut second, + 2, + delete_operation("session-a"), + RuntimeIpcErrorCode::SessionInUse, + ) + .await; + expect_response(&mut second, 3, delete_operation("session-b")).await; + + expect_response( + &mut first, + 4, + submit_operation(server.workspace.path(), "session-a", "turn-a"), + ) + .await; + expect_error( + &mut first, + 5, + delete_operation("session-c"), + RuntimeIpcErrorCode::SessionInUse, + ) + .await; + + let deletes = handler + .calls + .lock() + .expect("calls") + .iter() + .filter(|operation| matches!(operation, RuntimeIpcOperation::DeleteSession { .. })) + .count(); + assert_eq!( + deletes, 1, + "only the uncontrolled idle delete reaches Runtime" + ); + + drop(first); + drop(second); + server.finish().await; +} + +#[tokio::test] +async fn timed_out_delete_reports_unknown_outcome_and_closes_the_connection() { + let handler = Arc::new(FakeHandler { + delete_delay: Some(Duration::from_millis(100)), + ..FakeHandler::default() + }); + let mut config = server_config(); + config.request_timeout = Duration::from_millis(20); + let server = TestServer::start(config, handler).await; + let mut client = server.connect("delete-timeout").await; + + expect_error( + &mut client, + 2, + delete_operation("session-b"), + RuntimeIpcErrorCode::OutcomeUnknown, + ) + .await; + assert!(read_frame(&mut client).await.is_err()); + + drop(client); + server.finish().await; +} + #[tokio::test] async fn timed_out_rename_reports_unknown_outcome_and_closes_the_connection() { let handler = Arc::new(FakeHandler {