diff --git a/docs/architecture/extensions/external-ai-work-sources-design.md b/docs/architecture/extensions/external-ai-work-sources-design.md index f5c92b6b7..a3806fd27 100644 --- a/docs/architecture/extensions/external-ai-work-sources-design.md +++ b/docs/architecture/extensions/external-ai-work-sources-design.md @@ -12,7 +12,10 @@ OpenCode Prompt Command 适配器接入本地用户全局/项目来源；Desktop 交互式 TUI（ChatMode）可列出并执行 prompt-only Command。第二条纵向切片已让受支持的单文件 OpenCode `.js` standalone Tool 经静态 预览、来源/能力确认和同名冲突选择后进入现有 Tool Runtime；Desktop 与交互式 TUI（ChatMode）使用同一决策状态。第三条纵向 切片已把 OpenCode 全局/项目 Subagent 的安全子集通过独立 provider 契约接入现有 Subagent owner：首次启用与 -同名冲突使用非阻塞决策，fresh 调用固定不可变 generation，更新和撤下不会静默切换到同名实现。完整 +同名冲突使用非阻塞决策，fresh 调用固定不可变 generation，更新和撤下不会静默切换到同名实现。第四条纵向切片 +已把 OpenCode 用户/项目 MCP 的 local stdio 与 HTTPS remote 安全子集接入现有 MCP owner，沿用显式审批、冲突、 +工作区隔离和失败回推；现有 Skill Registry 另行展示来源、用户/项目作用域和固定优先级产生的覆盖结果，不并入上述 +可执行来源选择规则。完整 TypeScript/Bun、包依赖、package plugin、Codex/Claude Code 适配器、primary agent 替换和外部 Subagent 续接仍属于 后续阶段，不能因来源被识别就宣称已经可用。 @@ -60,7 +63,9 @@ TypeScript/Bun、包依赖、package plugin、Codex/Claude Code 适配器、prim ### 3.1 首次发现 -发现始终在后台进行。Desktop、交互式 TUI（ChatMode）和未来 Web 入口消费同一只读来源状态，但按宿主展示： +发现始终在后台进行。Desktop、交互式 TUI（ChatMode）和 Peer 控制界面消费事实所在 Host 的同一来源状态，但按 +宿主展示；Peer 控制界面只代理 Peer Host，不读取控制端同名来源。Server 当前只提供只读快照，未来 Web 入口必须 +通过已接入的 Host 能力消费，不能由浏览器扫描来源： ```text 已发现 OpenCode 工作内容 diff --git a/docs/architecture/extensions/opencode-extension-compatibility.md b/docs/architecture/extensions/opencode-extension-compatibility.md index 757404deb..2ad95c1d3 100644 --- a/docs/architecture/extensions/opencode-extension-compatibility.md +++ b/docs/architecture/extensions/opencode-extension-compatibility.md @@ -104,7 +104,7 @@ OpenCode，和 OpenCode 配置/插件进入 BitFun 是两个独立验收方向 | 独立 `tui.json/jsonc` | 融合现有能力 + 转换参数 | 未实现 | 可完整适配 | OC-R1 | 按 global、`OPENCODE_TUI_CONFIG`、project、`.opencode` 独立顺序加载，不能复用主配置优先级 | [TUI 来源](opencode-config-assets-adapter-design.md#32-tui-独立来源顺序) | | Rules / Instructions | 转换参数 | 未实现 | 可完整适配 | OC-R1 | R1 映射本地/已缓存内容；需要主动联网的远程 instruction 在 R2 通过归属模块保护后获取 | [声明式资产](opencode-config-assets-adapter-design.md#5-声明式资产映射) | | Agents / Modes | 融合现有能力 + 转换参数 | 部分实现：Subagent 安全子集 | 可主要适配 | OC-R1 | 已支持全局/项目 Markdown、JSON/JSONC 的 subagent/all、prompt、description、disable/hidden、精确模型与工具映射，并接入审批、冲突、更新、撤下和 fresh single-run Task；primary/mode、permission、variant/options、采样与续接明确降级 | [Agents 与 Skills](opencode-config-assets-adapter-design.md#52-agentsmodes-与-skills) | -| Skills | 转换参数 | 未实现 | 可完整适配 | OC-R2 | R1 发现/展示；R2 保留按需加载及 allow/deny/ask 顺序，可执行资源走 Skill 归属模块保护 | [Agents 与 Skills](opencode-config-assets-adapter-design.md#52-agentsmodes-与-skills) | +| Skills | 转换参数 | 部分实现：现有 Registry 发现与来源/覆盖展示 | 可完整适配 | OC-R2 | 已沿用现有根顺序发现 `.opencode` 等用户/项目 Skill，并在 GUI/TUI 展示来源、作用域和覆盖结果；尚未复现 OpenCode 按需加载、完整配置合并及 allow/deny/ask 顺序，也未并入外部来源策略 owner | [Agents 与 Skills](opencode-config-assets-adapter-design.md#52-agentsmodes-与-skills) | | References | 补基础能力 + 转换参数 | 未实现 | 可主要适配 | OC-R2 | R1 解析；R2 支持本地目录和 Git repository/branch/description/hidden，异步准备并接入 `@alias` | [声明式资产](opencode-config-assets-adapter-design.md#5-声明式资产映射) | | Commands | 补扩展接口 + 转换参数 | 部分实现：prompt-only 子集 | 可完整适配 | OC-R2 | 已支持全局/项目 JSON、JSONC、Markdown 命令、`$ARGUMENTS` 与位置参数，并接入动态命令目录、运行时刷新和显式冲突选择；`@file`、shell、Agent/model/variant/subtask 保持受限且不做部分执行 | [Commands](opencode-config-assets-adapter-design.md#53-commands) | | Models / Providers 配置 | 融合现有能力 | 未实现 | 可主要适配 | OC-R1 | 静态字段进入模型归属模块；动态模型、鉴权和请求头交给插件运行时 | [声明式资产](opencode-config-assets-adapter-design.md#5-声明式资产映射) | diff --git a/docs/architecture/extensions/plugin-runtime-host-design.md b/docs/architecture/extensions/plugin-runtime-host-design.md index 2b23c32af..ffc35f214 100644 --- a/docs/architecture/extensions/plugin-runtime-host-design.md +++ b/docs/architecture/extensions/plugin-runtime-host-design.md @@ -301,7 +301,9 @@ CLI 已允许在包文件缺失或损坏时清理残留启用状态，这是现 已经接入 prompt-only Command、受支持的单文件 `.js` standalone Tool、Subagent 安全子集和 OpenCode MCP 配置 安全子集；其中 standalone Tool 通过窄 `ScriptToolRuntime` 真实执行，但这不证明 package plugin、Hook、完整 Client 或 TUI target 已经可运行。 -四个切片当前都只支持本地执行域；Remote 外部来源发现、审批与冲突决策明确返回不支持，不回退读取本机来源。MCP +四个切片当前都只在事实所在 Host 的本地执行域运行；本机 Desktop/CLI 直接使用本机 Host，Peer 控制界面可代理 +Peer Host 的来源发现、审批与冲突决策，并按 Host 身份与工作区隔离结果。该能力不等于 SSH Remote 工作区执行； +SSH Remote 外部来源发现仍明确返回不支持，也不回退读取控制端或 Host 的同名本机来源。MCP 运行实例和工具 route 额外按规范化 workspace 隔离；更新、停用、空闲回收和删除先撤 route，再异步回收连接与进程， 慢启动在发布工具前复核撤销状态。 diff --git a/docs/architecture/product-architecture.md b/docs/architecture/product-architecture.md index 8b7547230..84c1a5a7a 100644 --- a/docs/architecture/product-architecture.md +++ b/docs/architecture/product-architecture.md @@ -233,11 +233,12 @@ Plugin Runtime P0 只验证了 BitFun 专用插件目录中的来源校验、工 不能称为“OpenCode 插件运行时”。详细代码事实集中在 [`plugin-runtime-host-design.md#8-当前实现附录`](extensions/plugin-runtime-host-design.md#8-当前实现附录)。 -与 Plugin Runtime 分离的三条纵向基线已经通过各自的能力专属 provider 契约接入：Prompt Command 可发现本地 +与 Plugin Runtime 分离的四条纵向基线已经通过各自的能力专属 provider 契约接入：Prompt Command 可发现本地 用户/项目 OpenCode Command、处理跨来源冲突，并在 CLI/TUI 中执行受支持的 prompt-only 模板；standalone Tool 可把受支持的单文件 `.js` 经确认后接入现有 Tool Runtime；Subagent 可把全局/项目声明的安全子集经确认和同名冲突 -选择后接入现有 Task/Subagent owner，并以 generation lease 固定 fresh single-run 调用。三类贡献对象互不复用， -主体逻辑不按生态分支。当前仍不表示 package plugin、Hook、primary agent、外部 agent 续接、Remote 来源发现或完整 +选择后接入现有 Task/Subagent owner，并以 generation lease 固定 fresh single-run 调用；MCP 可把受支持的用户/项目 +配置经确认和同名冲突选择后交给现有 MCP owner 运行。四类贡献对象互不复用，主体逻辑不按生态分支。当前仍不表示 +package plugin、Hook、primary agent、外部 agent 续接、SSH Remote 工作区来源发现或完整 配置兼容已经可用。 目标路线不要求 OpenCode 插件作者维护 `bitfun.plugin.json` 或复制到 `.bitfun/plugins`。BitFun 直接发现用户和 @@ -326,13 +327,13 @@ flowchart LR | 产品形态 | 当前扩展能力 | 入口行为 | |---|---|---| -| Desktop / product-full | 生产入口仍依赖 `bitfun-core/product-full` 作为兼容组装层；“外部 AI 应用”设置已消费 OpenCode Command、standalone Tool 和 Subagent 的统一来源快照、审批、冲突选择与诊断；Skills 场景和设置列表显示已发现 Skill 的生态来源与覆盖结果 | 仅本地执行域支持当前三条可执行纵向切片；Skill 仍使用独立 Registry，不据来源展示宣称已并入外部来源协调器。受管 package plugin 仍只有静态预览，不能据设置页接入宣称完整 OpenCode 插件运行时 | +| Desktop / product-full | 生产入口仍依赖 `bitfun-core/product-full` 作为兼容组装层；Beta“外部 AI 应用”设置已消费 OpenCode Command、standalone Tool、Subagent 和 MCP 的统一来源快照、审批、冲突选择与诊断；MCP 原生清单优先展示，外部候选保持独立来源、作用域和覆盖状态；Skills 场景显示已发现 Skill 的生态来源、用户/项目作用域与覆盖结果 | 当前四条可执行纵向切片由事实所在 Host 执行；本机 Desktop 使用本机 Host，Peer 控制界面代理 Peer Host，不在控制端回退发现或执行。Skill 仍使用独立 Registry，不据来源展示宣称已并入外部来源协调器。受管 package plugin 仍只有静态预览，不能据设置页接入宣称完整 OpenCode 插件运行时 | | CLI | 入口仍以 `bitfun-core/product-full` 作为执行兼容 owner；交互式 TUI 已可执行受支持的 Prompt Command，并在通用 `/tools` 与 `/agents` 入口按文字分组消费与 Desktop 相同的审批、冲突和刷新状态；`/agents` 同时承载主 Agent 切换、Subagent 管理和“外部 AI 应用”，不再注册 `/subagents` 或 `external-*` 平行命令。活动 turn 期间仍可查看和管理，只有主 Agent 切换被禁用。Skill 列表显示来源，模式配置按实际选择结果说明覆盖来源 | 已批准的 standalone Tool 进入现有 Tool Runtime；已批准的外部 Subagent 只支持 fresh single-run。CLI/TUI 不解析生态文件、不启动第二套 worker/Agent owner；非交互入口和 Remote 未接入时不得借本机 TUI 路径代执行。本地 Agent 与 Peer Host 路径选择 `DeliveryProfile::Cli` 并消费同一 Runtime Parts/SDK；主会话的恢复、转录、本地分支、用量报告生成/卡片持久化、模式更新与精确结算走 SDK，Peer Host 的基础会话创建/恢复/重命名/归档和 thread-goal 查询也走同一 SDK；远程分支明确不支持。TUI 模式切换异步持久化，期间保持输入和 resize 响应，只在成功后提交本地状态，失败保留原选择；用户可切换会话，退出有等待完成与再次强制退出两级路径。失效的主会话模式由 Core 恢复为可执行模式，TUI 显示迁移并阻止启动输入自动执行。本地工作区快照准备、会话文件清单、类型化统计和工作区文件回滚由 Desktop/Peer Host 共用的窄 owner port 调用现有 Core 实现；它不进入 SDK，不接受远程身份，Desktop 保留既有远程空结果，Peer Host 返回明确不支持错误，历史维护仍归宿主。账号同步、富历史及 Peer Host/ACP 其余持久化维护缺口仍由单一 Core 兼容门面转发。本切片不改变扩展执行边界 | | HarmonyOS PC 原生 CLI/TUI | 未来平台目标，当前未实现 | 目标、问题和风险见平台规约；具体适配另立专题，HAP、手机 Remote App 与远端代执行均不替代 | | HarmonyOS PC GUI | 完整 HarmonyOS PC 支持的另一目标形态，当前未实现 | 与 CLI/TUI 共享稳定能力和 Runtime 语义，但独立设计宿主、界面与发布验证；Web、Remote 或现有 Tauri Desktop 均不能替代 | | HarmonyOS 手机 Remote App | `src/apps/mobile/harmonyos` 是 phone-only ArkTS 远程入口，不持有本地 Rust Agent Runtime | 保持当前能力并按移动端专题独立演进；本轮不提前设计移动 Runtime/TUI/GUI，也不能据此宣称 HarmonyOS PC 本地能力 | | ACP | CLI 托管的服务端仍以 `bitfun-core/product-full` 作为兼容执行层 | 入口已选择 `DeliveryProfile::Acp` 并消费 Runtime Parts；组装层在入队前原子拒绝忙碌会话，不改变其他产品入口的排队行为；活动会话模型与模式写入走 SDK。`session/load` 先校验和建立临时 MCP，再恢复 Core，在历史回放成功后才发布活动状态；失败会卸载本次内存状态而不删除历史。同 ID 的重叠打开/关闭被明确拒绝。成功的 `session/close` 阻止新轮次、排空队列和后台子会话，再卸载临时 Core 状态并回收 MCP 与连接；失败保留会话所有权和历史，返回可重试阶段。持久化历史仍可重新加载。完整历史、模型/模式目录与配置读取仍留在现有 Core/ACP 归属，不据此宣称完整解耦 | -| Server / Remote | 当前生产路由没有插件状态消费闭环；Remote 插件执行未实现 | 不在本地替远端项目发现、准备或执行插件；未接入时返回明确不支持 | +| Server / Remote | Server 可返回 Host 只读外部来源快照并对变更操作 fail closed；Peer Host 已代理外部来源快照、策略和既有四类操作；SSH Remote 工作区的外部来源发现与执行仍未实现 | 只读 Host 不伪装为可管理；控制端用 Host 身份与工作区共同隔离异步结果。Peer Host 只处理 Host 上的真实工作区，不在控制端替远端发现；SSH Remote 未接入时返回明确不支持且不回退本机来源 | | Web / Mobile Web | 依赖现有后端入口，不持有插件执行单元 | 对应 profile 当前为空计划或未接入生产，不能据枚举值宣称独立产品能力 | | SDK | 仅有 preview 门面、空 profile 计划和测试替身 | 不牵引 `product-full`、具体服务管理器或插件 host ABI；未满足独立嵌入验证前不宣称可发布 | diff --git a/docs/plans/opencode-extension-compatibility-plan.md b/docs/plans/opencode-extension-compatibility-plan.md index 34fea4327..22e0a90e7 100644 --- a/docs/plans/opencode-extension-compatibility-plan.md +++ b/docs/plans/opencode-extension-compatibility-plan.md @@ -333,7 +333,8 @@ PR5 不新增扩展类型或运行能力，只补齐 Command、Tool、Subagent 因此只建议安装或修复后刷新，并说明用户可以继续使用其他能力；扩展入口不得建议或触发重启，也不得打断执行中的 session。 未来若产品增加统一重启动作，必须由 session owner 提供活动任务保护与恢复契约，默认允许“稍后”，不能由扩展模块自行判断。 -- Remote 工作区继续显示明确不支持；不加载本机配置代替远程配置。普通轮询仍为 5 秒，仅发现进行中使用 750 ms 有界轮询； +- SSH Remote 工作区继续显示明确不支持；不加载本机配置代替远程配置。Peer 控制界面代理 Peer Host 的真实状态和操作， + 不把控制端状态混入 Host。provider discovery deadline 仍为 5 秒；仅发现进行中使用请求完成后再调度的 750 ms、1.5 s、3 s、5 s 退避轮询； 本轮不增加 watcher、全局扫描、遥测平台或磁盘缓存，不改变 system prompt。 PR5 的退出条件是：三类已处理冲突在 GUI 中可见可改选；Tool/Subagent 在 TUI 中使用通用能力入口；原生候选顺序、 diff --git a/src/apps/cli/src/ui/agent_selector.rs b/src/apps/cli/src/ui/agent_selector.rs index 6fb36627a..19549a5a1 100644 --- a/src/apps/cli/src/ui/agent_selector.rs +++ b/src/apps/cli/src/ui/agent_selector.rs @@ -11,7 +11,10 @@ use ratatui::{ Frame, }; -use crate::ui::theme::{StyleKind, Theme}; +use crate::ui::{ + responsive_popup::{render_too_small, responsive_popup, ResponsivePopup}, + theme::{StyleKind, Theme}, +}; /// An agent item for display in the selector #[derive(Debug, Clone)] @@ -37,6 +40,7 @@ pub(super) struct AgentSelectorState { include_external_sources: bool, allow_mode_switch: bool, last_area: Option, + interaction_enabled: bool, } impl AgentSelectorState { @@ -49,6 +53,7 @@ impl AgentSelectorState { include_external_sources: false, allow_mode_switch: true, last_area: None, + interaction_enabled: true, } } @@ -71,6 +76,7 @@ impl AgentSelectorState { self.allow_mode_switch = allow_mode_switch; self.list_state.select(Some(initial_idx)); self.visible = true; + self.interaction_enabled = true; } pub(super) fn hide(&mut self) { @@ -95,7 +101,7 @@ impl AgentSelectorState { } pub(super) fn move_up(&mut self) { - if !self.visible || self.len() == 0 { + if !self.visible || !self.interaction_enabled || self.len() == 0 { return; } let selected = self.list_state.selected().unwrap_or(0); @@ -105,7 +111,7 @@ impl AgentSelectorState { } pub(super) fn move_down(&mut self) { - if !self.visible || self.len() == 0 { + if !self.visible || !self.interaction_enabled || self.len() == 0 { return; } let selected = self.list_state.selected().unwrap_or(0); @@ -115,7 +121,7 @@ impl AgentSelectorState { /// Get the selected action from the unified Agent entry. pub(super) fn confirm_selection(&self) -> Option { - if !self.visible { + if !self.visible || !self.interaction_enabled { return None; } let idx = self.list_state.selected()?; @@ -138,23 +144,20 @@ impl AgentSelectorState { return; } - let popup_width = area.width.saturating_sub(4).min(60); - let popup_height = (self.len() as u16 + 4).min(area.height.saturating_sub(2)); - if popup_height < 5 || popup_width < 20 { - self.last_area = None; - return; - } - - let popup_x = area.x + (area.width.saturating_sub(popup_width)) / 2; - let popup_y = area.y + (area.height.saturating_sub(popup_height)) / 2; - - let popup_area = Rect { - x: popup_x, - y: popup_y, - width: popup_width, - height: popup_height, + let ideal_height = self.len() as u16 + 4; + let layout = responsive_popup(area, 60, ideal_height, 20, 5); + let popup_area = match layout { + ResponsivePopup::Content(area) => area, + ResponsivePopup::TooSmall(area) => { + self.last_area = None; + self.interaction_enabled = false; + render_too_small(frame, area, theme, "Agents"); + return; + } }; + self.interaction_enabled = true; self.last_area = Some(popup_area); + let compact = popup_area.width < 40; let mut list_items: Vec = self .items @@ -180,49 +183,72 @@ impl AgentSelectorState { let desc_style = theme.style(StyleKind::Muted); let mut spans = vec![Span::styled(marker, marker_style)]; if !self.allow_mode_switch { - spans.push(Span::styled("After current turn · ", desc_style)); + spans.push(Span::styled( + if popup_area.width >= 28 { + "After current turn · " + } else { + "Wait · " + }, + desc_style, + )); + } + spans.push(Span::styled(&agent.id, name_style)); + if !compact { + spans.extend([ + Span::raw(" "), + Span::styled(format!("Main agent · {}", agent.description), desc_style), + ]); } - spans.extend([ - Span::styled(&agent.id, name_style), - Span::raw(" "), - Span::styled(format!("Main agent · {}", agent.description), desc_style), - ]); let line = Line::from(spans); ListItem::new(line) }) .collect(); - list_items.push(ListItem::new(Line::from(vec![ + let mut subagent_spans = vec![ Span::raw(" "), Span::styled( "Subagents", theme.style(StyleKind::Primary).add_modifier(Modifier::BOLD), ), - Span::raw(" "), - Span::styled( - "List, launch, or configure delegated agents", - theme.style(StyleKind::Muted), - ), - ]))); - if self.include_external_sources { - list_items.push(ListItem::new(Line::from(vec![ + ]; + if !compact { + subagent_spans.extend([ Span::raw(" "), Span::styled( - "External AI applications", - theme.style(StyleKind::Primary).add_modifier(Modifier::BOLD), + "List, launch, or configure delegated agents", + theme.style(StyleKind::Muted), ), + ]); + } + list_items.push(ListItem::new(Line::from(subagent_spans))); + if self.include_external_sources { + let mut external_spans = vec![ Span::raw(" "), Span::styled( - "Review imported agents and choices", - theme.style(StyleKind::Muted), + "External AI applications", + theme.style(StyleKind::Primary).add_modifier(Modifier::BOLD), ), - ]))); + ]; + if !compact { + external_spans.extend([ + Span::raw(" "), + Span::styled( + "Review imported agents and choices", + theme.style(StyleKind::Muted), + ), + ]); + } + list_items.push(ListItem::new(Line::from(external_spans))); } let block = Block::default() .borders(Borders::ALL) .border_style(theme.style(StyleKind::Primary)) .style(Style::default().bg(theme.background)) - .title(" Agents (↑↓ Navigate, Enter Select, Esc Cancel) "); + .title(if compact { + " Agents · Enter Select · Esc " + } else { + " Agents (↑↓ Navigate, Enter Select, Esc Cancel) " + }); let list = List::new(list_items) .block(block) @@ -444,4 +470,18 @@ mod tests { Some(AgentSelectorAction::ReviewExternalSources) )); } + + #[test] + fn too_small_fallback_disables_hidden_selection() { + let mut state = AgentSelectorState::new(); + state.show(modes(), Some("agentic".to_string()), true, true); + let mut terminal = Terminal::new(TestBackend::new(10, 3)).expect("test terminal"); + terminal + .draw(|frame| state.render(frame, frame.area(), &Theme::dark_ansi16())) + .expect("render tiny agent selector"); + + assert!(state.confirm_selection().is_none()); + state.move_down(); + assert!(state.confirm_selection().is_none()); + } } diff --git a/src/apps/cli/src/ui/chat/render.rs b/src/apps/cli/src/ui/chat/render.rs index 08f98d4d7..f8ef577d3 100644 --- a/src/apps/cli/src/ui/chat/render.rs +++ b/src/apps/cli/src/ui/chat/render.rs @@ -896,7 +896,7 @@ impl ChatView { self.mcp_selector.render(frame, area, &self.theme); } - fn render_mcp_add_dialog(&self, frame: &mut Frame, area: Rect) { + fn render_mcp_add_dialog(&mut self, frame: &mut Frame, area: Rect) { self.mcp_add_dialog.render(frame, area, &self.theme); } diff --git a/src/apps/cli/src/ui/mcp_add_dialog.rs b/src/apps/cli/src/ui/mcp_add_dialog.rs index bf15835e6..43cd1cf36 100644 --- a/src/apps/cli/src/ui/mcp_add_dialog.rs +++ b/src/apps/cli/src/ui/mcp_add_dialog.rs @@ -14,6 +14,7 @@ use ratatui::{ Frame, }; +use crate::ui::responsive_popup::{render_too_small, responsive_popup, ResponsivePopup}; use crate::ui::theme::{StyleKind, Theme}; /// Action returned by the MCP add dialog @@ -56,6 +57,8 @@ pub(super) struct McpAddDialogState { value_cursor: usize, /// Error message to display error: Option, + /// Disabled while the terminal cannot show a usable dialog. + interaction_enabled: bool, } impl McpAddDialogState { @@ -69,6 +72,7 @@ impl McpAddDialogState { value_buf: String::new(), value_cursor: 0, error: None, + interaction_enabled: true, } } @@ -81,6 +85,7 @@ impl McpAddDialogState { self.value_buf.clear(); self.value_cursor = 0; self.error = None; + self.interaction_enabled = true; } pub(super) fn hide(&mut self) { @@ -96,7 +101,7 @@ impl McpAddDialogState { /// Insert pasted text into the current active text field pub(super) fn insert_text(&mut self, text: &str) { - if !self.visible { + if !self.visible || !self.interaction_enabled { return; } let cleaned: String = text @@ -114,6 +119,14 @@ impl McpAddDialogState { return McpAddAction::None; } + if !self.interaction_enabled { + if key.code == KeyCode::Esc { + self.hide(); + return McpAddAction::Cancel; + } + return McpAddAction::None; + } + self.error = None; match self.step { @@ -319,7 +332,7 @@ impl McpAddDialogState { // ── Rendering ── - pub(super) fn render(&self, frame: &mut Frame, area: Rect, theme: &Theme) { + pub(super) fn render(&mut self, frame: &mut Frame, area: Rect, theme: &Theme) { if !self.visible { return; } @@ -333,20 +346,16 @@ impl McpAddDialogState { }; let has_error = self.error.is_some(); let dialog_height: u16 = 2 + completed_rows + 1 + 1 + 1 + if has_error { 1 } else { 0 }; - let dialog_width = area.width.saturating_sub(4).min(65); - if dialog_width < 35 || area.height < dialog_height + 2 { - return; - } - - let dialog_x = area.x + (area.width.saturating_sub(dialog_width)) / 2; - let dialog_y = area.y + (area.height.saturating_sub(dialog_height)) / 2; - - let dialog_area = Rect { - x: dialog_x, - y: dialog_y, - width: dialog_width, - height: dialog_height, + let dialog_area = match responsive_popup(area, 65, dialog_height, 18, 5) { + ResponsivePopup::Content(area) => area, + ResponsivePopup::TooSmall(area) => { + self.interaction_enabled = false; + render_too_small(frame, area, theme, "Add MCP"); + return; + } }; + self.interaction_enabled = true; + let compact = dialog_area.width < 35; frame.render_widget(Clear, dialog_area); @@ -355,7 +364,11 @@ impl McpAddDialogState { Step::ServerType => "2/3", Step::CommandOrUrl => "3/3", }; - let title = format!(" Add MCP Server ({}) ", step_label); + let title = if compact { + format!(" Add MCP {step_label} ") + } else { + format!(" Add MCP Server ({step_label}) ") + }; let block = Block::default() .borders(Borders::ALL) @@ -366,7 +379,7 @@ impl McpAddDialogState { let inner = block.inner(dialog_area); frame.render_widget(block, dialog_area); - if inner.width < 20 { + if inner.width < 16 { return; } @@ -374,7 +387,7 @@ impl McpAddDialogState { // ── Render completed steps as read-only summary ── - if self.step == Step::ServerType || self.step == Step::CommandOrUrl { + if !compact && (self.step == Step::ServerType || self.step == Step::CommandOrUrl) { let name_area = Rect { x: inner.x, y: inner.y + row, @@ -393,7 +406,7 @@ impl McpAddDialogState { row += 1; } - if self.step == Step::CommandOrUrl { + if !compact && self.step == Step::CommandOrUrl { let type_area = Rect { x: inner.x, y: inner.y + row, @@ -427,10 +440,12 @@ impl McpAddDialogState { width: inner.width, height: 1, }; - let prompt_line = Line::from(Span::styled( - "Enter MCP server name (used as identifier):", - theme.style(StyleKind::Info), - )); + let prompt = if compact { + "Server name" + } else { + "Enter MCP server name (used as identifier):" + }; + let prompt_line = Line::from(Span::styled(prompt, theme.style(StyleKind::Info))); frame.render_widget(Paragraph::new(prompt_line), prompt_area); row += 1; @@ -460,10 +475,12 @@ impl McpAddDialogState { width: inner.width, height: 1, }; - let prompt_line = Line::from(Span::styled( - "Select MCP server type:", - theme.style(StyleKind::Info), - )); + let prompt = if compact { + "Connection" + } else { + "Select MCP server type:" + }; + let prompt_line = Line::from(Span::styled(prompt, theme.style(StyleKind::Info))); frame.render_widget(Paragraph::new(prompt_line), prompt_area); row += 1; @@ -490,12 +507,25 @@ impl McpAddDialogState { } else { theme.style(StyleKind::Muted) }; - let type_line = Line::from(vec![ - Span::raw(" "), - Span::styled(" \u{25b6} local (stdio) ", local_style), - Span::raw(" "), - Span::styled(" \u{25b6} remote (streamable-http) ", remote_style), - ]); + let type_line = if compact { + match self.server_type { + ServerType::Local => Line::from(vec![ + Span::styled(" > Local ", local_style), + Span::styled("\u{2190}/\u{2192}", theme.style(StyleKind::Muted)), + ]), + ServerType::Remote => Line::from(vec![ + Span::styled(" > Remote ", remote_style), + Span::styled("\u{2190}/\u{2192}", theme.style(StyleKind::Muted)), + ]), + } + } else { + Line::from(vec![ + Span::raw(" "), + Span::styled(" \u{25b6} local (stdio) ", local_style), + Span::raw(" "), + Span::styled(" \u{25b6} remote (streamable-http) ", remote_style), + ]) + }; frame.render_widget(Paragraph::new(type_line), type_area); row += 1; } @@ -507,12 +537,14 @@ impl McpAddDialogState { width: inner.width, height: 1, }; - let (prompt_text, placeholder) = match self.server_type { - ServerType::Local => ( + let (prompt_text, placeholder) = match (self.server_type, compact) { + (ServerType::Local, true) => ("Command", "npx -y server"), + (ServerType::Remote, true) => ("URL", "https://..."), + (ServerType::Local, false) => ( "Enter command to run the MCP server:", "npx -y @modelcontextprotocol/server-xxx", ), - ServerType::Remote => ( + (ServerType::Remote, false) => ( "Enter the remote MCP server URL:", "https://example.com/mcp", ), @@ -568,10 +600,13 @@ impl McpAddDialogState { width: inner.width, height: 1, }; - let hint_text = match self.step { - Step::Name => "Enter: Next Esc: Cancel", - Step::ServerType => "\u{2190}\u{2192}/Tab: Switch Enter: Next Esc: Back", - Step::CommandOrUrl => "Enter: Confirm Ctrl+U: Clear Esc: Back", + let hint_text = match (self.step, compact) { + (Step::Name, true) => "Enter next Esc close", + (Step::ServerType, true) => "Enter next Esc back", + (Step::CommandOrUrl, true) => "Enter save Esc back", + (Step::Name, false) => "Enter: Next Esc: Cancel", + (Step::ServerType, false) => "\u{2190}\u{2192}/Tab: Switch Enter: Next Esc: Back", + (Step::CommandOrUrl, false) => "Enter: Confirm Ctrl+U: Clear Esc: Back", }; let hint = Paragraph::new(Line::from(Span::styled( hint_text, @@ -699,3 +734,66 @@ fn char_to_byte(s: &str, char_pos: usize) -> usize { .map(|(i, _)| i) .unwrap_or(s.len()) } + +#[cfg(test)] +mod tests { + use super::*; + use ratatui::{backend::TestBackend, Terminal}; + + #[test] + fn too_small_dialog_blocks_hidden_input_and_submit_but_allows_close() { + let mut state = McpAddDialogState::new(); + state.show(); + let mut terminal = Terminal::new(TestBackend::new(10, 3)).expect("test terminal"); + terminal + .draw(|frame| state.render(frame, frame.area(), &Theme::dark_ansi16())) + .expect("render tiny MCP add dialog"); + + state.insert_text("hidden-server"); + assert!(state.name_buf.is_empty()); + assert!(matches!( + state.handle_key_event(KeyEvent::new(KeyCode::Char('x'), KeyModifiers::NONE)), + McpAddAction::None + )); + assert!(state.name_buf.is_empty()); + assert_eq!(state.step, Step::Name); + assert!(matches!( + state.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)), + McpAddAction::None + )); + assert_eq!(state.step, Step::Name); + + assert!(matches!( + state.handle_key_event(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE)), + McpAddAction::Cancel + )); + assert!(!state.is_visible()); + } + + #[test] + fn compact_dialog_keeps_the_current_step_visible_and_editable() { + let mut state = McpAddDialogState::new(); + state.show(); + let mut terminal = Terminal::new(TestBackend::new(20, 6)).expect("test terminal"); + terminal + .draw(|frame| state.render(frame, frame.area(), &Theme::dark_ansi16())) + .expect("render compact MCP add dialog"); + + let buffer = terminal.backend().buffer(); + let rendered = (0..buffer.area.height) + .map(|y| { + (0..buffer.area.width) + .map(|x| buffer[(x, y)].symbol()) + .collect::() + }) + .collect::>() + .join("\n"); + assert!( + rendered.contains("Server name"), + "step missing: {rendered:?}" + ); + + state.insert_text("docs"); + assert_eq!(state.name_buf, "docs"); + } +} diff --git a/src/apps/cli/src/ui/mcp_selector.rs b/src/apps/cli/src/ui/mcp_selector.rs index 33213096b..f991ccbec 100644 --- a/src/apps/cli/src/ui/mcp_selector.rs +++ b/src/apps/cli/src/ui/mcp_selector.rs @@ -18,7 +18,10 @@ use ratatui::{ }; use unicode_width::UnicodeWidthChar; -use crate::ui::theme::{StyleKind, Theme}; +use crate::ui::{ + responsive_popup::{render_too_small, responsive_popup, ResponsivePopup}, + theme::{StyleKind, Theme}, +}; fn wrap_confirmation_detail(value: &str, width: usize) -> Vec { let width = width.max(1); @@ -132,6 +135,7 @@ pub(super) struct McpSelectorState { confirmation_max_scroll: u16, confirmation_reviewed: bool, last_area: Option, + interaction_enabled: bool, } impl McpSelectorState { @@ -147,6 +151,7 @@ impl McpSelectorState { confirmation_max_scroll: 0, confirmation_reviewed: false, last_area: None, + interaction_enabled: true, } } @@ -160,6 +165,7 @@ impl McpSelectorState { } self.loading_id = None; self.visible = true; + self.interaction_enabled = true; } /// Update items in-place (after toggle completes) without closing @@ -260,7 +266,7 @@ impl McpSelectorState { } pub(super) fn move_up(&mut self) { - if !self.visible || self.items.is_empty() { + if !self.visible || !self.interaction_enabled || self.items.is_empty() { return; } if self.confirm_external_id.is_some() { @@ -274,7 +280,7 @@ impl McpSelectorState { } pub(super) fn move_down(&mut self) { - if !self.visible || self.items.is_empty() { + if !self.visible || !self.interaction_enabled || self.items.is_empty() { return; } if self.confirm_external_id.is_some() { @@ -294,7 +300,7 @@ impl McpSelectorState { /// Get the selected MCP item (for toggle action) pub(super) fn confirm_selection(&self) -> Option { - if !self.visible { + if !self.visible || !self.interaction_enabled { return None; } let idx = self.list_state.selected()?; @@ -311,36 +317,51 @@ impl McpSelectorState { return; } - let popup_width = area.width.saturating_sub(4).min(72); - // +5 for border(2) + title(1) + hint(1) + padding(1) + let provisional_width = area.width.saturating_sub(4).min(72).max(1); let confirmation_height = self .confirm_external_id .as_ref() .and_then(|id| self.items.iter().find(|item| &item.id == id)) .map(|item| { - wrap_confirmation_detail(&item.detail, popup_width.saturating_sub(6) as usize).len() - as u16 + wrap_confirmation_detail(&item.detail, provisional_width.saturating_sub(6) as usize) + .len() as u16 + 2 }) .unwrap_or(0); - let max_popup_height = area.height.saturating_sub(2); - if max_popup_height < 6 || popup_width < 30 { - self.last_area = None; - return; - } - let popup_height = (self.items.len() as u16 + 5 + confirmation_height) - .min(max_popup_height) - .max(6); - - let popup_x = area.x + (area.width.saturating_sub(popup_width)) / 2; - let popup_y = area.y + (area.height.saturating_sub(popup_height)) / 2; - - let popup_area = Rect { - x: popup_x, - y: popup_y, - width: popup_width, - height: popup_height, + let initial_layout = responsive_popup( + area, + 72, + (self.items.len() as u16 + 5 + confirmation_height).max(6), + 18, + 6, + ); + let initial_area = match initial_layout { + ResponsivePopup::Content(area) => area, + ResponsivePopup::TooSmall(area) => { + self.last_area = None; + self.interaction_enabled = false; + render_too_small(frame, area, theme, "MCP Servers"); + return; + } }; + let compact_layout = initial_area.width < 50; + let list_height = self.items.len() as u16 * if compact_layout { 2 } else { 1 }; + // Compact rows keep source/name and status on separate lines and omit the footer. + let chrome_height = if compact_layout { 2 } else { 5 }; + let ideal_height = (list_height + chrome_height + confirmation_height).max(6); + let layout = responsive_popup(area, 72, ideal_height, 18, 6); + let popup_area = match layout { + ResponsivePopup::Content(area) => area, + ResponsivePopup::TooSmall(area) => { + self.last_area = None; + self.interaction_enabled = false; + render_too_small(frame, area, theme, "MCP Servers"); + return; + } + }; + self.interaction_enabled = true; + let popup_width = popup_area.width; + let popup_height = popup_area.height; self.last_area = Some(popup_area); if let Some(item) = self @@ -399,21 +420,33 @@ impl McpSelectorState { // If this item is pending delete confirmation, show special style if is_confirm_delete { - let line = Line::from(vec![ - Span::styled( - "\u{2717} ", - theme.style(StyleKind::Error).add_modifier(Modifier::BOLD), - ), - Span::styled( - &item.name, - theme.style(StyleKind::Error).add_modifier(Modifier::BOLD), - ), - Span::styled( - " \u{2190} Press 'd' again to delete, any other key to cancel", - theme.style(StyleKind::Error), - ), - ]); - return ListItem::new(line); + let identity_spans = || { + vec![ + Span::styled( + "\u{2717} ", + theme.style(StyleKind::Error).add_modifier(Modifier::BOLD), + ), + Span::styled( + &item.name, + theme.style(StyleKind::Error).add_modifier(Modifier::BOLD), + ), + ] + }; + if popup_width < 50 { + return ListItem::new(vec![ + Line::from(identity_spans()), + Line::from(Span::styled( + " Press 'd' again to delete", + theme.style(StyleKind::Error), + )), + ]); + } + let mut spans = identity_spans(); + spans.push(Span::styled( + " \u{2190} Press 'd' again to delete, any other key to cancel", + theme.style(StyleKind::Error), + )); + return ListItem::new(Line::from(spans)); } // Status indicator @@ -457,20 +490,40 @@ impl McpSelectorState { String::new() }; - let line = Line::from(vec![ - Span::styled(marker, marker_style), - Span::styled(&item.name, name_style), - Span::raw(" "), - Span::styled(&item.server_type, type_style), - Span::raw(" "), - Span::styled(status_text, status_style), - Span::styled(tool_text, theme.style(StyleKind::Muted)), - Span::styled( - format!(" [{}]", item.source_label), - theme.style(StyleKind::Muted), - ), - ]); - ListItem::new(line) + if popup_width < 50 { + ListItem::new(vec![ + Line::from(vec![ + Span::styled( + if item.external { + "[External] " + } else { + "[BitFun] " + }, + theme.style(StyleKind::Muted), + ), + Span::styled(&item.name, name_style), + ]), + Line::from(vec![ + Span::raw(" "), + Span::styled(marker, marker_style), + Span::styled(status_text, status_style), + ]), + ]) + } else { + ListItem::new(Line::from(vec![ + Span::styled(marker, marker_style), + Span::styled(&item.name, name_style), + Span::raw(" "), + Span::styled(&item.server_type, type_style), + Span::raw(" "), + Span::styled(status_text, status_style), + Span::styled(tool_text, theme.style(StyleKind::Muted)), + Span::styled( + format!(" [{}]", item.source_label), + theme.style(StyleKind::Muted), + ), + ])) + } }) .collect(); @@ -494,10 +547,12 @@ impl McpSelectorState { } else { " a:Add d:Delete e:Edit Config Space:Toggle Esc:Close" }; - list_items.push(ListItem::new(Line::from(Span::styled( - hint_text, - theme.style(StyleKind::Muted), - )))); + if popup_width >= 50 { + list_items.push(ListItem::new(Line::from(Span::styled( + hint_text, + theme.style(StyleKind::Muted), + )))); + } let block = Block::default() .borders(Borders::ALL) @@ -593,7 +648,8 @@ impl McpSelectorState { } let offset = self.list_state.offset(); - let index = (row - inner_y) as usize + offset; + let row_height = if area.width < 50 { 2 } else { 1 }; + let index = ((row - inner_y) / row_height) as usize + offset; if index >= self.items.len() { return None; } @@ -608,8 +664,11 @@ mod tests { confirmation_max_scroll, confirmation_review_lines, wrap_confirmation_detail, McpItem, McpItemAction, McpSelectorState, }; + use ratatui::{backend::TestBackend, Terminal}; use unicode_width::UnicodeWidthStr; + use crate::ui::theme::Theme; + fn item(action: McpItemAction, external: bool) -> McpItem { McpItem { id: "github".to_string(), @@ -734,4 +793,106 @@ mod tests { assert!(state.loading_id.is_none()); assert!(state.confirm_external_id.is_none()); } + + #[test] + fn twenty_by_six_popup_keeps_source_name_and_status_visible() { + let mut native = item(McpItemAction::NativeToggle, false); + native.status = "Healthy".to_string(); + let mut external = item( + McpItemAction::ReadOnly { + reason: "Managed externally".to_string(), + }, + true, + ); + external.id = "docs".to_string(); + external.name = "docs".to_string(); + external.status = "Pending".to_string(); + + let mut state = McpSelectorState::new(); + state.show(vec![native, external]); + let mut terminal = Terminal::new(TestBackend::new(20, 6)).expect("test terminal"); + terminal + .draw(|frame| state.render(frame, frame.area(), &Theme::dark_ansi16())) + .expect("render compact MCP selector"); + + let buffer = terminal.backend().buffer(); + let rendered = (0..buffer.area.height) + .map(|y| { + (0..buffer.area.width) + .map(|x| buffer[(x, y)].symbol()) + .collect::() + }) + .collect::>() + .join("\n"); + + assert!( + rendered.contains("[BitFun] github"), + "native row missing: {rendered:?}" + ); + assert!( + rendered.contains("[External] docs"), + "external row missing: {rendered:?}" + ); + assert!( + rendered.contains("Healthy"), + "native status missing: {rendered:?}" + ); + assert!( + rendered.contains("Pending"), + "external status missing: {rendered:?}" + ); + } + + #[test] + fn too_small_fallback_disables_hidden_selection() { + let mut state = McpSelectorState::new(); + state.show(vec![item(McpItemAction::NativeToggle, false)]); + let mut terminal = Terminal::new(TestBackend::new(10, 3)).expect("test terminal"); + terminal + .draw(|frame| state.render(frame, frame.area(), &Theme::dark_ansi16())) + .expect("render tiny MCP selector"); + + assert!(state.confirm_selection().is_none()); + state.move_down(); + assert!(state.confirm_selection().is_none()); + } + + #[test] + fn compact_boundary_uses_two_line_height_for_render_and_mouse_hit_testing() { + let first = item(McpItemAction::NativeToggle, false); + let mut second = item(McpItemAction::NativeToggle, false); + second.id = "second".to_string(); + second.name = "second".to_string(); + let mut state = McpSelectorState::new(); + state.show(vec![first, second]); + let mut terminal = Terminal::new(TestBackend::new(53, 10)).expect("test terminal"); + terminal + .draw(|frame| state.render(frame, frame.area(), &Theme::dark_ansi16())) + .expect("render boundary-width MCP selector"); + + let area = state.last_area.expect("popup area"); + assert!(area.width < 50); + assert_eq!(state.item_index_at(area.y + 2, area), Some(0)); + assert_eq!(state.item_index_at(area.y + 4, area), Some(1)); + } + + #[test] + fn compact_delete_confirmation_keeps_following_item_mouse_hit_aligned() { + let first = item(McpItemAction::NativeToggle, false); + let first_id = first.id.clone(); + let mut second = item(McpItemAction::NativeToggle, false); + second.id = "second".to_string(); + second.name = "second".to_string(); + let mut state = McpSelectorState::new(); + state.show(vec![first, second]); + state.start_confirm_delete(first_id); + let mut terminal = Terminal::new(TestBackend::new(53, 10)).expect("test terminal"); + terminal + .draw(|frame| state.render(frame, frame.area(), &Theme::dark_ansi16())) + .expect("render compact MCP delete confirmation"); + + let area = state.last_area.expect("popup area"); + assert!(area.width < 50); + assert_eq!(state.item_index_at(area.y + 3, area), Some(1)); + } } diff --git a/src/apps/cli/src/ui/mod.rs b/src/apps/cli/src/ui/mod.rs index e5dca4aed..40e307ee2 100644 --- a/src/apps/cli/src/ui/mod.rs +++ b/src/apps/cli/src/ui/mod.rs @@ -16,6 +16,7 @@ pub(crate) mod model_selector; pub(crate) mod permission; pub(crate) mod provider_selector; pub(crate) mod question; +mod responsive_popup; pub(crate) mod session_selector; pub(crate) mod skill_selector; pub(crate) mod startup; diff --git a/src/apps/cli/src/ui/responsive_popup.rs b/src/apps/cli/src/ui/responsive_popup.rs new file mode 100644 index 000000000..313906b27 --- /dev/null +++ b/src/apps/cli/src/ui/responsive_popup.rs @@ -0,0 +1,86 @@ +use ratatui::{ + layout::Rect, + style::Style, + text::Line, + widgets::{Block, Borders, Clear, Paragraph}, + Frame, +}; + +use crate::ui::theme::{StyleKind, Theme}; + +pub(super) enum ResponsivePopup { + Content(Rect), + TooSmall(Rect), +} + +pub(super) fn responsive_popup( + area: Rect, + max_width: u16, + ideal_height: u16, + min_content_width: u16, + min_content_height: u16, +) -> ResponsivePopup { + if area.width == 0 || area.height == 0 { + return ResponsivePopup::TooSmall(area); + } + + let inset_width = area.width.saturating_sub(4).min(max_width); + let inset_height = ideal_height.min(area.height.saturating_sub(2)); + let (width, height) = if inset_width >= min_content_width && inset_height >= min_content_height + { + (inset_width, inset_height) + } else if area.width >= min_content_width && area.height >= min_content_height { + (area.width.min(max_width), ideal_height.min(area.height)) + } else { + return ResponsivePopup::TooSmall(area); + }; + + ResponsivePopup::Content(Rect { + x: area.x + area.width.saturating_sub(width) / 2, + y: area.y + area.height.saturating_sub(height) / 2, + width, + height, + }) +} + +pub(super) fn render_too_small(frame: &mut Frame, area: Rect, theme: &Theme, title: &str) { + frame.render_widget(Clear, area); + if area.width < 4 || area.height < 3 { + frame.render_widget( + Paragraph::new(Line::from("Esc")).style(theme.style(StyleKind::Muted)), + area, + ); + return; + } + + let block = Block::default() + .borders(Borders::ALL) + .border_style(theme.style(StyleKind::Primary)) + .style(Style::default().bg(theme.background)) + .title(format!(" {title} ")); + frame.render_widget( + Paragraph::new("Terminal too small - resize or Esc") + .block(block) + .style(Style::default().bg(theme.background)), + area, + ); +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn uses_full_area_before_falling_back_to_a_message() { + let compact = responsive_popup(Rect::new(0, 0, 20, 6), 72, 10, 18, 6); + assert!( + matches!(compact, ResponsivePopup::Content(area) if area == Rect::new(0, 0, 20, 6)) + ); + + let tiny = responsive_popup(Rect::new(0, 0, 10, 3), 72, 10, 18, 6); + assert!(matches!(tiny, ResponsivePopup::TooSmall(_))); + + let zero = responsive_popup(Rect::new(0, 0, 0, 0), 72, 10, 18, 6); + assert!(matches!(zero, ResponsivePopup::TooSmall(_))); + } +} diff --git a/src/apps/cli/src/ui/skill_selector.rs b/src/apps/cli/src/ui/skill_selector.rs index 65bb6fd58..15e0da340 100644 --- a/src/apps/cli/src/ui/skill_selector.rs +++ b/src/apps/cli/src/ui/skill_selector.rs @@ -12,7 +12,10 @@ use ratatui::{ }; use std::collections::HashMap; -use crate::ui::theme::{StyleKind, Theme}; +use crate::ui::{ + responsive_popup::{render_too_small, responsive_popup, ResponsivePopup}, + theme::{StyleKind, Theme}, +}; /// A skill item for display in the selector. #[derive(Debug, Clone)] @@ -97,6 +100,7 @@ pub(super) struct SkillSelectorState { list_state: ListState, visible: bool, last_area: Option, + interaction_enabled: bool, screen: SkillSelectorScreen, } @@ -108,6 +112,7 @@ impl SkillSelectorState { list_state: ListState::default(), visible: false, last_area: None, + interaction_enabled: true, screen: SkillSelectorScreen::Menu, } } @@ -118,6 +123,7 @@ impl SkillSelectorState { self.screen = SkillSelectorScreen::Menu; self.list_state.select(Some(0)); self.visible = true; + self.interaction_enabled = true; } /// Show the current mode's runtime-visible skills. @@ -131,6 +137,7 @@ impl SkillSelectorState { self.screen = SkillSelectorScreen::List; self.list_state.select(Some(0)); self.visible = true; + self.interaction_enabled = true; } /// Show all discovered skills with mode-specific enablement checkboxes. @@ -157,6 +164,7 @@ impl SkillSelectorState { self.screen = SkillSelectorScreen::Configure; self.list_state.select(Some(next_index)); self.visible = true; + self.interaction_enabled = true; } pub(super) fn hide(&mut self) { @@ -177,7 +185,7 @@ impl SkillSelectorState { } pub(super) fn move_up(&mut self) { - if !self.visible || self.len() == 0 { + if !self.visible || !self.interaction_enabled || self.len() == 0 { return; } let selected = self.list_state.selected().unwrap_or(0); @@ -187,7 +195,7 @@ impl SkillSelectorState { } pub(super) fn move_down(&mut self) { - if !self.visible || self.len() == 0 { + if !self.visible || !self.interaction_enabled || self.len() == 0 { return; } let selected = self.list_state.selected().unwrap_or(0); @@ -197,7 +205,7 @@ impl SkillSelectorState { /// Get the selected action. pub(super) fn confirm_selection(&self) -> Option { - if !self.visible { + if !self.visible || !self.interaction_enabled { return None; } let idx = self.list_state.selected()?; @@ -234,25 +242,32 @@ impl SkillSelectorState { return; } - let popup_width = area.width.saturating_sub(4).min(92); - let popup_height = (self.len() as u16 + 4).min(area.height.saturating_sub(2)); - if popup_height < 5 || popup_width < 20 { - self.last_area = None; - return; - } - - let popup_x = area.x + (area.width.saturating_sub(popup_width)) / 2; - let popup_y = area.y + (area.height.saturating_sub(popup_height)) / 2; - - let popup_area = Rect { - x: popup_x, - y: popup_y, - width: popup_width, - height: popup_height, + let initial_layout = responsive_popup(area, 92, self.len() as u16 + 4, 20, 5); + let initial_area = match initial_layout { + ResponsivePopup::Content(area) => area, + ResponsivePopup::TooSmall(area) => { + self.last_area = None; + self.interaction_enabled = false; + render_too_small(frame, area, theme, "Skills"); + return; + } + }; + let row_height = self.row_height(initial_area.width); + let ideal_height = self.len() as u16 * row_height + 4; + let layout = responsive_popup(area, 92, ideal_height, 20, 5); + let popup_area = match layout { + ResponsivePopup::Content(area) => area, + ResponsivePopup::TooSmall(area) => { + self.last_area = None; + self.interaction_enabled = false; + render_too_small(frame, area, theme, "Skills"); + return; + } }; + self.interaction_enabled = true; self.last_area = Some(popup_area); - let content_width = popup_width.saturating_sub(2); + let content_width = popup_area.width.saturating_sub(2); let list_items = self.render_items(theme, content_width); let title = match self.screen { SkillSelectorScreen::Menu => " Skills ", @@ -339,7 +354,8 @@ impl SkillSelectorState { } let offset = self.list_state.offset(); - let index = (row - inner_y) as usize + offset; + let row_height = self.row_height(area.width); + let index = ((row - inner_y) / row_height) as usize + offset; if index >= self.len() { return None; } @@ -347,6 +363,18 @@ impl SkillSelectorState { Some(index) } + fn row_height(&self, popup_width: u16) -> u16 { + match self.screen { + SkillSelectorScreen::Menu => 1, + SkillSelectorScreen::List | SkillSelectorScreen::Configure + if popup_width.saturating_sub(2) < 40 => + { + 2 + } + SkillSelectorScreen::List | SkillSelectorScreen::Configure => 1, + } + } + fn render_items(&self, theme: &Theme, content_width: u16) -> Vec> { match self.screen { SkillSelectorScreen::Menu => vec![ @@ -456,6 +484,24 @@ impl SkillSelectorState { spans.push(Span::styled(skill.description.clone(), desc_style)); } + if compact { + let scope = match skill.level.as_str() { + "project" => "project", + "user" => "user", + _ => "other", + }; + return ListItem::new(vec![ + Line::from(spans), + Line::from(Span::styled( + format!( + " {scope} · {}", + compact_source_label(skill.display_source_label()) + ), + level_style, + )), + ]); + } + ListItem::new(Line::from(spans)) } } @@ -553,5 +599,57 @@ mod tests { rendered.contains("[~] pdf < BitFun"), "narrow configuration should prioritize the skill name and coverage: {rendered:?}" ); + assert!( + rendered.contains("user · Claude"), + "source and scope missing: {rendered:?}" + ); + assert!( + rendered.contains("project · BitFun"), + "source and scope missing: {rendered:?}" + ); + } + + #[test] + fn too_small_fallback_disables_hidden_selection() { + let mut state = SkillSelectorState::new(); + state.show_list(vec![skill_item("project::bitfun::pdf", "BitFun")]); + let mut terminal = Terminal::new(TestBackend::new(10, 3)).expect("test terminal"); + terminal + .draw(|frame| state.render(frame, frame.area(), &Theme::dark_ansi16())) + .expect("render tiny skill selector"); + + assert!(state.confirm_selection().is_none()); + state.move_down(); + assert!(state.confirm_selection().is_none()); + } + + #[test] + fn compact_boundary_uses_two_line_height_for_render_and_mouse_hit_testing() { + let first = skill_item("project::bitfun::pdf", "BitFun"); + let second = skill_item("user::home.claude::pdf", "Claude Code"); + let mut state = SkillSelectorState::new(); + state.show_config(vec![first, second]); + let mut terminal = Terminal::new(TestBackend::new(44, 8)).expect("test terminal"); + terminal + .draw(|frame| state.render(frame, frame.area(), &Theme::dark_ansi16())) + .expect("render boundary-width skill selector"); + + let area = state.last_area.expect("popup area"); + assert_eq!(state.item_index_at(area.y + 2, area), Some(0)); + assert_eq!(state.item_index_at(area.y + 4, area), Some(1)); + } + + #[test] + fn compact_menu_keeps_single_line_mouse_hit_testing() { + let mut state = SkillSelectorState::new(); + state.show_menu(); + let mut terminal = Terminal::new(TestBackend::new(44, 8)).expect("test terminal"); + terminal + .draw(|frame| state.render(frame, frame.area(), &Theme::dark_ansi16())) + .expect("render compact skill menu"); + + let area = state.last_area.expect("popup area"); + assert_eq!(state.item_index_at(area.y + 1, area), Some(0)); + assert_eq!(state.item_index_at(area.y + 2, area), Some(1)); } } diff --git a/src/apps/cli/src/ui/subagent_selector.rs b/src/apps/cli/src/ui/subagent_selector.rs index e7c937977..dd943686f 100644 --- a/src/apps/cli/src/ui/subagent_selector.rs +++ b/src/apps/cli/src/ui/subagent_selector.rs @@ -11,6 +11,7 @@ use ratatui::{ Frame, }; +use crate::ui::responsive_popup::{render_too_small, responsive_popup, ResponsivePopup}; use crate::ui::theme::{StyleKind, Theme}; /// A subagent item for display in the selector @@ -45,6 +46,7 @@ pub(super) struct SubagentSelectorState { list_state: ListState, visible: bool, last_area: Option, + interaction_enabled: bool, screen: SubagentSelectorScreen, } @@ -55,6 +57,7 @@ impl SubagentSelectorState { list_state: ListState::default(), visible: false, last_area: None, + interaction_enabled: true, screen: SubagentSelectorScreen::Menu, } } @@ -64,6 +67,7 @@ impl SubagentSelectorState { self.screen = SubagentSelectorScreen::Menu; self.list_state.select(Some(0)); self.visible = true; + self.interaction_enabled = true; } /// Show subagents available to the current parent mode. @@ -76,6 +80,7 @@ impl SubagentSelectorState { self.screen = SubagentSelectorScreen::List; self.list_state.select(Some(0)); self.visible = true; + self.interaction_enabled = true; } /// Show all discovered subagents with mode-specific enablement checkboxes. @@ -101,6 +106,7 @@ impl SubagentSelectorState { self.screen = SubagentSelectorScreen::Configure; self.list_state.select(Some(next_index)); self.visible = true; + self.interaction_enabled = true; } pub(super) fn hide(&mut self) { @@ -121,7 +127,7 @@ impl SubagentSelectorState { } pub(super) fn move_up(&mut self) { - if !self.visible || self.len() == 0 { + if !self.visible || !self.interaction_enabled || self.len() == 0 { return; } let selected = self.list_state.selected().unwrap_or(0); @@ -131,7 +137,7 @@ impl SubagentSelectorState { } pub(super) fn move_down(&mut self) { - if !self.visible || self.len() == 0 { + if !self.visible || !self.interaction_enabled || self.len() == 0 { return; } let selected = self.list_state.selected().unwrap_or(0); @@ -141,7 +147,7 @@ impl SubagentSelectorState { /// Get the selected action. pub(super) fn confirm_selection(&self) -> Option { - if !self.visible { + if !self.visible || !self.interaction_enabled { return None; } let idx = self.list_state.selected()?; @@ -178,22 +184,18 @@ impl SubagentSelectorState { return; } - let popup_width = area.width.saturating_sub(4).min(92); - let popup_height = (self.len() as u16 + 4).min(area.height.saturating_sub(2)); - if popup_height < 5 || popup_width < 20 { - self.last_area = None; - return; - } - - let popup_x = area.x + (area.width.saturating_sub(popup_width)) / 2; - let popup_y = area.y + (area.height.saturating_sub(popup_height)) / 2; - - let popup_area = Rect { - x: popup_x, - y: popup_y, - width: popup_width, - height: popup_height, + let ideal_height = self.len() as u16 + 4; + let layout = responsive_popup(area, 92, ideal_height, 20, 5); + let popup_area = match layout { + ResponsivePopup::Content(area) => area, + ResponsivePopup::TooSmall(area) => { + self.last_area = None; + self.interaction_enabled = false; + render_too_small(frame, area, theme, "Subagents"); + return; + } }; + self.interaction_enabled = true; self.last_area = Some(popup_area); let list_items = self.render_items(theme); @@ -373,3 +375,31 @@ impl SubagentSelectorState { ListItem::new(Line::from(spans)) } } + +#[cfg(test)] +mod tests { + use super::{SubagentItem, SubagentSelectorState}; + use crate::ui::theme::Theme; + use ratatui::{backend::TestBackend, Terminal}; + + #[test] + fn too_small_fallback_disables_hidden_selection() { + let mut state = SubagentSelectorState::new(); + state.show_list(vec![SubagentItem { + key: "review".to_string(), + id: "review".to_string(), + name: "Review".to_string(), + description: String::new(), + source: "builtin".to_string(), + enabled: true, + }]); + let mut terminal = Terminal::new(TestBackend::new(10, 3)).expect("test terminal"); + terminal + .draw(|frame| state.render(frame, frame.area(), &Theme::dark_ansi16())) + .expect("render tiny subagent selector"); + + assert!(state.confirm_selection().is_none()); + state.move_down(); + assert!(state.confirm_selection().is_none()); + } +} diff --git a/src/web-ui/src/app/scenes/settings/settingsConfig.ts b/src/web-ui/src/app/scenes/settings/settingsConfig.ts index f7377caf1..e006f0ef8 100644 --- a/src/web-ui/src/app/scenes/settings/settingsConfig.ts +++ b/src/web-ui/src/app/scenes/settings/settingsConfig.ts @@ -218,6 +218,7 @@ export const SETTINGS_CATEGORIES: ConfigCategoryDef[] = [ id: 'external-sources', labelKey: 'configCenter.tabs.externalSources', descriptionKey: 'configCenter.tabDescriptions.externalSources', + beta: true, keywords: [ 'external ai applications', 'import work', diff --git a/src/web-ui/src/app/scenes/skills/SkillsScene.scss b/src/web-ui/src/app/scenes/skills/SkillsScene.scss index d75e64e29..60ab34a2f 100644 --- a/src/web-ui/src/app/scenes/skills/SkillsScene.scss +++ b/src/web-ui/src/app/scenes/skills/SkillsScene.scss @@ -773,41 +773,13 @@ .skills-card__meta { display: flex; + flex-wrap: wrap; align-items: center; gap: 8px; padding-top: 4px; border-top: 1px solid var(--border-subtle); } -.skills-card__path { - flex: 1; - min-width: 0; - display: inline-flex; - align-items: center; - gap: 4px; - padding: 0; - border: none; - background: none; - color: var(--color-text-muted); - font-size: 10px; - font-family: $font-family-mono; - line-height: 1; - opacity: 0.65; - text-align: left; - white-space: nowrap; - overflow: hidden; - text-overflow: ellipsis; - cursor: pointer; - transition: opacity $motion-fast $easing-standard; - - &:hover { - opacity: 1; - color: var(--color-accent-500); - text-decoration: underline; - text-underline-offset: 2px; - } -} - .skills-card__actions { display: flex; align-items: center; diff --git a/src/web-ui/src/app/scenes/skills/SkillsScene.tsx b/src/web-ui/src/app/scenes/skills/SkillsScene.tsx index 6081a0bb6..c4abb6b08 100644 --- a/src/web-ui/src/app/scenes/skills/SkillsScene.tsx +++ b/src/web-ui/src/app/scenes/skills/SkillsScene.tsx @@ -29,6 +29,8 @@ import { getSkillSourceLabel, } from '@/infrastructure/config/skillSourcePresentation'; import { workspaceAPI } from '@/infrastructure/api'; +import { usePeerDeviceModeOptional } from '@/infrastructure/peer-device/PeerDeviceContext'; +import { isTauriRuntime } from '@/infrastructure/runtime'; import { workspaceManager } from '@/infrastructure/services/business/workspaceManager'; import { useNotification } from '@/shared/notification-system'; import { isRemoteWorkspace } from '@/shared/types'; @@ -66,6 +68,9 @@ const CATEGORIES: CategoryInfo[] = [ const SkillsScene: React.FC = () => { const { t } = useTranslation('scenes/skills'); const notification = useNotification(); + const peerDevice = usePeerDeviceModeOptional(); + const remoteConnectionActive = peerDevice?.peerMode.active === true; + const desktopConfigAvailable = isTauriRuntime() && !remoteConnectionActive; const { searchDraft, marketQuery, @@ -93,6 +98,7 @@ const SkillsScene: React.FC = () => { const installed = useInstalledSkills({ searchQuery: installedSearch, activeFilter: installedFilter, + enabled: desktopConfigAvailable, }); const installedSkillNames = useMemo( @@ -118,14 +124,45 @@ const SkillsScene: React.FC = () => { } }, [installed.loading, selectedDetail, selectedInstalledSkill]); + useEffect(() => { + if (desktopConfigAvailable) { + return; + } + setActiveTab('installed'); + setAddFormOpen(false); + setDeleteTarget(null); + setSelectedDetail(null); + }, [desktopConfigAvailable, setAddFormOpen]); + const market = useSkillMarket({ searchQuery: marketQuery, installedSkillNames, pageSize: 15, + enabled: desktopConfigAvailable, onInstalledChanged: async () => { await installed.loadSkills(true); }, }); + const installedSkillAriaLabel = useCallback((skill: SkillInfo) => { + const source = getSkillSourceLabel(skill, t('list.item.unknownSource')); + const scope = market.isRemoteWorkspace + ? skill.level === 'user' + ? t('list.item.localUser') + : t('list.item.remoteProject') + : skill.level === 'user' + ? t('list.item.user') + : t('list.item.project'); + return [ + skill.name, + source, + scope, + skill.isShadowed + ? t('list.item.shadowedTooltip', { + source: coverageSourceBySkillKey.get(skill.key) ?? t('list.item.unknownSource'), + }) + : null, + ].filter(Boolean).join('. '); + }, [coverageSourceBySkillKey, market.isRemoteWorkspace, t]); const refetchSkillsScene = useCallback(async () => { await Promise.all([installed.loadSkills(true), market.refresh()]); @@ -199,6 +236,7 @@ const SkillsScene: React.FC = () => { @@ -208,7 +246,7 @@ const SkillsScene: React.FC = () => { {activeTab === 'installed' && (
-
- + }
- {installedFilter === 'suite' ? ( + {!desktopConfigAvailable ? ( +
+ + {t(remoteConnectionActive ? 'list.remoteUnavailable' : 'list.desktopUnavailable')} +
+ ) : installedFilter === 'suite' ? ( ) : ( <> @@ -281,7 +324,14 @@ const SkillsScene: React.FC = () => { {!installed.loading && installed.error && (
- {installed.error} + {t('list.loadFailed')} +
)} @@ -316,7 +366,7 @@ const SkillsScene: React.FC = () => { setSelectedDetail({ type: 'installed', skillKey: skill.key }); } }} - aria-label={skill.name} + aria-label={installedSkillAriaLabel(skill)} data-testid="skill-list-item" data-skill-key={skill.key} data-skill-id={skill.key} @@ -343,17 +393,6 @@ const SkillsScene: React.FC = () => {
- {skill.isShadowed && ( - - - - {t('list.item.shadowed')} - - - )} {getSkillSourceLabel(skill, t('list.item.unknownSource'))} @@ -371,18 +410,16 @@ const SkillsScene: React.FC = () => { ? t('list.item.user') : t('list.item.project')} - {skill.path && ( - + {skill.isShadowed && ( + + + + {t('list.item.shadowed')} + + )}
@@ -451,7 +488,7 @@ const SkillsScene: React.FC = () => { )} - {activeTab === 'discover' && ( + {desktopConfigAvailable && activeTab === 'discover' && (
@@ -611,7 +648,7 @@ const SkillsScene: React.FC = () => {
setSelectedDetail(null)} icon={selectedMarketSkill ? : } iconGradient={getCardGradient( @@ -779,7 +816,7 @@ const SkillsScene: React.FC = () => { { installed.resetForm(); setAddFormOpen(false); @@ -882,10 +919,10 @@ const SkillsScene: React.FC = () => { setDeleteTarget(null)} onConfirm={async () => { - if (!deleteTarget || !canDeleteSkill(deleteTarget)) { + if (!desktopConfigAvailable || !deleteTarget || !canDeleteSkill(deleteTarget)) { setDeleteTarget(null); return; } diff --git a/src/web-ui/src/app/scenes/skills/hooks/useInstalledSkills.test.tsx b/src/web-ui/src/app/scenes/skills/hooks/useInstalledSkills.test.tsx new file mode 100644 index 000000000..a63b733d0 --- /dev/null +++ b/src/web-ui/src/app/scenes/skills/hooks/useInstalledSkills.test.tsx @@ -0,0 +1,198 @@ +// @vitest-environment jsdom + +import React, { act } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import type { SkillInfo } from '@/infrastructure/config/types'; +import { useInstalledSkills } from './useInstalledSkills'; + +const getSkillConfigsMock = vi.hoisted(() => vi.fn()); +const deleteSkillMock = vi.hoisted(() => vi.fn()); +const validateSkillPathMock = vi.hoisted(() => vi.fn()); +const notificationMocks = vi.hoisted(() => ({ + success: vi.fn(), + warning: vi.fn(), + error: vi.fn(), +})); + +vi.mock('@tauri-apps/plugin-dialog', () => ({ open: vi.fn() })); +vi.mock('react-i18next', () => ({ + useTranslation: () => ({ t: (key: string) => key }), +})); +vi.mock('@/infrastructure/api', () => ({ + configAPI: { + getSkillConfigs: getSkillConfigsMock, + validateSkillPath: validateSkillPathMock, + addSkill: vi.fn(), + deleteSkill: deleteSkillMock, + }, +})); +vi.mock('@/infrastructure/hooks/useWorkspaceManagerSync', () => ({ + useWorkspaceManagerSync: () => ({ + workspacePath: 'D:/workspace/project', + hasWorkspace: true, + isRemoteWorkspace: false, + }), +})); +vi.mock('@/shared/notification-system', () => ({ + useNotification: () => notificationMocks, +})); + +let currentInstalled: ReturnType | null = null; + +function Harness({ enabled }: { enabled: boolean }) { + const installed = useInstalledSkills({ + searchQuery: '', + activeFilter: 'all', + enabled, + }); + currentInstalled = installed; + return {installed.loading ? 'loading' : 'idle'}; +} + +describe('useInstalledSkills', () => { + let container: HTMLDivElement; + let root: Root; + + beforeEach(() => { + container = document.createElement('div'); + document.body.appendChild(container); + root = createRoot(container); + getSkillConfigsMock.mockReset().mockResolvedValue([]); + deleteSkillMock.mockReset().mockResolvedValue(undefined); + validateSkillPathMock.mockReset().mockResolvedValue({ valid: true, name: 'test' }); + notificationMocks.success.mockReset(); + notificationMocks.warning.mockReset(); + notificationMocks.error.mockReset(); + currentInstalled = null; + }); + + afterEach(async () => { + await act(async () => root.unmount()); + container.remove(); + vi.useRealTimers(); + }); + + it('does not query desktop skill configuration during a remote connection', async () => { + await act(async () => { + root.render(); + await Promise.resolve(); + }); + + expect(getSkillConfigsMock).not.toHaveBeenCalled(); + expect(container.textContent).toBe('idle'); + + await act(async () => { + root.render(); + await Promise.resolve(); + }); + + expect(getSkillConfigsMock).toHaveBeenCalledTimes(1); + }); + + it('ignores a desktop skill load that finishes after switching away', async () => { + let resolveLoad: ((skills: SkillInfo[]) => void) | undefined; + getSkillConfigsMock.mockReturnValueOnce(new Promise((resolve) => { + resolveLoad = resolve; + })); + + await act(async () => { + root.render(); + await Promise.resolve(); + }); + await act(async () => { + root.render(); + await Promise.resolve(); + }); + await act(async () => { + resolveLoad?.([{ + key: 'bitfun:user:test', + name: 'test', + description: '', + path: 'D:/skills/test', + level: 'user', + sourceSlot: 'bitfun-user', + sourceId: 'bitfun', + dirName: 'test', + isBuiltin: false, + }]); + await Promise.resolve(); + }); + + expect(currentInstalled?.skills).toEqual([]); + expect(container.textContent).toBe('idle'); + }); + + it('does not notify or reload after a pending delete loses desktop capability', async () => { + let resolveDelete: (() => void) | undefined; + deleteSkillMock.mockReturnValueOnce(new Promise((resolve) => { + resolveDelete = resolve; + })); + const skill: SkillInfo = { + key: 'bitfun:user:test', + name: 'test', + description: '', + path: 'D:/skills/test', + level: 'user', + sourceSlot: 'bitfun-user', + sourceId: 'bitfun', + dirName: 'test', + isBuiltin: false, + }; + + await act(async () => { + root.render(); + await Promise.resolve(); + }); + let deletion: Promise | undefined; + await act(async () => { + deletion = currentInstalled?.handleDelete(skill); + await Promise.resolve(); + }); + await act(async () => { + root.render(); + await Promise.resolve(); + }); + await act(async () => { + resolveDelete?.(); + await deletion; + }); + + expect(notificationMocks.success).not.toHaveBeenCalled(); + expect(notificationMocks.error).not.toHaveBeenCalled(); + expect(getSkillConfigsMock).toHaveBeenCalledTimes(1); + }); + + it('ignores path validation that finishes after switching away', async () => { + vi.useFakeTimers(); + let resolveValidation: ((result: { valid: boolean; name: string }) => void) | undefined; + validateSkillPathMock.mockReturnValueOnce(new Promise((resolve) => { + resolveValidation = resolve; + })); + + await act(async () => { + root.render(); + await Promise.resolve(); + }); + await act(async () => { + currentInstalled?.setFormPath('D:/skills/test'); + await Promise.resolve(); + }); + await act(async () => { + await vi.advanceTimersByTimeAsync(300); + }); + expect(validateSkillPathMock).toHaveBeenCalledTimes(1); + + await act(async () => { + root.render(); + await Promise.resolve(); + }); + await act(async () => { + resolveValidation?.({ valid: true, name: 'test' }); + await Promise.resolve(); + }); + + expect(currentInstalled?.validationResult).toBeNull(); + expect(currentInstalled?.isValidating).toBe(false); + }); +}); diff --git a/src/web-ui/src/app/scenes/skills/hooks/useInstalledSkills.ts b/src/web-ui/src/app/scenes/skills/hooks/useInstalledSkills.ts index 90aa5ef66..94e7171c5 100644 --- a/src/web-ui/src/app/scenes/skills/hooks/useInstalledSkills.ts +++ b/src/web-ui/src/app/scenes/skills/hooks/useInstalledSkills.ts @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; +import { useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react'; import { open } from '@tauri-apps/plugin-dialog'; import { useTranslation } from 'react-i18next'; import { configAPI } from '@/infrastructure/api'; @@ -14,9 +14,14 @@ const log = createLogger('SkillsScene:useInstalledSkills'); interface UseInstalledSkillsOptions { searchQuery: string; activeFilter: InstalledFilter; + enabled?: boolean; } -export function useInstalledSkills({ searchQuery, activeFilter }: UseInstalledSkillsOptions) { +export function useInstalledSkills({ + searchQuery, + activeFilter, + enabled = true, +}: UseInstalledSkillsOptions) { const { t } = useTranslation('scenes/skills'); const notification = useNotification(); const { workspacePath, hasWorkspace, isRemoteWorkspace } = useWorkspaceManagerSync(); @@ -31,8 +36,33 @@ export function useInstalledSkills({ searchQuery, activeFilter }: UseInstalledSk const [isValidating, setIsValidating] = useState(false); const [isAdding, setIsAdding] = useState(false); const loadRequestIdRef = useRef(0); + const validationRequestIdRef = useRef(0); + const capabilityKey = `${enabled}\u0000${workspacePath ?? ''}\u0000${isRemoteWorkspace}`; + const capabilityRef = useRef({ key: capabilityKey, epoch: 0, enabled }); + useLayoutEffect(() => { + if (capabilityRef.current.key !== capabilityKey) { + capabilityRef.current = { + key: capabilityKey, + epoch: capabilityRef.current.epoch + 1, + enabled, + }; + } else { + capabilityRef.current.enabled = enabled; + } + }, [capabilityKey, enabled]); + + const currentCapabilityEpoch = useCallback((): number | null => ( + capabilityRef.current.enabled ? capabilityRef.current.epoch : null + ), []); + const capabilityIsCurrent = useCallback((epoch: number): boolean => ( + capabilityRef.current.enabled && capabilityRef.current.epoch === epoch + ), []); const loadSkills = useCallback(async (forceRefresh?: boolean) => { + const capabilityEpoch = currentCapabilityEpoch(); + if (capabilityEpoch === null) { + return; + } const requestId = ++loadRequestIdRef.current; try { @@ -42,28 +72,44 @@ export function useInstalledSkills({ searchQuery, activeFilter }: UseInstalledSk forceRefresh, workspacePath: workspacePath || undefined, }); - if (requestId !== loadRequestIdRef.current) { + if (requestId !== loadRequestIdRef.current || !capabilityIsCurrent(capabilityEpoch)) { return; } setSkills(list); } catch (err) { - if (requestId !== loadRequestIdRef.current) { + if (requestId !== loadRequestIdRef.current || !capabilityIsCurrent(capabilityEpoch)) { return; } log.error('Failed to load skills', err); setError(err instanceof Error ? err.message : String(err)); } finally { - if (requestId === loadRequestIdRef.current) { + if (requestId === loadRequestIdRef.current && capabilityIsCurrent(capabilityEpoch)) { setLoading(false); } } - }, [workspacePath]); + }, [capabilityIsCurrent, currentCapabilityEpoch, workspacePath]); useEffect(() => { - loadSkills(); - }, [loadSkills]); + loadRequestIdRef.current += 1; + validationRequestIdRef.current += 1; + setValidationResult(null); + setIsValidating(false); + setIsAdding(false); + if (!enabled) { + setSkills([]); + setError(null); + setLoading(false); + return; + } + void loadSkills(); + }, [capabilityKey, enabled, loadSkills]); const validatePath = useCallback(async (path: string) => { + const capabilityEpoch = currentCapabilityEpoch(); + if (capabilityEpoch === null) { + return; + } + const requestId = ++validationRequestIdRef.current; if (!path.trim()) { setValidationResult(null); return; @@ -71,38 +117,59 @@ export function useInstalledSkills({ searchQuery, activeFilter }: UseInstalledSk try { setIsValidating(true); const result = await configAPI.validateSkillPath(path); + if ( + requestId !== validationRequestIdRef.current + || !capabilityIsCurrent(capabilityEpoch) + ) { + return; + } setValidationResult(result); } catch (err) { + if ( + requestId !== validationRequestIdRef.current + || !capabilityIsCurrent(capabilityEpoch) + ) { + return; + } setValidationResult({ valid: false, error: err instanceof Error ? err.message : String(err), }); } finally { - setIsValidating(false); + if ( + requestId === validationRequestIdRef.current + && capabilityIsCurrent(capabilityEpoch) + ) { + setIsValidating(false); + } } - }, []); + }, [capabilityIsCurrent, currentCapabilityEpoch]); useEffect(() => { const timer = window.setTimeout(() => { validatePath(formPath); }, 300); return () => window.clearTimeout(timer); - }, [formPath, validatePath]); + }, [capabilityKey, formPath, validatePath]); const handleBrowse = useCallback(async () => { + const capabilityEpoch = currentCapabilityEpoch(); + if (capabilityEpoch === null) { + return; + } try { const selected = await open({ directory: true, multiple: false, title: t('form.path.label'), }); - if (selected) { + if (selected && capabilityIsCurrent(capabilityEpoch)) { setFormPath(selected as string); } } catch (err) { log.error('Failed to open file dialog', err); } - }, [t]); + }, [capabilityIsCurrent, currentCapabilityEpoch, t]); const resetForm = useCallback(() => { setFormPath(''); @@ -111,6 +178,10 @@ export function useInstalledSkills({ searchQuery, activeFilter }: UseInstalledSk }, []); const handleAdd = useCallback(async () => { + const capabilityEpoch = currentCapabilityEpoch(); + if (capabilityEpoch === null) { + return false; + } if (!validationResult?.valid || !formPath.trim()) { notification.warning(t('messages.invalidPath')); return false; @@ -130,11 +201,17 @@ export function useInstalledSkills({ searchQuery, activeFilter }: UseInstalledSk level: formLevel, workspacePath: workspacePath || undefined, }); + if (!capabilityIsCurrent(capabilityEpoch)) { + return false; + } notification.success(t('messages.addSuccess', { name: validationResult.name })); resetForm(); await loadSkills(true); - return true; + return capabilityIsCurrent(capabilityEpoch); } catch (err) { + if (!capabilityIsCurrent(capabilityEpoch)) { + return false; + } notification.error( t('messages.addFailed', { error: err instanceof Error ? err.message : String(err), @@ -142,11 +219,17 @@ export function useInstalledSkills({ searchQuery, activeFilter }: UseInstalledSk ); return false; } finally { - setIsAdding(false); + if (capabilityIsCurrent(capabilityEpoch)) { + setIsAdding(false); + } } - }, [formLevel, formPath, hasWorkspace, isRemoteWorkspace, loadSkills, notification, resetForm, t, validationResult, workspacePath]); + }, [capabilityIsCurrent, currentCapabilityEpoch, formLevel, formPath, hasWorkspace, isRemoteWorkspace, loadSkills, notification, resetForm, t, validationResult, workspacePath]); const handleDelete = useCallback(async (skill: SkillInfo) => { + const capabilityEpoch = currentCapabilityEpoch(); + if (capabilityEpoch === null) { + return false; + } if (!canDeleteSkill(skill)) { return false; } @@ -155,10 +238,16 @@ export function useInstalledSkills({ searchQuery, activeFilter }: UseInstalledSk skillKey: skill.key, workspacePath: workspacePath || undefined, }); + if (!capabilityIsCurrent(capabilityEpoch)) { + return false; + } notification.success(t('messages.deleteSuccess', { name: skill.name })); await loadSkills(true); - return true; + return capabilityIsCurrent(capabilityEpoch); } catch (err) { + if (!capabilityIsCurrent(capabilityEpoch)) { + return false; + } notification.error( t('messages.deleteFailed', { error: err instanceof Error ? err.message : String(err), @@ -166,7 +255,7 @@ export function useInstalledSkills({ searchQuery, activeFilter }: UseInstalledSk ); return false; } - }, [loadSkills, notification, t, workspacePath]); + }, [capabilityIsCurrent, currentCapabilityEpoch, loadSkills, notification, t, workspacePath]); const normalizedQuery = searchQuery.trim().toLowerCase(); diff --git a/src/web-ui/src/app/scenes/skills/hooks/useSkillMarket.test.tsx b/src/web-ui/src/app/scenes/skills/hooks/useSkillMarket.test.tsx new file mode 100644 index 000000000..1d33e338e --- /dev/null +++ b/src/web-ui/src/app/scenes/skills/hooks/useSkillMarket.test.tsx @@ -0,0 +1,163 @@ +// @vitest-environment jsdom + +import React, { act } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import type { SkillMarketItem } from '@/infrastructure/config/types'; +import { useSkillMarket } from './useSkillMarket'; + +const listSkillMarketMock = vi.hoisted(() => vi.fn()); +const searchSkillMarketMock = vi.hoisted(() => vi.fn()); +const downloadSkillMarketMock = vi.hoisted(() => vi.fn()); +const installedChangedMock = vi.hoisted(() => vi.fn()); +const notificationMocks = vi.hoisted(() => ({ + success: vi.fn(), + warning: vi.fn(), + error: vi.fn(), +})); + +vi.mock('react-i18next', () => ({ + useTranslation: () => ({ t: (key: string) => key }), +})); +vi.mock('@/infrastructure/api', () => ({ + configAPI: { + listSkillMarket: listSkillMarketMock, + searchSkillMarket: searchSkillMarketMock, + downloadSkillMarket: downloadSkillMarketMock, + }, +})); +vi.mock('@/infrastructure/hooks/useWorkspaceManagerSync', () => ({ + useWorkspaceManagerSync: () => ({ + workspacePath: 'D:/workspace/project', + hasWorkspace: true, + isRemoteWorkspace: false, + }), +})); +vi.mock('@/shared/notification-system', () => ({ + useNotification: () => notificationMocks, +})); + +let currentMarket: ReturnType | null = null; + +function Harness({ enabled }: { enabled: boolean }) { + const market = useSkillMarket({ + searchQuery: '', + installedSkillNames: new Set(), + enabled, + onInstalledChanged: installedChangedMock, + }); + currentMarket = market; + return {market.marketLoading ? 'loading' : 'idle'}; +} + +describe('useSkillMarket', () => { + let container: HTMLDivElement; + let root: Root; + + beforeEach(() => { + container = document.createElement('div'); + document.body.appendChild(container); + root = createRoot(container); + listSkillMarketMock.mockReset().mockResolvedValue([]); + searchSkillMarketMock.mockReset().mockResolvedValue([]); + downloadSkillMarketMock.mockReset(); + installedChangedMock.mockReset(); + notificationMocks.success.mockReset(); + notificationMocks.warning.mockReset(); + notificationMocks.error.mockReset(); + currentMarket = null; + }); + + afterEach(async () => { + await act(async () => root.unmount()); + container.remove(); + }); + + it('does not query the skill market outside the desktop app', async () => { + await act(async () => { + root.render(); + await Promise.resolve(); + }); + + expect(listSkillMarketMock).not.toHaveBeenCalled(); + expect(searchSkillMarketMock).not.toHaveBeenCalled(); + expect(downloadSkillMarketMock).not.toHaveBeenCalled(); + expect(container.textContent).toBe('idle'); + + await act(async () => { + root.render(); + await Promise.resolve(); + }); + + expect(listSkillMarketMock).toHaveBeenCalledTimes(1); + }); + + it('ignores a market load that finishes after switching away', async () => { + let resolveLoad: ((skills: SkillMarketItem[]) => void) | undefined; + listSkillMarketMock.mockReturnValueOnce(new Promise((resolve) => { + resolveLoad = resolve; + })); + + await act(async () => { + root.render(); + await Promise.resolve(); + }); + await act(async () => { + root.render(); + await Promise.resolve(); + }); + await act(async () => { + resolveLoad?.([{ + id: 'test', + name: 'test', + description: '', + source: 'test', + installs: 0, + url: 'https://example.com/test', + installId: 'test', + }]); + await Promise.resolve(); + }); + + expect(currentMarket?.marketSkills).toEqual([]); + expect(container.textContent).toBe('idle'); + }); + + it('does not notify or reload after a pending download loses desktop capability', async () => { + let resolveDownload: ((result: { installedSkills: string[] }) => void) | undefined; + downloadSkillMarketMock.mockReturnValueOnce(new Promise((resolve) => { + resolveDownload = resolve; + })); + const skill: SkillMarketItem = { + id: 'test', + name: 'test', + description: '', + source: 'test', + installs: 0, + url: 'https://example.com/test', + installId: 'test', + }; + + await act(async () => { + root.render(); + await Promise.resolve(); + }); + let download: Promise | undefined; + await act(async () => { + download = currentMarket?.handleDownload(skill, 'user'); + await Promise.resolve(); + }); + await act(async () => { + root.render(); + await Promise.resolve(); + }); + await act(async () => { + resolveDownload?.({ installedSkills: ['test'] }); + await download; + }); + + expect(notificationMocks.success).not.toHaveBeenCalled(); + expect(notificationMocks.error).not.toHaveBeenCalled(); + expect(installedChangedMock).not.toHaveBeenCalled(); + }); +}); diff --git a/src/web-ui/src/app/scenes/skills/hooks/useSkillMarket.ts b/src/web-ui/src/app/scenes/skills/hooks/useSkillMarket.ts index 3b7348eaa..e7b7dcfe2 100644 --- a/src/web-ui/src/app/scenes/skills/hooks/useSkillMarket.ts +++ b/src/web-ui/src/app/scenes/skills/hooks/useSkillMarket.ts @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useMemo, useState } from 'react'; +import { useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react'; import { useTranslation } from 'react-i18next'; import { configAPI } from '@/infrastructure/api'; import type { SkillLevel, SkillMarketItem } from '@/infrastructure/config/types'; @@ -16,6 +16,7 @@ interface UseSkillMarketOptions { installedSkillNames: Set; onInstalledChanged?: () => Promise | void; pageSize?: number; + enabled?: boolean; } export function useSkillMarket({ @@ -23,6 +24,7 @@ export function useSkillMarket({ installedSkillNames, onInstalledChanged, pageSize = DEFAULT_PAGE_SIZE, + enabled = true, }: UseSkillMarketOptions) { const { t } = useTranslation('scenes/skills'); const notification = useNotification(); @@ -35,6 +37,27 @@ export function useSkillMarket({ const [downloadingPackage, setDownloadingPackage] = useState(null); const [currentPage, setCurrentPage] = useState(0); const [hasMore, setHasMore] = useState(true); + const marketRequestIdRef = useRef(0); + const capabilityKey = `${enabled}\u0000${workspacePath ?? ''}\u0000${isRemoteWorkspace}`; + const capabilityRef = useRef({ key: capabilityKey, epoch: 0, enabled }); + useLayoutEffect(() => { + if (capabilityRef.current.key !== capabilityKey) { + capabilityRef.current = { + key: capabilityKey, + epoch: capabilityRef.current.epoch + 1, + enabled, + }; + } else { + capabilityRef.current.enabled = enabled; + } + }, [capabilityKey, enabled]); + + const currentCapabilityEpoch = useCallback((): number | null => ( + capabilityRef.current.enabled ? capabilityRef.current.epoch : null + ), []); + const capabilityIsCurrent = useCallback((epoch: number): boolean => ( + capabilityRef.current.enabled && capabilityRef.current.epoch === epoch + ), []); const fetchSkills = useCallback(async (query: string | undefined, limit: number) => { const normalized = query?.trim(); @@ -44,24 +67,49 @@ export function useSkillMarket({ }, []); const loadFirstPage = useCallback(async (query?: string) => { + const capabilityEpoch = currentCapabilityEpoch(); + if (capabilityEpoch === null) { + return; + } + const requestId = ++marketRequestIdRef.current; + setMarketLoading(true); setMarketError(null); setCurrentPage(0); try { const skillList = await fetchSkills(query, pageSize); + if (requestId !== marketRequestIdRef.current || !capabilityIsCurrent(capabilityEpoch)) { + return; + } setMarketSkills(skillList); setHasMore(skillList.length >= pageSize); } catch (err) { + if (requestId !== marketRequestIdRef.current || !capabilityIsCurrent(capabilityEpoch)) { + return; + } log.error('Failed to load skill market', err); setMarketError(err instanceof Error ? err.message : String(err)); } finally { - setMarketLoading(false); + if (requestId === marketRequestIdRef.current && capabilityIsCurrent(capabilityEpoch)) { + setMarketLoading(false); + } } - }, [fetchSkills, pageSize]); + }, [capabilityIsCurrent, currentCapabilityEpoch, fetchSkills, pageSize]); useEffect(() => { + marketRequestIdRef.current += 1; + if (!enabled) { + setMarketSkills([]); + setMarketLoading(false); + setLoadingMore(false); + setMarketError(null); + setDownloadingPackage(null); + setCurrentPage(0); + setHasMore(false); + return; + } loadFirstPage(searchQuery || undefined); - }, [loadFirstPage, searchQuery]); + }, [capabilityKey, enabled, loadFirstPage, searchQuery]); const refresh = useCallback(async () => { await loadFirstPage(searchQuery || undefined); @@ -97,10 +145,19 @@ export function useSkillMarket({ ), [currentPage, displayMarketSkills, pageSize]); const goToPrevPage = useCallback(() => { + if (currentCapabilityEpoch() === null) { + return; + } setCurrentPage((page) => Math.max(0, page - 1)); - }, []); + }, [currentCapabilityEpoch]); const goToNextPage = useCallback(async () => { + const capabilityEpoch = currentCapabilityEpoch(); + if (capabilityEpoch === null) { + return; + } + const requestId = ++marketRequestIdRef.current; + const nextPage = currentPage + 1; const neededCount = Math.min((nextPage + 1) * pageSize, MAX_TOTAL_SKILLS); @@ -118,18 +175,31 @@ export function useSkillMarket({ try { setLoadingMore(true); const skillList = await fetchSkills(searchQuery || undefined, neededCount); + if (requestId !== marketRequestIdRef.current || !capabilityIsCurrent(capabilityEpoch)) { + return; + } setMarketSkills(skillList); const hitCap = neededCount >= MAX_TOTAL_SKILLS; setHasMore(!hitCap && skillList.length >= neededCount); } catch (err) { + if (requestId !== marketRequestIdRef.current || !capabilityIsCurrent(capabilityEpoch)) { + return; + } log.error('Failed to load more skills', err); setCurrentPage(currentPage); } finally { - setLoadingMore(false); + if (requestId === marketRequestIdRef.current && capabilityIsCurrent(capabilityEpoch)) { + setLoadingMore(false); + } } - }, [currentPage, displayMarketSkills.length, fetchSkills, hasMore, pageSize, searchQuery]); + }, [capabilityIsCurrent, currentCapabilityEpoch, currentPage, displayMarketSkills.length, fetchSkills, hasMore, pageSize, searchQuery]); const handleDownload = useCallback(async (skill: SkillMarketItem, targetLevel: SkillLevel = 'project') => { + const capabilityEpoch = currentCapabilityEpoch(); + if (capabilityEpoch === null) { + return; + } + const resolvedLevel: SkillLevel = isRemoteWorkspace ? 'user' : targetLevel; if (resolvedLevel === 'project' && !hasWorkspace) { notification.warning(t('messages.noWorkspace')); @@ -142,19 +212,27 @@ export function useSkillMarket({ level: resolvedLevel, workspacePath: resolvedLevel === 'project' ? workspacePath || undefined : undefined, }); + if (!capabilityIsCurrent(capabilityEpoch)) { + return; + } const installedName = result.installedSkills[0] ?? skill.name; notification.success(t('messages.marketDownloadSuccess', { name: installedName })); await onInstalledChanged?.(); } catch (err) { + if (!capabilityIsCurrent(capabilityEpoch)) { + return; + } notification.error( t('messages.marketDownloadFailed', { error: err instanceof Error ? err.message : String(err), }), ); } finally { - setDownloadingPackage(null); + if (capabilityIsCurrent(capabilityEpoch)) { + setDownloadingPackage(null); + } } - }, [hasWorkspace, isRemoteWorkspace, notification, onInstalledChanged, t, workspacePath]); + }, [capabilityIsCurrent, currentCapabilityEpoch, hasWorkspace, isRemoteWorkspace, notification, onInstalledChanged, t, workspacePath]); return { marketSkills: paginatedSkills, diff --git a/src/web-ui/src/infrastructure/config/components/ExternalMcpOverview.test.tsx b/src/web-ui/src/infrastructure/config/components/ExternalMcpOverview.test.tsx index 77641f972..239a0585e 100644 --- a/src/web-ui/src/infrastructure/config/components/ExternalMcpOverview.test.tsx +++ b/src/web-ui/src/infrastructure/config/components/ExternalMcpOverview.test.tsx @@ -8,6 +8,7 @@ import ExternalMcpOverview from './ExternalMcpOverview'; const getSnapshotMock = vi.hoisted(() => vi.fn()); const workspaceState = vi.hoisted(() => ({ path: 'D:/workspace/project' })); +const peerState = vi.hoisted(() => ({ deviceId: '' })); const warnMock = vi.hoisted(() => vi.fn()); const apiErrorState = vi.hoisted(() => ({ ExternalSourceApiError: class ExternalSourceApiError extends Error { @@ -30,7 +31,17 @@ vi.mock('react-i18next', () => ({ })); vi.mock('@/infrastructure/contexts/WorkspaceContext', () => ({ - useCurrentWorkspace: () => ({ workspacePath: workspaceState.path }), + useCurrentWorkspace: () => ({ + workspace: { id: workspaceState.path, workspaceKind: 'local' }, + workspacePath: workspaceState.path, + }), +})); +vi.mock('@/infrastructure/peer-device/PeerDeviceContext', () => ({ + usePeerDeviceModeOptional: () => ({ + peerMode: peerState.deviceId + ? { active: true, deviceId: peerState.deviceId, deviceName: peerState.deviceId } + : { active: false }, + }), })); vi.mock('@/infrastructure/api/service-api/ExternalSourcesAPI', () => ({ @@ -121,6 +132,7 @@ describe('ExternalMcpOverview', () => { getSnapshotMock.mockReset().mockResolvedValue(snapshot); warnMock.mockReset(); workspaceState.path = 'D:/workspace/project'; + peerState.deviceId = ''; useSettingsStore.setState({ activeTab: 'mcp-tools', searchQuery: '' }); }); @@ -194,6 +206,42 @@ describe('ExternalMcpOverview', () => { expect(container.textContent).not.toContain('OpenCode'); }); + it('does not project a snapshot returned by the previous Peer Host', async () => { + let resolvePeerA: ((value: typeof snapshot) => void) | undefined; + getSnapshotMock + .mockImplementationOnce(() => new Promise((resolve) => { + resolvePeerA = resolve; + })) + .mockResolvedValueOnce({ + ...snapshot, + mcpServers: [{ + ...snapshot.mcpServers[0], + candidateId: 'peer-b-docs', + definition: { ...snapshot.mcpServers[0].definition, name: 'peer-b-docs' }, + }], + }); + + peerState.deviceId = 'peer-a'; + await act(async () => { + root.render(); + await Promise.resolve(); + }); + peerState.deviceId = 'peer-b'; + await act(async () => { + root.render(); + await Promise.resolve(); + await Promise.resolve(); + }); + await act(async () => { + resolvePeerA?.(snapshot); + await Promise.resolve(); + }); + + expect(Array.from( + container.querySelectorAll('[data-testid="external-mcp-item"] .bitfun-collection-item__name'), + ).map((node) => node.textContent)).toEqual(['peer-b-docs']); + }); + it('logs typed failure facts without logging raw error details', async () => { getSnapshotMock.mockRejectedValue(new apiErrorState.ExternalSourceApiError( 'unavailable', @@ -276,6 +324,32 @@ describe('ExternalMcpOverview', () => { } }); + it('keeps the last external MCP snapshot when a refresh fails', async () => { + vi.useFakeTimers(); + try { + getSnapshotMock + .mockResolvedValueOnce({ ...snapshot, discoveryPending: true }) + .mockRejectedValueOnce(new Error('refresh failed')); + + await act(async () => { + root.render(); + await Promise.resolve(); + await Promise.resolve(); + }); + await act(async () => { + await vi.advanceTimersByTimeAsync(750); + await Promise.resolve(); + }); + + expect(container.textContent).toContain('docs'); + expect(container.textContent).toContain('external.status.stale'); + expect(container.querySelector('[aria-label="external.retry"]')).not.toBeNull(); + expect(container.textContent).not.toContain('external.unavailable'); + } finally { + vi.useRealTimers(); + } + }); + it('cancels pending discovery polling when the workspace changes', async () => { vi.useFakeTimers(); try { diff --git a/src/web-ui/src/infrastructure/config/components/ExternalMcpOverview.tsx b/src/web-ui/src/infrastructure/config/components/ExternalMcpOverview.tsx index e74c05180..950616319 100644 --- a/src/web-ui/src/infrastructure/config/components/ExternalMcpOverview.tsx +++ b/src/web-ui/src/infrastructure/config/components/ExternalMcpOverview.tsx @@ -4,6 +4,7 @@ import { useTranslation } from 'react-i18next'; import { IconButton } from '@/component-library'; import { useSettingsStore } from '@/app/scenes/settings/settingsStore'; import { useCurrentWorkspace } from '@/infrastructure/contexts/WorkspaceContext'; +import { usePeerDeviceModeOptional } from '@/infrastructure/peer-device/PeerDeviceContext'; import { type ExternalMcpActivation, type ExternalMcpCatalogEntry, @@ -14,6 +15,7 @@ import { } from '@/infrastructure/api/service-api/ExternalSourcesAPI'; import { createLogger } from '@/shared/utils/logger'; import { ConfigCollectionItem, ConfigPageSection } from './common'; +import { externalSourceRequestScopeKey } from './externalSourceRequestScope'; const log = createLogger('ExternalMcpOverview'); const DISCOVERY_POLL_DELAYS_MS = [750, 1_500, 3_000, 5_000] as const; @@ -108,31 +110,44 @@ const ExternalMcpDetail: React.FC<{ const ExternalMcpOverview: React.FC = () => { const { t } = useTranslation('settings/mcp'); const { t: tShared } = useTranslation('shared'); - const { workspacePath } = useCurrentWorkspace(); + const { workspace, workspacePath } = useCurrentWorkspace(); + const peerDevice = usePeerDeviceModeOptional(); const setSettingsTab = useSettingsStore((state) => state.setActiveTab); const requestIdRef = useRef(0); - const workspaceScope = workspacePath || ''; + const peerDeviceId = peerDevice?.peerMode.active ? peerDevice.peerMode.deviceId : undefined; + const requestScope = externalSourceRequestScopeKey({ + peerDeviceId, + workspaceId: workspace?.id, + workspaceKind: workspace?.workspaceKind, + remoteConnectionId: workspace?.connectionId, + remoteHost: workspace?.sshHost, + workspacePath, + }); const [snapshotState, setSnapshotState] = useState<{ scope: string; snapshot: ExternalSourceCatalogSnapshot | null; } | null>(null); const [loading, setLoading] = useState(true); const [loadFailed, setLoadFailed] = useState(false); - const snapshot = snapshotState?.scope === workspaceScope ? snapshotState.snapshot : null; - const scopedLoading = loading || snapshotState?.scope !== workspaceScope; + const snapshot = snapshotState?.scope === requestScope ? snapshotState.snapshot : null; + const scopedLoading = loading || snapshotState?.scope !== requestScope; const loadSnapshot = useCallback(async () => { const requestId = ++requestIdRef.current; setLoading(true); setLoadFailed(false); try { - const nextSnapshot = await externalSourcesAPI.getSnapshot(workspaceScope || undefined); + const nextSnapshot = await externalSourcesAPI.getSnapshot(workspacePath || undefined); if (requestId === requestIdRef.current) { - setSnapshotState({ scope: workspaceScope, snapshot: nextSnapshot }); + setSnapshotState({ scope: requestScope, snapshot: nextSnapshot }); } } catch (error) { if (requestId === requestIdRef.current) { - setSnapshotState({ scope: workspaceScope, snapshot: null }); + setSnapshotState((current) => ( + current?.scope === requestScope && current.snapshot + ? current + : { scope: requestScope, snapshot: null } + )); setLoadFailed(true); log.warn('Failed to load external MCP summary', safeLoadErrorFacts(error)); } @@ -141,7 +156,7 @@ const ExternalMcpOverview: React.FC = () => { setLoading(false); } } - }, [workspaceScope]); + }, [requestScope, workspacePath]); useEffect(() => { void loadSnapshot(); @@ -300,10 +315,8 @@ const ExternalMcpOverview: React.FC = () => { - {entries.length} {snapshot.discoveryPending ? ( {t('external.status.checking')} @@ -314,6 +327,11 @@ const ExternalMcpOverview: React.FC = () => { {t('external.status.readOnly')} ) : null} + {loadFailed && snapshot ? ( + + {t('external.status.stale')} + + ) : null} {hasMcpDiagnostics ? ( {t('external.status.degraded')} diff --git a/src/web-ui/src/infrastructure/config/components/ExternalSourcesConfig.scss b/src/web-ui/src/infrastructure/config/components/ExternalSourcesConfig.scss index 6d60da3b9..d645bf0bb 100644 --- a/src/web-ui/src/infrastructure/config/components/ExternalSourcesConfig.scss +++ b/src/web-ui/src/infrastructure/config/components/ExternalSourcesConfig.scss @@ -2,16 +2,20 @@ container-name: external-sources; container-type: inline-size; - &__policy-card { - margin-bottom: 18px; - padding: 14px; - border: 1px solid var(--border-base); - border-radius: 12px; - background: var(--color-bg-secondary); + .bitfun-config-page-header__inner { + position: relative; + } + + .bitfun-config-page-section__body { + border: 0; + border-radius: 0; + background: transparent; + } + + .bitfun-config-page-row { + padding: 12px 0; } - &__policy-heading, - &__policy-identity, &__ecosystem-heading, &__policy-actions, &__ecosystem-name { @@ -19,44 +23,19 @@ align-items: center; } - &__policy-heading { - justify-content: space-between; - gap: 16px; - } - - &__policy-identity, &__ecosystem-heading, &__policy-actions { gap: 10px; } - &__policy-icon, - &__ecosystem-mark { - display: grid; - flex: 0 0 auto; - place-items: center; - color: var(--color-accent-500); - background: var(--color-accent-100); - } - - &__policy-icon { - width: 34px; - height: 34px; - border-radius: 10px; - } - - &__policy-title, &__ecosystem-name { color: var(--color-text-primary); font-size: 13px; font-weight: 600; } - &__policy-summary, - &__ecosystem-meta { - margin-top: 2px; - color: var(--color-text-secondary); - font-size: 11px; + &__policy-summary { + color: inherit; } &__policy-summary button { @@ -82,7 +61,6 @@ } } - &__origin-badge, &__inherited-badge, &__override-badge, &__limited-badge { @@ -94,22 +72,14 @@ font-weight: 500; } - &__origin-badge { - margin-left: 7px; - padding: 2px 6px; - color: var(--color-accent-500); - background: var(--color-accent-100); - } - &__scope-bar { display: flex; flex-wrap: wrap; align-items: center; gap: 4px; - margin: 14px 0 10px; - padding: 3px; - border-radius: 9px; - background: var(--color-bg-primary); + min-height: 36px; + padding: 2px 0 0; + background: transparent; > button { display: inline-flex; @@ -164,12 +134,10 @@ display: flex; align-items: center; gap: 8px; - margin-top: 12px; - padding: 8px 10px; - border: 1px solid var(--color-warning); - border-radius: 8px; + padding: 8px 12px; + border-left: 2px solid var(--color-warning); color: var(--color-text-primary); - background: var(--color-bg-primary); + background: color-mix(in srgb, var(--color-warning) 7%, transparent); font-size: 12px; > span { @@ -181,12 +149,9 @@ display: flex; align-items: center; gap: 8px; - margin-bottom: 12px; - padding: 8px 10px; - border: 1px solid var(--border-subtle); - border-radius: 8px; + margin-bottom: 4px; + padding: 0; color: var(--color-text-secondary); - background: var(--color-bg-secondary); font-size: 12px; } @@ -195,19 +160,9 @@ align-items: center; justify-content: space-between; gap: 16px; - padding: 11px 12px; - border: 1px solid var(--border-subtle); - border-radius: 10px; - background: var(--color-bg-primary); - } - - &__ecosystem-mark { - width: 32px; - height: 32px; - border-radius: 8px; - font-size: 10px; - font-weight: 700; - letter-spacing: 0.02em; + padding: 12px 0; + background: transparent; + border-top: 1px solid var(--border-subtle); } &__ecosystem-name { @@ -241,17 +196,17 @@ height: 30px; padding: 0; place-items: center; - border: 1px solid var(--border-subtle); - border-radius: 8px; + border: 0; + border-radius: 6px; color: var(--color-text-secondary); - background: var(--color-bg-secondary); + background: transparent; cursor: pointer; &:hover, &:focus-visible, &[aria-expanded='true'] { color: var(--color-accent-500); - border-color: var(--color-accent-500); + background: var(--color-bg-secondary); } &:focus-visible { @@ -263,8 +218,8 @@ &__capability-grid { display: grid; grid-template-columns: repeat(2, minmax(0, 1fr)); - gap: 8px; - margin-top: 10px; + column-gap: 24px; + padding: 0 0 4px; } &__capability-row { @@ -273,9 +228,8 @@ align-items: center; justify-content: space-between; gap: 10px; - padding: 8px 10px; - border: 1px solid var(--border-subtle); - border-radius: 8px; + padding: 10px 0; + border-bottom: 1px solid var(--border-subtle); color: var(--color-text-primary); font-size: 12px; @@ -298,9 +252,18 @@ &__notice { margin-bottom: 12px; - padding: 10px 12px; - border: 1px solid var(--border-subtle); - border-radius: 8px; + padding: 0; + } + + &__empty { + padding: 4px 0; + } + + &__notice[role='alert'], + &__notice[data-external-attention='true'] { + padding: 8px 12px; + border-left: 2px solid var(--color-warning); + background: color-mix(in srgb, var(--color-warning) 7%, transparent); } &__notice summary { @@ -348,14 +311,13 @@ } &__tool-card { - padding: 12px; - border: 1px solid var(--border-subtle); - border-radius: 8px; - background: var(--color-bg-secondary); + padding: 10px 14px; + border-left: 2px solid var(--border-medium); + background: transparent; overflow-wrap: anywhere; & + & { - margin-top: 10px; + margin-top: 12px; } } @@ -450,3 +412,13 @@ } } } + +@container external-sources (max-width: 520px) { + .bitfun-external-sources-config .bitfun-config-page-header__extra { + position: absolute; + top: 0; + right: 0; + width: auto; + margin-top: 0; + } +} diff --git a/src/web-ui/src/infrastructure/config/components/ExternalSourcesConfig.test.tsx b/src/web-ui/src/infrastructure/config/components/ExternalSourcesConfig.test.tsx index 466934e55..21ea7fbe5 100644 --- a/src/web-ui/src/infrastructure/config/components/ExternalSourcesConfig.test.tsx +++ b/src/web-ui/src/infrastructure/config/components/ExternalSourcesConfig.test.tsx @@ -15,7 +15,8 @@ const chooseSubagentConflictMock = vi.hoisted(() => vi.fn()); const setMcpServerDecisionMock = vi.hoisted(() => vi.fn()); const chooseMcpConflictMock = vi.hoisted(() => vi.fn()); const updateIntegrationPolicyMock = vi.hoisted(() => vi.fn()); -const workspaceState = vi.hoisted(() => ({ path: 'D:/workspace/project' })); +const workspaceState = vi.hoisted(() => ({ path: 'D:/workspace/project', kind: 'normal' })); +const peerState = vi.hoisted(() => ({ deviceId: '' })); vi.mock('react-i18next', () => ({ initReactI18next: { @@ -30,13 +31,23 @@ vi.mock('react-i18next', () => ({ vi.mock('@/infrastructure/contexts/WorkspaceContext', () => ({ useCurrentWorkspace: () => ({ - workspace: { rootPath: workspaceState.path }, + workspace: { id: workspaceState.path, workspaceKind: workspaceState.kind, rootPath: workspaceState.path }, workspacePath: workspaceState.path, }), })); +vi.mock('@/infrastructure/peer-device/PeerDeviceContext', () => ({ + usePeerDeviceModeOptional: () => ({ + peerMode: peerState.deviceId + ? { active: true, deviceId: peerState.deviceId, deviceName: peerState.deviceId } + : { active: false }, + }), +})); vi.mock('@/infrastructure/runtime', () => ({ isTauriRuntime: () => true })); -vi.mock('@/shared/types', () => ({ isRemoteWorkspace: () => false })); +vi.mock('@/shared/types', () => ({ + isRemoteWorkspace: () => false, + WorkspaceKind: { Normal: 'normal', Assistant: 'assistant', Remote: 'remote' }, +})); vi.mock('@/infrastructure/api/service-api/ExternalSourcesAPI', () => ({ externalSourcesAPI: { getSnapshot: getSnapshotMock, @@ -193,6 +204,8 @@ describe('ExternalSourcesConfig', () => { beforeEach(() => { vi.useFakeTimers(); workspaceState.path = 'D:/workspace/project'; + workspaceState.kind = 'normal'; + peerState.deviceId = ''; getSnapshotMock.mockResolvedValue(snapshot); setSourceEnabledMock.mockResolvedValue(snapshot); setConflictChoiceMock.mockResolvedValue({ @@ -239,7 +252,7 @@ describe('ExternalSourcesConfig', () => { }); expect(container.textContent).toContain('policy.title'); - expect(container.textContent).toContain('policy.externalBadge'); + expect(container.textContent).not.toContain('policy.externalBadge'); expect(container.textContent).toContain('OpenCode'); expect(container.textContent).toContain('policy.mode.recommended'); expect(container.textContent).toContain('policy.inherited'); @@ -749,6 +762,94 @@ describe('ExternalSourcesConfig', () => { expect(container.textContent).not.toContain('checkingNonBlocking'); }); + it('waits for each discovery poll before scheduling the next backoff step', async () => { + let resolvePoll: ((value: typeof snapshot) => void) | undefined; + getSnapshotMock + .mockResolvedValueOnce({ + ...snapshot, + discoveryPending: true, + sources: [], + diagnostics: [], + commandConflicts: [], + }) + .mockImplementationOnce(() => new Promise((resolve) => { + resolvePoll = resolve; + })) + .mockResolvedValue(snapshot); + + await act(async () => { + root.render(); + await Promise.resolve(); + }); + await act(async () => vi.advanceTimersByTimeAsync(750)); + expect(getSnapshotMock).toHaveBeenCalledTimes(2); + + await act(async () => vi.advanceTimersByTimeAsync(5_000)); + expect(getSnapshotMock).toHaveBeenCalledTimes(2); + + await act(async () => { + resolvePoll?.({ + ...snapshot, + discoveryPending: true, + sources: [], + diagnostics: [], + commandConflicts: [], + }); + await Promise.resolve(); + }); + await act(async () => vi.advanceTimersByTimeAsync(1_499)); + expect(getSnapshotMock).toHaveBeenCalledTimes(2); + await act(async () => vi.advanceTimersByTimeAsync(1)); + expect(getSnapshotMock).toHaveBeenCalledTimes(3); + }); + + it('keeps polling when a read overlaps a source change', async () => { + let resolveMutation: ((value: typeof snapshot) => void) | undefined; + const pendingSnapshot = { ...snapshot, discoveryPending: true }; + getSnapshotMock + .mockResolvedValueOnce(pendingSnapshot) + .mockResolvedValueOnce(pendingSnapshot) + .mockResolvedValue(snapshot); + setSourceEnabledMock.mockImplementationOnce(() => new Promise((resolve) => { + resolveMutation = resolve; + })); + + await act(async () => { + root.render(); + await Promise.resolve(); + }); + const sourceToggle = container.querySelector( + 'input[aria-label^="sources.toggleLabel"]', + ) as HTMLInputElement; + await act(async () => sourceToggle.click()); + await act(async () => vi.advanceTimersByTimeAsync(750)); + expect(getSnapshotMock).toHaveBeenCalledTimes(2); + + await act(async () => { + resolveMutation?.(pendingSnapshot); + await Promise.resolve(); + }); + await act(async () => vi.advanceTimersByTimeAsync(1_500)); + expect(getSnapshotMock).toHaveBeenCalledTimes(3); + }); + + it('recovers discovery polling after a transient read failure', async () => { + getSnapshotMock + .mockResolvedValueOnce({ ...snapshot, discoveryPending: true }) + .mockRejectedValueOnce(new Error('temporary discovery failure')) + .mockResolvedValue(snapshot); + + await act(async () => { + root.render(); + await Promise.resolve(); + }); + await act(async () => vi.advanceTimersByTimeAsync(750)); + expect(getSnapshotMock).toHaveBeenCalledTimes(2); + await act(async () => vi.advanceTimersByTimeAsync(1_500)); + expect(getSnapshotMock).toHaveBeenCalledTimes(3); + expect(container.textContent).toContain('OpenCode project commands'); + }); + it('distinguishes initial load failures from uncertain mutation results', async () => { getSnapshotMock.mockRejectedValueOnce(new Error('initial load failed')); await act(async () => { @@ -757,6 +858,7 @@ describe('ExternalSourcesConfig', () => { }); expect(container.textContent).toContain('errors.loadFailed'); expect(container.textContent).not.toContain('initial load failed'); + expect(container.textContent).not.toContain('sources.empty'); await act(async () => root.unmount()); container.remove(); @@ -786,8 +888,11 @@ describe('ExternalSourcesConfig', () => { await Promise.resolve(); }); const refresh = Array.from(container.querySelectorAll('button')).find((button) => - button.textContent?.includes('actions.refresh')); - await act(async () => refresh?.click()); + button.getAttribute('aria-label') === 'actions.refresh'); + await act(async () => { + refresh?.click(); + await Promise.resolve(); + }); expect(container.textContent).toContain('errors.refreshFailed'); expect(container.textContent).toContain('OpenCode project commands'); expect(container.textContent).not.toContain('refresh failed'); @@ -1383,6 +1488,47 @@ describe('ExternalSourcesConfig', () => { expect(container.textContent).not.toContain('OpenCode project commands'); }); + it('isolates an in-flight response when the controlling Peer Host changes', async () => { + let resolveFirstHost: ((value: typeof snapshot) => void) | undefined; + getSnapshotMock + .mockImplementationOnce(() => new Promise((resolve) => { + resolveFirstHost = resolve; + })) + .mockResolvedValueOnce({ + ...snapshot, + generation: 2, + sources: [{ + ...snapshot.sources[0], + stableKey: 'peer-b-source', + record: { + ...snapshot.sources[0].record, + displayName: 'Peer B commands', + }, + }], + diagnostics: [], + commandConflicts: [], + }); + + peerState.deviceId = 'peer-a'; + await act(async () => { + root.render(); + await Promise.resolve(); + }); + peerState.deviceId = 'peer-b'; + await act(async () => { + root.render(); + await Promise.resolve(); + await Promise.resolve(); + }); + await act(async () => { + resolveFirstHost?.(snapshot); + await Promise.resolve(); + }); + + expect(container.textContent).toContain('Peer B commands'); + expect(container.textContent).not.toContain('OpenCode project commands'); + }); + it('ignores a source mutation response from the previous workspace', async () => { let resolveMutation: ((value: typeof snapshot) => void) | undefined; const pendingMutation = new Promise((resolve) => { @@ -1463,6 +1609,76 @@ describe('ExternalSourcesConfig', () => { expect(container.textContent).toContain('lifecycle.suppressed'); }); + it('ignores a failed refresh while a source mutation is pending', async () => { + let resolveMutation: ((value: typeof snapshot) => void) | undefined; + getSnapshotMock + .mockResolvedValueOnce(snapshot) + .mockRejectedValueOnce(new Error('stale refresh failure')); + setSourceEnabledMock.mockReturnValue(new Promise((resolve) => { + resolveMutation = resolve; + })); + + await act(async () => { + root.render(); + await Promise.resolve(); + }); + const sourceToggle = container.querySelector( + 'input[aria-label^="sources.toggleLabel"]', + ) as HTMLInputElement; + await act(async () => sourceToggle.click()); + await act(async () => { + window.dispatchEvent(new Event('focus')); + await Promise.resolve(); + }); + + expect(container.textContent).not.toContain('errors.refreshFailed'); + + await act(async () => { + resolveMutation?.({ + ...snapshot, + generation: 2, + sources: [{ ...snapshot.sources[0], lifecycle: 'suppressed' }], + }); + await Promise.resolve(); + }); + + expect(container.textContent).toContain('lifecycle.suppressed'); + expect(container.textContent).not.toContain('errors.refreshFailed'); + }); + + it('gives remote projects a useful unavailable next step', async () => { + workspaceState.kind = 'remote'; + getSnapshotMock.mockRejectedValueOnce({ + code: 'host_unavailable', + message: 'unavailable', + }); + + await act(async () => { + root.render(); + await Promise.resolve(); + }); + + expect(container.textContent).toContain('unavailable.remoteDescription'); + expect(container.textContent).not.toContain('unavailable.hostDescription'); + }); + + it('gives a failed remote connection a useful next step', async () => { + peerState.deviceId = 'remote-device'; + getSnapshotMock.mockRejectedValueOnce({ + code: 'host_unavailable', + message: 'unavailable', + }); + + await act(async () => { + root.render(); + await Promise.resolve(); + }); + + expect(container.textContent).toContain('unavailable.remoteConnectionDescription'); + expect(container.textContent).not.toContain('unavailable.hostDescription'); + expect(container.textContent).not.toContain('unavailable.remoteDescription'); + }); + it('fails closed for a legacy read-only host and never sends a mutation', async () => { const { hostCapabilities: _omitted, ...legacySnapshot } = snapshot; getSnapshotMock.mockResolvedValue(legacySnapshot); diff --git a/src/web-ui/src/infrastructure/config/components/ExternalSourcesConfig.tsx b/src/web-ui/src/infrastructure/config/components/ExternalSourcesConfig.tsx index d42718fa0..60afead1c 100644 --- a/src/web-ui/src/infrastructure/config/components/ExternalSourcesConfig.tsx +++ b/src/web-ui/src/infrastructure/config/components/ExternalSourcesConfig.tsx @@ -1,4 +1,4 @@ -import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react'; +import React, { useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react'; import type { TFunction } from 'i18next'; import { useTranslation } from 'react-i18next'; import { @@ -21,6 +21,8 @@ import { Tooltip, } from '@/component-library'; import { useCurrentWorkspace } from '@/infrastructure/contexts/WorkspaceContext'; +import { usePeerDeviceModeOptional } from '@/infrastructure/peer-device/PeerDeviceContext'; +import { WorkspaceKind } from '@/shared/types'; import { externalSourcesAPI, type ExternalIntegrationAccess, @@ -37,8 +39,16 @@ import { ConfigPageRow, ConfigPageSection, } from './common'; +import { externalSourceRequestScopeKey } from './externalSourceRequestScope'; import './ExternalSourcesConfig.scss'; +const DISCOVERY_POLL_DELAYS_MS = [750, 1_500, 3_000, 5_000] as const; + +type SnapshotLoadResult = + | { status: 'accepted'; snapshot: ExternalSourceCatalogSnapshot } + | { status: 'ignored' } + | { status: 'error' }; + function abbreviatedLocation(location: string): string { const normalized = location.replace(/\\/g, '/'); const segments = normalized.split('/').filter(Boolean); @@ -195,9 +205,10 @@ function activeAgentAvailabilityChanges( const ExternalSourcesConfig: React.FC = () => { const { t } = useTranslation('settings/external-sources'); const { workspace, workspacePath } = useCurrentWorkspace(); + const peerDevice = usePeerDeviceModeOptional(); const translateRef = useRef(t); translateRef.current = t; - const [snapshot, setSnapshot] = useState(null); + const peerDeviceId = peerDevice?.peerMode.active ? peerDevice.peerMode.deviceId : undefined; const [loading, setLoading] = useState(true); const [refreshing, setRefreshing] = useState(false); const [busyKey, setBusyKey] = useState(null); @@ -211,7 +222,11 @@ const ExternalSourcesConfig: React.FC = () => { workspacePath ? 'workspace' : 'user', ); const [expandedEcosystems, setExpandedEcosystems] = useState>(() => new Set()); - const [resetPolicyConfirmOpen, setResetPolicyConfirmOpen] = useState(false); + const [resetPolicyConfirmation, setResetPolicyConfirmation] = useState<{ + requestScope: string; + workspacePath?: string; + preferenceRevision: number; + } | null>(null); const [agentChangeNotice, setAgentChangeNotice] = useState(null); const snapshotRef = useRef(null); const agentChangeNoticeRef = useRef(null); @@ -219,26 +234,35 @@ const ExternalSourcesConfig: React.FC = () => { const acceptedSequence = useRef(0); const pendingMutations = useRef(new Map()); const latestMutationByScope = useRef(new Map()); - const mutationInFlight = useRef(false); + const activeMutation = useRef<{ scope: string; sequence: number } | null>(null); const foregroundSequence = useRef(null); - const requestScope = workspace - ? JSON.stringify({ - id: workspace.id, - kind: workspace.workspaceKind, - connectionId: workspace.connectionId ?? null, - sshHost: workspace.sshHost ?? null, - path: workspacePath, - }) - : '__user_global__'; + const requestScope = externalSourceRequestScopeKey({ + peerDeviceId, + workspaceId: workspace?.id, + workspaceKind: workspace?.workspaceKind, + remoteConnectionId: workspace?.connectionId, + remoteHost: workspace?.sshHost, + workspacePath, + }); + const [snapshotState, setSnapshotState] = useState<{ + scope: string; + snapshot: ExternalSourceCatalogSnapshot; + } | null>(null); + const snapshot = snapshotState?.scope === requestScope ? snapshotState.snapshot : null; const requestScopeRef = useRef(requestScope); - if (requestScopeRef.current !== requestScope) { - requestScopeRef.current = requestScope; - requestSequence.current += 1; - acceptedSequence.current = requestSequence.current; - } + useLayoutEffect(() => { + if (requestScopeRef.current !== requestScope) { + requestScopeRef.current = requestScope; + requestSequence.current += 1; + acceptedSequence.current = requestSequence.current; + snapshotRef.current = null; + agentChangeNoticeRef.current = null; + } + }, [requestScope]); const applySnapshot = useCallback(( next: ExternalSourceCatalogSnapshot, + scope: string, partition: 'all' | 'subagents' = 'all', origin: 'read' | 'mutation' = 'read', ) => { @@ -299,7 +323,7 @@ const ExternalSourcesConfig: React.FC = () => { } snapshotRef.current = selected; - setSnapshot(selected); + setSnapshotState({ scope, snapshot: selected }); }, []); const acceptReadSnapshot = useCallback(( @@ -310,7 +334,7 @@ const ExternalSourcesConfig: React.FC = () => { if (requestScopeRef.current !== scope || sequence < acceptedSequence.current) return false; if (Array.from(pendingMutations.current.values()).includes(scope)) return false; acceptedSequence.current = sequence; - applySnapshot(next); + applySnapshot(next, scope); return true; }, [applySnapshot]); @@ -323,11 +347,14 @@ const ExternalSourcesConfig: React.FC = () => { if (requestScopeRef.current !== scope) return false; if ((latestMutationByScope.current.get(scope) ?? sequence) > sequence) return false; acceptedSequence.current = Math.max(acceptedSequence.current, sequence); - applySnapshot(next, partition, 'mutation'); + applySnapshot(next, scope, partition, 'mutation'); return true; }, [applySnapshot]); - const loadSnapshot = useCallback(async (forceRefresh: boolean, foreground: boolean) => { + const loadSnapshot = useCallback(async ( + forceRefresh: boolean, + foreground: boolean, + ): Promise => { const scope = requestScope; const sequence = ++requestSequence.current; if (foreground) { @@ -336,12 +363,18 @@ const ExternalSourcesConfig: React.FC = () => { } try { const next = await externalSourcesAPI.getSnapshot(workspacePath, forceRefresh); - if (!acceptReadSnapshot(next, scope, sequence)) return; + if (!acceptReadSnapshot(next, scope, sequence)) return { status: 'ignored' }; setError(null); + return { status: 'accepted', snapshot: next }; } catch (loadError) { - if (requestScopeRef.current !== scope || sequence < acceptedSequence.current) return; + if (requestScopeRef.current !== scope + || sequence < acceptedSequence.current + || Array.from(pendingMutations.current.values()).includes(scope)) { + return { status: 'ignored' }; + } acceptedSequence.current = sequence; setError({ kind: 'load', ...externalOperationErrorFacts(loadError) }); + return { status: 'error' }; } finally { if (requestScopeRef.current === scope) { if (sequence >= acceptedSequence.current) setLoading(false); @@ -354,7 +387,7 @@ const ExternalSourcesConfig: React.FC = () => { }, [acceptReadSnapshot, requestScope, workspacePath]); useEffect(() => { - setSnapshot(null); + setSnapshotState(null); snapshotRef.current = null; agentChangeNoticeRef.current = null; setAgentChangeNotice(null); @@ -365,6 +398,7 @@ const ExternalSourcesConfig: React.FC = () => { setReviewingAgentKey(null); setReviewingMcpKey(null); setReviewingMcpConflictKey(null); + setResetPolicyConfirmation(null); setLoading(true); setPolicyScope(workspacePath ? 'workspace' : 'user'); void loadSnapshot(false, false); @@ -383,8 +417,26 @@ const ExternalSourcesConfig: React.FC = () => { useEffect(() => { if (!snapshot?.discoveryPending) return undefined; - const timer = window.setInterval(() => void loadSnapshot(false, false), 750); - return () => window.clearInterval(timer); + let cancelled = false; + let timer: number | undefined; + let attempt = 0; + const schedulePoll = () => { + const delay = DISCOVERY_POLL_DELAYS_MS[ + Math.min(attempt, DISCOVERY_POLL_DELAYS_MS.length - 1) + ]; + timer = window.setTimeout(async () => { + const result = await loadSnapshot(false, false); + if (cancelled) return; + if (result.status === 'accepted' && !result.snapshot.discoveryPending) return; + attempt += 1; + schedulePoll(); + }, delay); + }; + schedulePoll(); + return () => { + cancelled = true; + if (timer !== undefined) window.clearTimeout(timer); + }; }, [loadSnapshot, snapshot?.discoveryPending]); const commandCounts = useMemo(() => { @@ -474,6 +526,10 @@ const ExternalSourcesConfig: React.FC = () => { const hostReadOnly = !hostCapabilities.canMutatePolicy && !hostCapabilities.canManageSources && !hostCapabilities.canApproveRuntime; + const remoteWorkspace = workspace?.workspaceKind === WorkspaceKind.Remote; + const readOnlyHintKey = remoteWorkspace + ? 'policy.remoteReadOnlyHint' + : 'policy.hostReadOnlyHint'; const runMutation = useCallback(async ( mutationKey: string, @@ -494,16 +550,16 @@ const ExternalSourcesConfig: React.FC = () => { if (!current || currentCapabilities[requiredCapability] !== true || (currentStatus !== 'compatible' && !(allowIncompatiblePolicy && currentStatus === 'incompatible_schema'))) { - setOperationStatus(t('policy.hostReadOnlyHint')); + setOperationStatus(t(readOnlyHintKey)); return false; } - if (mutationInFlight.current) { + if (activeMutation.current?.scope === requestScope) { setOperationStatus(t('actions.waitForUpdate')); return false; } - mutationInFlight.current = true; const scope = requestScope; const sequence = ++requestSequence.current; + activeMutation.current = { scope, sequence }; pendingMutations.current.set(sequence, scope); latestMutationByScope.current.set(scope, sequence); setBusyKey(mutationKey); @@ -524,13 +580,16 @@ const ExternalSourcesConfig: React.FC = () => { } return false; } finally { - mutationInFlight.current = false; + if (activeMutation.current?.scope === scope + && activeMutation.current.sequence === sequence) { + activeMutation.current = null; + } pendingMutations.current.delete(sequence); if (requestScopeRef.current === scope) { setBusyKey((current) => (current === mutationKey ? null : current)); } } - }, [acceptMutationSnapshot, requestScope, t]); + }, [acceptMutationSnapshot, readOnlyHintKey, requestScope, t]); const setEnabled = useCallback(async (sourceKey: string, enabled: boolean) => { if (!snapshot) return; @@ -779,10 +838,6 @@ const ExternalSourcesConfig: React.FC = () => { } return accessRank[requested] <= accessRank[ceiling] ? requested : ceiling; }; - const externalAssetCount = (snapshot?.commands.length ?? 0) - + (snapshot?.tools?.length ?? 0) - + (snapshot?.subagents?.length ?? 0) - + (snapshot?.mcpServers?.length ?? 0); const diagnosticAttentionCount = (snapshot?.diagnostics ?? []) .filter((diagnostic) => diagnostic.severity !== 'info').length + (snapshot?.sources ?? []).reduce((count, source) => ( @@ -829,12 +884,16 @@ const ExternalSourcesConfig: React.FC = () => { access, }), [updatePolicy]); - const resetIncompatiblePolicy = useCallback(() => { - if (!snapshot) return Promise.resolve(false); + const resetIncompatiblePolicy = useCallback((confirmation: { + requestScope: string; + workspacePath?: string; + preferenceRevision: number; + }) => { + if (requestScope !== confirmation.requestScope) return Promise.resolve(false); return runMutation( 'integration-policy:recovery', - () => externalSourcesAPI.updateIntegrationPolicy(workspacePath, { - expectedPreferenceRevision: snapshot.preferenceRevision ?? 0, + () => externalSourcesAPI.updateIntegrationPolicy(confirmation.workspacePath, { + expectedPreferenceRevision: confirmation.preferenceRevision, scope: 'user', change: { operation: 'reset_incompatible_policy' }, }), @@ -844,7 +903,7 @@ const ExternalSourcesConfig: React.FC = () => { 'canMutatePolicy', true, ); - }, [runMutation, snapshot, t, workspacePath]); + }, [requestScope, runMutation, t]); const scrollToFirstAttentionItem = useCallback(() => { const target = document.querySelector( @@ -878,22 +937,31 @@ const ExternalSourcesConfig: React.FC = () => { title={t('title')} subtitle={t('subtitle')} extra={( - + + )} /> {hostUnavailable ? ( {null} @@ -917,44 +985,27 @@ const ExternalSourcesConfig: React.FC = () => { {snapshot && hostReadOnly ? (
) : null} {snapshot && policy ? ( -
-
-
- -
-
- {t('policy.title')} - - {t('policy.externalBadge')} - -
-
- {externalAttentionCount > 0 ? ( - - ) : t('policy.readySummary', { count: externalAssetCount })} -
-
-
+ title={t('policy.title')} + description={externalAttentionCount > 0 ? ( + + + + ) : undefined} + extra={( { enabled: event.currentTarget.checked, })} /> -
- + )} + > {policyIncompatible ? (
{ variant="secondary" size="small" disabled={busyKey !== null || !hostCapabilities.canMutatePolicy} - onClick={() => setResetPolicyConfirmOpen(true)} + onClick={() => setResetPolicyConfirmation({ + requestScope, + workspacePath, + preferenceRevision: snapshot.preferenceRevision ?? 0, + })} > {t('policy.backupAndReset')} @@ -1060,9 +1115,6 @@ const ExternalSourcesConfig: React.FC = () => {
-
{ecosystem.descriptor.displayName} @@ -1074,13 +1126,6 @@ const ExternalSourcesConfig: React.FC = () => { {t(`policy.state.${ecosystem.state}`)}
-
- {t('policy.adapterRevision', { - revision: ecosystem.descriptor.adapterRevision, - })} - {' · '} - {t('policy.sourceCount', { count: ecosystem.sourceLocations.length })} -
@@ -1204,7 +1249,7 @@ const ExternalSourcesConfig: React.FC = () => { ) : null} ))} -
+ ) : null} {operationStatus ? (
{ ) : null} - {!workspacePath ? ( -
- {t('sources.globalOnly')} -
- ) : null} - {snapshot?.discoveryPending ? (
{t('checkingNonBlocking')} @@ -1262,7 +1301,6 @@ const ExternalSourcesConfig: React.FC = () => { {(snapshot?.mcpApprovalRequests?.length ?? 0) > 0 ? ( {snapshot?.mcpApprovalRequests?.map((request) => { const source = snapshot.sources.find((candidate) => ( @@ -1368,7 +1406,7 @@ const ExternalSourcesConfig: React.FC = () => { ) : null} {(snapshot?.mcpServers?.length ?? 0) > 0 ? ( - + {snapshot?.mcpServers?.map((server) => { const state = server.activationState.state; const reviewing = reviewingMcpKey === server.candidateId; @@ -1493,7 +1531,6 @@ const ExternalSourcesConfig: React.FC = () => { {mcpConflicts.length > 0 ? ( {mcpConflicts.map((conflict) => (
{ ) : null} {(snapshot?.subagents?.length ?? 0) > 0 ? ( - + {snapshot?.subagents?.map((agent) => { const reviewing = reviewingAgentKey === agent.candidateId; const state = agent.activationState.state; @@ -1782,7 +1819,6 @@ const ExternalSourcesConfig: React.FC = () => { {agentConflicts.length > 0 ? ( {agentConflicts.map((conflict) => { const selectedExternalAgent = snapshot?.subagents?.find((agent) => ( @@ -1906,7 +1942,6 @@ const ExternalSourcesConfig: React.FC = () => { {(snapshot?.toolApprovalRequests?.length ?? 0) > 0 ? ( {snapshot?.toolApprovalRequests?.map((request) => { const targetTools = (snapshot.tools ?? []).filter((tool) => ( @@ -2009,9 +2044,8 @@ const ExternalSourcesConfig: React.FC = () => { - {!snapshot?.discoveryPending && (snapshot?.sources.length ?? 0) === 0 ? ( + {snapshot && !snapshot.discoveryPending && snapshot.sources.length === 0 ? (
{t('sources.empty')}
) : snapshot?.sources.map((source) => { const sourcePair = `${source.record.key.providerId}\u0000${source.record.key.sourceId}`; @@ -2094,7 +2128,7 @@ const ExternalSourcesConfig: React.FC = () => { {(snapshot?.tools?.length ?? 0) > 0 ? ( - + {snapshot?.tools?.map((tool) => { const toolKey = `${tool.definition.id.target.source.providerId}:${tool.definition.id.target.source.sourceId}:${tool.definition.id.target.localId}:${tool.definition.id.exportId}`; const source = snapshot.sources.find((candidate) => matchesToolSource(candidate, tool)); @@ -2259,7 +2293,6 @@ const ExternalSourcesConfig: React.FC = () => { {commandConflicts.length > 0 ? ( {commandConflicts.map((conflict) => { const selectedChoiceUnavailable = conflict.candidates.some((candidate) => ( @@ -2343,7 +2376,6 @@ const ExternalSourcesConfig: React.FC = () => { {toolConflicts.length > 0 ? ( {toolConflicts.map((conflict) => { const selectedCandidate = conflict.candidates.find((candidate) => ( @@ -2426,11 +2458,12 @@ const ExternalSourcesConfig: React.FC = () => { )} setResetPolicyConfirmOpen(false)} + isOpen={resetPolicyConfirmation !== null} + onClose={() => setResetPolicyConfirmation(null)} onConfirm={() => { - setResetPolicyConfirmOpen(false); - void resetIncompatiblePolicy(); + const confirmation = resetPolicyConfirmation; + setResetPolicyConfirmation(null); + if (confirmation) void resetIncompatiblePolicy(confirmation); }} title={t('policy.resetConfirmTitle')} message={t('policy.resetConfirmMessage')} diff --git a/src/web-ui/src/infrastructure/config/components/McpToolsConfig.scss b/src/web-ui/src/infrastructure/config/components/McpToolsConfig.scss index c60b8b028..1920a52ac 100644 --- a/src/web-ui/src/infrastructure/config/components/McpToolsConfig.scss +++ b/src/web-ui/src/infrastructure/config/components/McpToolsConfig.scss @@ -1,9 +1,24 @@ @use '../../../component-library/styles/tokens' as *; .bitfun-mcp-tools { + .bitfun-config-page-section__body { + border: 0; + border-radius: 0; + background: transparent; + } + + .bitfun-config-page-row { + padding: $size-gap-3 0; + } + + .bitfun-collection-empty { + justify-content: flex-start; + padding: $size-gap-2 0; + font-size: var(--font-size-xs); + } // ─── JSON editor (MCP) ───────────────────────────────────────────────────── &__json-editor { - padding: $size-gap-4; + padding: $size-gap-3 0; border-bottom: 1px solid var(--border-subtle); display: flex; flex-direction: column; diff --git a/src/web-ui/src/infrastructure/config/components/McpToolsConfig.test.tsx b/src/web-ui/src/infrastructure/config/components/McpToolsConfig.test.tsx new file mode 100644 index 000000000..3f8bb4e71 --- /dev/null +++ b/src/web-ui/src/infrastructure/config/components/McpToolsConfig.test.tsx @@ -0,0 +1,234 @@ +// @vitest-environment jsdom + +import React, { act } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import McpToolsConfig from './McpToolsConfig'; + +const peerState = vi.hoisted(() => ({ active: true })); +const runtimeState = vi.hoisted(() => ({ desktop: true })); +const getServersMock = vi.hoisted(() => vi.fn()); +const loadJsonConfigMock = vi.hoisted(() => vi.fn()); +const startServerMock = vi.hoisted(() => vi.fn()); +const notificationMocks = vi.hoisted(() => ({ + success: vi.fn(), + warning: vi.fn(), + error: vi.fn(), +})); + +vi.mock('react-i18next', () => ({ + initReactI18next: { type: '3rdParty', init: vi.fn() }, + useTranslation: () => ({ t: (key: string) => key }), +})); +vi.mock('@/infrastructure/peer-device/PeerDeviceContext', () => ({ + usePeerDeviceModeOptional: () => ({ + peerMode: peerState.active + ? { active: true, deviceId: 'remote-device', deviceName: 'Remote device' } + : { active: false }, + }), +})); +vi.mock('@/infrastructure/runtime', () => ({ + isTauriRuntime: () => runtimeState.desktop, +})); +vi.mock('@/shared/notification-system', () => ({ + useNotification: () => notificationMocks, +})); +vi.mock('../../api/service-api/MCPAPI', () => ({ + MCPAPI: { + getServers: getServersMock, + loadMCPJsonConfig: loadJsonConfigMock, + startServer: startServerMock, + }, +})); +vi.mock('../../api/service-api/SystemAPI', () => ({ systemAPI: {} })); +vi.mock('./ExternalMcpOverview', () => ({ + default: () =>
, +})); + +describe('McpToolsConfig remote behavior', () => { + let container: HTMLDivElement; + let root: Root; + + beforeEach(() => { + container = document.createElement('div'); + document.body.appendChild(container); + root = createRoot(container); + peerState.active = true; + runtimeState.desktop = true; + getServersMock.mockReset().mockResolvedValue([]); + loadJsonConfigMock.mockReset().mockResolvedValue('{"mcpServers":{}}'); + startServerMock.mockReset().mockResolvedValue(undefined); + notificationMocks.success.mockReset(); + notificationMocks.warning.mockReset(); + notificationMocks.error.mockReset(); + }); + + afterEach(async () => { + await act(async () => root.unmount()); + container.remove(); + }); + + it('does not call desktop MCP management APIs during a remote connection', async () => { + await act(async () => { + root.render(); + await Promise.resolve(); + await Promise.resolve(); + }); + + expect(getServersMock).not.toHaveBeenCalled(); + expect(loadJsonConfigMock).not.toHaveBeenCalled(); + expect(container.textContent).toContain('section.serverList.remoteUnavailable'); + + peerState.active = false; + runtimeState.desktop = false; + await act(async () => { + root.render(); + await Promise.resolve(); + }); + + expect(getServersMock).not.toHaveBeenCalled(); + expect(loadJsonConfigMock).not.toHaveBeenCalled(); + expect(container.textContent).toContain('section.serverList.desktopUnavailable'); + + runtimeState.desktop = true; + await act(async () => { + root.render(); + await Promise.resolve(); + }); + + expect(getServersMock).toHaveBeenCalledTimes(1); + expect(loadJsonConfigMock).toHaveBeenCalledTimes(1); + }); + + it('ignores a desktop MCP load that finishes after switching to a remote connection', async () => { + let resolveServers: ((servers: Array>) => void) | undefined; + getServersMock.mockReturnValueOnce(new Promise((resolve) => { + resolveServers = resolve; + })); + peerState.active = false; + + await act(async () => { + root.render(); + await Promise.resolve(); + }); + peerState.active = true; + await act(async () => { + root.render(); + await Promise.resolve(); + }); + await act(async () => { + resolveServers?.([{ + id: 'local-test', + name: 'Local test server', + status: 'Stopped', + serverType: 'local', + transport: 'stdio', + enabled: true, + autoStart: false, + startSupported: true, + }]); + await Promise.resolve(); + }); + + expect(container.textContent).not.toContain('Local test server'); + expect(container.textContent).toContain('section.serverList.remoteUnavailable'); + }); + + it('shows a retryable failure instead of an empty native MCP list', async () => { + getServersMock.mockRejectedValueOnce(new Error('load failed')); + peerState.active = false; + + await act(async () => { + root.render(); + await Promise.resolve(); + await Promise.resolve(); + }); + + expect(container.textContent).toContain('section.serverList.loadFailed'); + const retry = container.querySelector('[aria-label="actions.refresh"]') as HTMLButtonElement; + expect(retry).not.toBeNull(); + await act(async () => { + retry.click(); + await Promise.resolve(); + await Promise.resolve(); + }); + expect(getServersMock).toHaveBeenCalledTimes(2); + expect(container.textContent).not.toContain('section.serverList.loadFailed'); + }); + + it('does not replace an unreadable MCP config with example JSON', async () => { + loadJsonConfigMock.mockRejectedValueOnce(new Error('config unavailable')); + peerState.active = false; + + await act(async () => { + root.render(); + await Promise.resolve(); + await Promise.resolve(); + }); + const openEditor = container.querySelector( + '[aria-label="actions.jsonConfig"]', + ) as HTMLButtonElement; + await act(async () => openEditor.click()); + + expect(container.textContent).toContain('jsonEditor.loadFailed'); + expect(container.querySelector('.bitfun-mcp-tools__json-textarea')).toBeNull(); + expect(container.textContent).not.toContain('example-server'); + + const retry = container.querySelector('[aria-label="actions.refresh"]') as HTMLButtonElement; + await act(async () => { + retry.click(); + await Promise.resolve(); + await Promise.resolve(); + }); + expect(loadJsonConfigMock).toHaveBeenCalledTimes(2); + expect(container.textContent).not.toContain('jsonEditor.loadFailed'); + expect(container.querySelector('.bitfun-mcp-tools__json-textarea')).not.toBeNull(); + }); + + it('does not notify or reload after a pending start loses desktop capability', async () => { + let resolveStart: (() => void) | undefined; + startServerMock.mockReturnValueOnce(new Promise((resolve) => { + resolveStart = resolve; + })); + getServersMock.mockResolvedValueOnce([{ + id: 'local-test', + name: 'Local test server', + status: 'Stopped', + serverType: 'local', + transport: 'stdio', + enabled: true, + autoStart: false, + commandAvailable: true, + startSupported: true, + }]); + peerState.active = false; + + await act(async () => { + root.render(); + await Promise.resolve(); + await Promise.resolve(); + }); + const startButton = Array.from(container.querySelectorAll('button')).find((button) => ( + button.getAttribute('aria-label') === 'actions.start' + || button.getAttribute('title') === 'actions.start' + )); + expect(startButton).toBeDefined(); + await act(async () => { + startButton?.click(); + await Promise.resolve(); + }); + peerState.active = true; + await act(async () => { + root.render(); + await Promise.resolve(); + }); + await act(async () => { + resolveStart?.(); + await Promise.resolve(); + }); + + expect(notificationMocks.success).not.toHaveBeenCalled(); + expect(notificationMocks.error).not.toHaveBeenCalled(); + expect(getServersMock).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/web-ui/src/infrastructure/config/components/McpToolsConfig.tsx b/src/web-ui/src/infrastructure/config/components/McpToolsConfig.tsx index a7035eeb3..efffd9046 100644 --- a/src/web-ui/src/infrastructure/config/components/McpToolsConfig.tsx +++ b/src/web-ui/src/infrastructure/config/components/McpToolsConfig.tsx @@ -4,7 +4,7 @@ * Uses settings/mcp-tools for page title/subtitle, settings/mcp for the MCP section. */ -import React, { useRef, useState, useEffect } from 'react'; +import React, { useCallback, useEffect, useLayoutEffect, useRef, useState } from 'react'; import { useTranslation } from 'react-i18next'; import { FileJson, @@ -28,6 +28,8 @@ import { } from './common'; import { useNotification } from '@/shared/notification-system'; import { createLogger } from '@/shared/utils/logger'; +import { usePeerDeviceModeOptional } from '@/infrastructure/peer-device/PeerDeviceContext'; +import { isTauriRuntime } from '@/infrastructure/runtime'; import { MCPAPI, MCPRemoteOAuthSessionSnapshot, @@ -164,16 +166,25 @@ const McpToolsConfig: React.FC = () => { const { t: tMcp } = useTranslation('settings/mcp'); const notification = useNotification(); + const peerDevice = usePeerDeviceModeOptional(); + const remoteConnectionActive = peerDevice?.peerMode.active === true; + const desktopConfigAvailable = isTauriRuntime() && !remoteConnectionActive; const classifyError = createErrorClassifier(tMcp); // ─── MCP state ───────────────────────────────────────────────────────────── const jsonEditorRef = useRef(null); const jsonLintSeqRef = useRef(0); const oauthPollTimerRef = useRef(null); + const serverLoadRequestIdRef = useRef(0); + const jsonLoadRequestIdRef = useRef(0); + const capabilityRef = useRef({ available: desktopConfigAvailable, epoch: 0 }); const [servers, setServers] = useState([]); const [mcpLoading, setMcpLoading] = useState(true); + const [serverLoadFailed, setServerLoadFailed] = useState(false); const [showJsonEditor, setShowJsonEditor] = useState(false); const [jsonConfig, setJsonConfig] = useState(''); + const [jsonLoading, setJsonLoading] = useState(true); + const [jsonLoadFailed, setJsonLoadFailed] = useState(false); const [authDialogServer, setAuthDialogServer] = useState(null); const [authValue, setAuthValue] = useState(''); const [authSubmitting, setAuthSubmitting] = useState(false); @@ -187,6 +198,22 @@ const McpToolsConfig: React.FC = () => { position?: number; } | null>(null); + useLayoutEffect(() => { + if (capabilityRef.current.available !== desktopConfigAvailable) { + capabilityRef.current = { + available: desktopConfigAvailable, + epoch: capabilityRef.current.epoch + 1, + }; + } + }, [desktopConfigAvailable]); + + const currentCapabilityEpoch = useCallback((): number | null => ( + capabilityRef.current.available ? capabilityRef.current.epoch : null + ), []); + const capabilityIsCurrent = useCallback((epoch: number): boolean => ( + capabilityRef.current.available && capabilityRef.current.epoch === epoch + ), []); + const tryFormatJson = (input: string): string | null => { try { return JSON.stringify(JSON.parse(input), null, 2); @@ -198,37 +225,84 @@ const McpToolsConfig: React.FC = () => { // ─── MCP effects & handlers ───────────────────────────────────────────────── const LOAD_SERVERS_TIMEOUT_MS = 15_000; - const loadServers = async () => { + const loadServers = useCallback(async (): Promise => { + const capabilityEpoch = currentCapabilityEpoch(); + if (capabilityEpoch === null) return false; + const requestId = ++serverLoadRequestIdRef.current; try { setMcpLoading(true); + setServerLoadFailed(false); const serverList = await Promise.race([ MCPAPI.getServers(), new Promise((_, reject) => setTimeout(() => reject(new Error('MCP servers load timed out')), LOAD_SERVERS_TIMEOUT_MS) ), ]); + if ( + requestId !== serverLoadRequestIdRef.current + || !capabilityIsCurrent(capabilityEpoch) + ) { + return false; + } setServers(serverList); + setServerLoadFailed(false); + return true; } catch (error) { + if ( + requestId !== serverLoadRequestIdRef.current + || !capabilityIsCurrent(capabilityEpoch) + ) { + return false; + } log.error('Failed to load MCP servers', error); + setServerLoadFailed(true); + return false; } finally { - setMcpLoading(false); + if ( + requestId === serverLoadRequestIdRef.current + && capabilityIsCurrent(capabilityEpoch) + ) { + setMcpLoading(false); + } } - }; - - const loadJsonConfig = async () => { + }, [capabilityIsCurrent, currentCapabilityEpoch]); + + const loadJsonConfig = useCallback(async (): Promise => { + const capabilityEpoch = currentCapabilityEpoch(); + if (capabilityEpoch === null) return false; + const requestId = ++jsonLoadRequestIdRef.current; + setJsonLoading(true); + setJsonLoadFailed(false); try { const config = await MCPAPI.loadMCPJsonConfig(); + if ( + requestId !== jsonLoadRequestIdRef.current + || !capabilityIsCurrent(capabilityEpoch) + ) { + return false; + } setJsonConfig(config); - } catch { - setJsonConfig( - JSON.stringify( - { mcpServers: { 'example-server': { command: 'npx', args: ['-y', '@example/mcp-server'], env: {} } } }, - null, - 2 - ) - ); + setJsonLoadFailed(false); + return true; + } catch (error) { + if ( + requestId !== jsonLoadRequestIdRef.current + || !capabilityIsCurrent(capabilityEpoch) + ) { + return false; + } + log.error('Failed to load MCP JSON configuration', error); + setJsonLoadFailed(true); + return false; + } finally { + if ( + requestId === jsonLoadRequestIdRef.current + && capabilityIsCurrent(capabilityEpoch) + ) { + setJsonLoading(false); + } } - }; + }, [capabilityIsCurrent, currentCapabilityEpoch]); function stopOAuthPolling() { if (oauthPollTimerRef.current !== null) { @@ -239,8 +313,10 @@ const McpToolsConfig: React.FC = () => { const handleOAuthSessionUpdate = async ( serverId: string, - session: MCPRemoteOAuthSessionSnapshot | null + session: MCPRemoteOAuthSessionSnapshot | null, + capabilityEpoch: number, ) => { + if (!capabilityIsCurrent(capabilityEpoch)) return; setOauthSession(session); const status = session?.status; @@ -259,6 +335,7 @@ const McpToolsConfig: React.FC = () => { } ); await loadServers(); + if (!capabilityIsCurrent(capabilityEpoch)) return; closeAuthDialog(); return; } @@ -274,13 +351,20 @@ const McpToolsConfig: React.FC = () => { } }; - const pollOAuthSession = (serverId: string) => { + const pollOAuthSession = (serverId: string, capabilityEpoch: number) => { + if (!capabilityIsCurrent(capabilityEpoch)) return; stopOAuthPolling(); oauthPollTimerRef.current = window.setInterval(async () => { + if (!capabilityIsCurrent(capabilityEpoch)) { + stopOAuthPolling(); + return; + } try { const session = await MCPAPI.getRemoteOAuthSession({ serverId }); - await handleOAuthSessionUpdate(serverId, session); + if (!capabilityIsCurrent(capabilityEpoch)) return; + await handleOAuthSessionUpdate(serverId, session, capabilityEpoch); } catch (error) { + if (!capabilityIsCurrent(capabilityEpoch)) return; stopOAuthPolling(); notification.error( error instanceof Error ? error.message : String(error), @@ -294,9 +378,26 @@ const McpToolsConfig: React.FC = () => { }; useEffect(() => { - loadServers(); - loadJsonConfig(); - }, []); + serverLoadRequestIdRef.current += 1; + jsonLoadRequestIdRef.current += 1; + if (!desktopConfigAvailable) { + setServers([]); + setMcpLoading(false); + setServerLoadFailed(false); + setShowJsonEditor(false); + setJsonLoading(false); + setJsonLoadFailed(false); + setAuthDialogServer(null); + setAuthSubmitting(false); + setOauthSession(null); + setOauthStarting(false); + setOauthCancelling(false); + stopOAuthPolling(); + return; + } + void loadServers(); + void loadJsonConfig(); + }, [desktopConfigAvailable, loadJsonConfig, loadServers]); useEffect(() => { return () => { @@ -349,6 +450,8 @@ const McpToolsConfig: React.FC = () => { }, [jsonConfig, showJsonEditor]); const handleSaveJsonConfig = async () => { + const capabilityEpoch = currentCapabilityEpoch(); + if (capabilityEpoch === null) return; try { let parsedConfig; try { @@ -365,6 +468,7 @@ const McpToolsConfig: React.FC = () => { throw new Error(tMcp('errors.mcpServersMustBeObject')); await MCPAPI.saveMCPJsonConfig(jsonConfig); + if (!capabilityIsCurrent(capabilityEpoch)) return; notification.success(tMcp('messages.saveSuccess'), { title: tMcp('notifications.saveSuccess'), duration: 3000, @@ -374,18 +478,26 @@ const McpToolsConfig: React.FC = () => { void (async () => { try { await loadServers(); + if (!capabilityIsCurrent(capabilityEpoch)) return; await MCPAPI.initializeServers(); + if (!capabilityIsCurrent(capabilityEpoch)) return; } catch { + if (!capabilityIsCurrent(capabilityEpoch)) return; notification.warning(tMcp('messages.partialStartFailed'), { title: tMcp('notifications.partialStartFailed'), duration: 5000, }); } finally { - await loadServers(); - await loadJsonConfig(); + if (capabilityIsCurrent(capabilityEpoch)) { + await loadServers(); + if (capabilityIsCurrent(capabilityEpoch)) { + await loadJsonConfig(); + } + } } })(); } catch (error) { + if (!capabilityIsCurrent(capabilityEpoch)) return; const errorInfo = classifyError(error, tMcp('actions.saveConfig')); let fullMessage = errorInfo.message; if (errorInfo.suggestions?.length) { @@ -543,6 +655,8 @@ const McpToolsConfig: React.FC = () => { }; const handleStartServer = async (server: MCPServerInfo) => { + const capabilityEpoch = currentCapabilityEpoch(); + if (capabilityEpoch === null) return; if (!canStartServer(server)) { notifyServerStartUnavailable(server); return; @@ -551,12 +665,14 @@ const McpToolsConfig: React.FC = () => { const serverId = server.id; try { await MCPAPI.startServer(serverId); + if (!capabilityIsCurrent(capabilityEpoch)) return; notification.success(tMcp('messages.startSuccess', { serverId }), { title: tMcp('notifications.startSuccess'), duration: 3000, }); await loadServers(); } catch (error) { + if (!capabilityIsCurrent(capabilityEpoch)) return; if (isRemoteServer(server) && isLikelyRemoteAuthError(error)) { handleOpenAuthDialog(server); if (server.oauthEnabled) { @@ -573,14 +689,18 @@ const McpToolsConfig: React.FC = () => { }; const handleStopServer = async (serverId: string) => { + const capabilityEpoch = currentCapabilityEpoch(); + if (capabilityEpoch === null) return; try { await MCPAPI.stopServer(serverId); + if (!capabilityIsCurrent(capabilityEpoch)) return; notification.success(tMcp('messages.stopSuccess', { serverId }), { title: tMcp('notifications.stopSuccess'), duration: 3000, }); await loadServers(); } catch (error) { + if (!capabilityIsCurrent(capabilityEpoch)) return; notification.error( tMcp('messages.stopFailed', { serverId }) + ': ' + @@ -591,6 +711,8 @@ const McpToolsConfig: React.FC = () => { }; const handleRestartServer = async (server: MCPServerInfo) => { + const capabilityEpoch = currentCapabilityEpoch(); + if (capabilityEpoch === null) return; if (!canStartServer(server)) { notifyServerStartUnavailable(server); return; @@ -599,12 +721,14 @@ const McpToolsConfig: React.FC = () => { const serverId = server.id; try { await MCPAPI.restartServer(serverId); + if (!capabilityIsCurrent(capabilityEpoch)) return; notification.success(tMcp('messages.restartSuccess', { serverId }), { title: tMcp('notifications.restartSuccess'), duration: 3000, }); await loadServers(); } catch (error) { + if (!capabilityIsCurrent(capabilityEpoch)) return; if (isRemoteServer(server) && isLikelyRemoteAuthError(error)) { handleOpenAuthDialog(server); if (server.oauthEnabled) { @@ -621,6 +745,8 @@ const McpToolsConfig: React.FC = () => { }; function handleOpenAuthDialog(server: MCPServerInfo) { + const capabilityEpoch = currentCapabilityEpoch(); + if (capabilityEpoch === null) return; setAuthDialogServer(server); setAuthValue(''); setOauthSession(null); @@ -632,11 +758,13 @@ const McpToolsConfig: React.FC = () => { void (async () => { try { const session = await MCPAPI.getRemoteOAuthSession({ serverId: server.id }); + if (!capabilityIsCurrent(capabilityEpoch)) return; setOauthSession(session); if (session && !['authorized', 'failed', 'cancelled'].includes(session.status)) { - pollOAuthSession(server.id); + pollOAuthSession(server.id, capabilityEpoch); } } catch (error) { + if (!capabilityIsCurrent(capabilityEpoch)) return; log.warn('Failed to load remote OAuth session', { serverId: server.id, error }); } })(); @@ -654,6 +782,11 @@ const McpToolsConfig: React.FC = () => { const handleCloseAuthDialog = () => { if (authSubmitting || oauthCancelling) return; + const capabilityEpoch = currentCapabilityEpoch(); + if (capabilityEpoch === null) { + closeAuthDialog(); + return; + } if ( authDialogServer && @@ -665,13 +798,16 @@ const McpToolsConfig: React.FC = () => { try { await MCPAPI.cancelRemoteOAuth({ serverId: authDialogServer.id }); } catch (error) { + if (!capabilityIsCurrent(capabilityEpoch)) return; log.warn('Failed to cancel remote OAuth session', { serverId: authDialogServer.id, error, }); } finally { - setOauthCancelling(false); - closeAuthDialog(); + if (capabilityIsCurrent(capabilityEpoch)) { + setOauthCancelling(false); + closeAuthDialog(); + } } })(); return; @@ -682,6 +818,8 @@ const McpToolsConfig: React.FC = () => { const handleSaveRemoteAuth = async () => { if (!authDialogServer || authSubmitting) return; + const capabilityEpoch = currentCapabilityEpoch(); + if (capabilityEpoch === null) return; const trimmed = authValue.trim(); if (!trimmed) { @@ -698,6 +836,7 @@ const McpToolsConfig: React.FC = () => { serverId: authDialogServer.id, authorizationValue: trimmed, }); + if (!capabilityIsCurrent(capabilityEpoch)) return; notification.success( tMcp('messages.remoteAuthUpdated', { serverId: authDialogServer.id }), { @@ -708,22 +847,28 @@ const McpToolsConfig: React.FC = () => { closeAuthDialog(); await loadServers(); } catch (error) { + if (!capabilityIsCurrent(capabilityEpoch)) return; const errorInfo = classifyError(error, tMcp('actions.saveConfig')); notification.error(errorInfo.message, { title: errorInfo.title, duration: errorInfo.duration, }); } finally { - setAuthSubmitting(false); + if (capabilityIsCurrent(capabilityEpoch)) { + setAuthSubmitting(false); + } } }; const handleDeleteServer = async (server: MCPServerInfo) => { + const capabilityEpoch = currentCapabilityEpoch(); + if (capabilityEpoch === null) return; const confirmed = await window.confirm(tMcp('messages.deleteConfirm')); - if (!confirmed) return; + if (!confirmed || !capabilityIsCurrent(capabilityEpoch)) return; try { await MCPAPI.deleteServer({ serverId: server.id }); + if (!capabilityIsCurrent(capabilityEpoch)) return; if (authDialogServer?.id === server.id) { closeAuthDialog(); } @@ -733,6 +878,7 @@ const McpToolsConfig: React.FC = () => { }); await loadServers(); } catch (error) { + if (!capabilityIsCurrent(capabilityEpoch)) return; const errorInfo = classifyError(error, tMcp('actions.delete')); notification.error( tMcp('errors.deleteServerFailed', { @@ -748,14 +894,18 @@ const McpToolsConfig: React.FC = () => { }; async function startRemoteOAuthFlow(server: MCPServerInfo) { + const capabilityEpoch = currentCapabilityEpoch(); + if (capabilityEpoch === null) return; setOauthStarting(true); try { const session = await MCPAPI.startRemoteOAuth({ serverId: server.id }); + if (!capabilityIsCurrent(capabilityEpoch)) return; setOauthSession(session); if (session.authorizationUrl) { await systemAPI.openExternal(session.authorizationUrl); + if (!capabilityIsCurrent(capabilityEpoch)) return; } - pollOAuthSession(server.id); + pollOAuthSession(server.id, capabilityEpoch); notification.success( session.message || tMcp('messages.remoteOAuthStarted', { serverId: server.id }), { @@ -764,13 +914,16 @@ const McpToolsConfig: React.FC = () => { } ); } catch (error) { + if (!capabilityIsCurrent(capabilityEpoch)) return; const errorInfo = classifyError(error, tMcp('actions.remoteAuth')); notification.error(errorInfo.message, { title: errorInfo.title, duration: errorInfo.duration, }); } finally { - setOauthStarting(false); + if (capabilityIsCurrent(capabilityEpoch)) { + setOauthStarting(false); + } } } @@ -921,14 +1074,35 @@ const McpToolsConfig: React.FC = () => { }; const mcpSectionExtra = ( - setShowJsonEditor(!showJsonEditor)} - tooltip={showJsonEditor ? tMcp('actions.backToList') : tMcp('actions.jsonConfig')} - > - {showJsonEditor ? : } - + <> + {serverLoadFailed && !showJsonEditor ? ( + <> + {servers.length > 0 ? ( + + {tMcp('external.status.stale')} + + ) : null} + void loadServers()} + tooltip={tMcp('actions.refresh')} + aria-label={tMcp('actions.refresh')} + > + + + ) : null} + setShowJsonEditor(!showJsonEditor)} + tooltip={showJsonEditor ? tMcp('actions.backToList') : tMcp('actions.jsonConfig')} + aria-label={showJsonEditor ? tMcp('actions.backToList') : tMcp('actions.jsonConfig')} + > + {showJsonEditor ? : } + + ); const renderServerBadge = (server: MCPServerInfo) => ( @@ -946,6 +1120,7 @@ const McpToolsConfig: React.FC = () => { variant="ghost" onClick={() => handleOpenAuthDialog(server)} tooltip={tMcp('actions.remoteAuth')} + aria-label={tMcp('actions.remoteAuth')} > @@ -955,6 +1130,7 @@ const McpToolsConfig: React.FC = () => { variant="ghost" onClick={() => handleDeleteServer(server)} tooltip={tMcp('actions.delete')} + aria-label={tMcp('actions.delete')} > @@ -968,6 +1144,11 @@ const McpToolsConfig: React.FC = () => { ? tMcp('actions.start') : tMcp('messages.commandUnavailable', { serverId: server.id }) } + aria-label={ + canStartServer(server) + ? tMcp('actions.start') + : tMcp('messages.commandUnavailable', { serverId: server.id }) + } > @@ -977,6 +1158,7 @@ const McpToolsConfig: React.FC = () => { variant="warning" onClick={() => handleStopServer(server.id)} tooltip={tMcp('actions.stop')} + aria-label={tMcp('actions.stop')} > @@ -990,6 +1172,11 @@ const McpToolsConfig: React.FC = () => { ? tMcp('actions.restart') : tMcp('messages.commandUnavailable', { serverId: server.id }) } + aria-label={ + canStartServer(server) + ? tMcp('actions.restart') + : tMcp('messages.commandUnavailable', { serverId: server.id }) + } > @@ -1093,18 +1280,46 @@ const McpToolsConfig: React.FC = () => { return ( - + - - - {/* MCP section */} - {showJsonEditor && ( + {!desktopConfigAvailable && ( +
+

{tMcp(remoteConnectionActive + ? 'section.serverList.remoteUnavailable' + : 'section.serverList.desktopUnavailable')}

+
+ )} + + {desktopConfigAvailable && showJsonEditor && jsonLoading && ( +
+

{tMcp('loading')}

+
+ )} + + {desktopConfigAvailable && showJsonEditor && !jsonLoading && jsonLoadFailed && ( +
+

{tMcp('jsonEditor.loadFailed')}

+ void loadJsonConfig()} + tooltip={tMcp('actions.refresh')} + aria-label={tMcp('actions.refresh')} + > + +
+ )} + + {desktopConfigAvailable && showJsonEditor && !jsonLoading && !jsonLoadFailed && (

{tMcp('jsonEditor.title')}

@@ -1160,13 +1375,21 @@ const McpToolsConfig: React.FC = () => {
)} - {!showJsonEditor && mcpLoading && ( + {desktopConfigAvailable && !showJsonEditor && mcpLoading && (

{tMcp('loading')}

)} - {!showJsonEditor && !mcpLoading && servers.length === 0 && ( + {desktopConfigAvailable && !showJsonEditor && !mcpLoading + && serverLoadFailed && servers.length === 0 && ( +
+

{tMcp('section.serverList.loadFailed')}

+
+ )} + + {desktopConfigAvailable && !showJsonEditor && !mcpLoading + && !serverLoadFailed && servers.length === 0 && (
)} - {!showJsonEditor && + {desktopConfigAvailable && !showJsonEditor && servers.map((server) => ( { /> ))} + +