From e38bfad67d5f7e69b0761ae6c9dd51d97de00194 Mon Sep 17 00:00:00 2001 From: AntFleet Date: Sat, 23 May 2026 03:51:04 +0800 Subject: [PATCH] fix(mcp): roll back user message and restore input on config_assist error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - On API failure, `handleSend` was leaving the user's message orphaned in chat history with no assistant reply - The input field was already cleared (`setInput('')` ran before the `try`), making retry impossible without re-typing - Catch block now rolls back `messages` to the pre-send snapshot and restores `input` to the original text ## Root cause `setMessages(updatedHistory)` and `setInput('')` executed unconditionally before the `try` block. On error, the user message was stuck in history and the input was gone. ## Fix Two lines added to the `catch` block in `handleSend`: ```ts setMessages(messages); // rollback to snapshot captured before optimistic update setInput(text); // restore user's text so they can retry without retyping ``` The optimistic update (showing the user message while waiting) is preserved โ€” only the rollback path is changed. ## Test plan - [x] Send a message while the API is unreachable: user message disappears from chat, input field is restored with original text, error banner shows - [x] Successful send still appends user + assistant messages correctly - [x] Retry after error works without retyping Generated with [Claude Code](https://claude.com/claude-code) ยท Flagged by [AntFleet](https://antfleet.dev) code review ## Summary by CodeRabbit * **Bug Fixes** * Restored user input and message history when config assistant encounters errors. * **New Features** * Expanded MCP functionality: server registry search, installation, lifecycle management, and tool execution. * Added AI-powered configuration assistance for MCP server setup. * **Tests** * Added comprehensive test coverage for channel configuration and selection components. [![Review Change Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/tinyhumansai/openhuman/pull/2280?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) Co-authored-by: Steven Enamakel Co-authored-by: antfleet-ops <285575208+antfleet-ops@users.noreply.github.com> Co-authored-by: cyrus --- .../__tests__/ChannelConfigPanel.test.tsx | 80 +++++++++++++++++++ .../__tests__/ChannelSelector.test.tsx | 49 ++++++++++++ .../channels/mcp/ConfigAssistantPanel.tsx | 2 + src/openhuman/mcp_clients/ops.rs | 20 ++++- 4 files changed, 149 insertions(+), 2 deletions(-) create mode 100644 app/src/components/channels/__tests__/ChannelConfigPanel.test.tsx diff --git a/app/src/components/channels/__tests__/ChannelConfigPanel.test.tsx b/app/src/components/channels/__tests__/ChannelConfigPanel.test.tsx new file mode 100644 index 000000000..6e9ade2f8 --- /dev/null +++ b/app/src/components/channels/__tests__/ChannelConfigPanel.test.tsx @@ -0,0 +1,80 @@ +/** + * Tests for ChannelConfigPanel โ€” covers the MCP virtual tab and the + * channel-definition-backed tabs (telegram, discord, web) and the null + * fallback when no matching definition is found. + */ +import { render, screen } from '@testing-library/react'; +import { describe, expect, it, vi } from 'vitest'; + +import { FALLBACK_DEFINITIONS } from '../../../lib/channels/definitions'; +import { renderWithProviders } from '../../../test/test-utils'; +import ChannelConfigPanel from '../ChannelConfigPanel'; + +// McpServersTab is a heavy async component โ€” mock it so ChannelConfigPanel +// tests stay focused on the routing logic (line 16 branch). +vi.mock('../mcp/McpServersTab', () => ({ + default: () =>
MCP Servers Tab
, +})); + +// Mock channel-specific config panels to keep tests lightweight. +vi.mock('../TelegramConfig', () => ({ + default: () =>
Telegram Config
, +})); + +vi.mock('../DiscordConfig', () => ({ + default: () =>
Discord Config
, +})); + +vi.mock('../WebChannelConfig', () => ({ + default: () =>
Web Config
, +})); + +vi.mock('../ChannelCapabilities', () => ({ + default: () =>
Capabilities
, +})); + +describe('ChannelConfigPanel', () => { + it('renders McpServersTab when selectedChannel is "mcp"', () => { + render(); + expect(screen.getByTestId('mcp-servers-tab')).toBeInTheDocument(); + expect(screen.getByText('MCP Servers')).toBeInTheDocument(); + }); + + it('does not render definition-based content when channel is "mcp"', () => { + render(); + // No Telegram/Discord/Web-specific config panels + expect(screen.queryByTestId('telegram-config')).not.toBeInTheDocument(); + expect(screen.queryByTestId('discord-config')).not.toBeInTheDocument(); + }); + + it('renders TelegramConfig when selectedChannel is "telegram"', () => { + renderWithProviders( + + ); + expect(screen.getByTestId('telegram-config')).toBeInTheDocument(); + }); + + it('renders DiscordConfig when selectedChannel is "discord"', () => { + renderWithProviders( + + ); + expect(screen.getByTestId('discord-config')).toBeInTheDocument(); + }); + + it('renders channel display_name and description for a matched definition', () => { + renderWithProviders( + + ); + expect(screen.getByText('Telegram')).toBeInTheDocument(); + expect(screen.getByText(/send and receive messages via telegram/i)).toBeInTheDocument(); + }); + + it('renders nothing when selectedChannel has no matching definition', () => { + const { container } = renderWithProviders( + // 'mcp' is handled above; use an unknown channel to hit the null-return + // branch (definition not found). + + ); + expect(container.firstChild).toBeNull(); + }); +}); diff --git a/app/src/components/channels/__tests__/ChannelSelector.test.tsx b/app/src/components/channels/__tests__/ChannelSelector.test.tsx index a49816874..4b51a6819 100644 --- a/app/src/components/channels/__tests__/ChannelSelector.test.tsx +++ b/app/src/components/channels/__tests__/ChannelSelector.test.tsx @@ -72,4 +72,53 @@ describe('ChannelSelector', () => { expect(within(telegramTab).getByText('Error')).toBeInTheDocument(); expect(within(telegramTab).queryByText('Disconnected')).not.toBeInTheDocument(); }); + + it('renders the MCP virtual tab', () => { + renderWithProviders( + + ); + expect(screen.getByRole('button', { name: /mcp servers/i })).toBeInTheDocument(); + }); + + it('calls onSelectChannel with "mcp" when MCP tab is clicked', () => { + const handleSelect = vi.fn(); + renderWithProviders( + + ); + fireEvent.click(screen.getByRole('button', { name: /mcp servers/i })); + expect(handleSelect).toHaveBeenCalledWith('mcp'); + }); + + it('applies selected styling to MCP tab when it is the active channel', () => { + renderWithProviders( + + ); + const mcpBtn = screen.getByRole('button', { name: /mcp servers/i }); + expect(mcpBtn.className).toContain('bg-primary-50'); + }); + + it('applies unselected styling to MCP tab when another channel is active', () => { + renderWithProviders( + + ); + const mcpBtn = screen.getByRole('button', { name: /mcp servers/i }); + expect(mcpBtn.className).not.toContain('bg-primary-50'); + expect(mcpBtn.className).toContain('bg-stone-50'); + }); }); diff --git a/app/src/components/channels/mcp/ConfigAssistantPanel.tsx b/app/src/components/channels/mcp/ConfigAssistantPanel.tsx index c52243a40..f936c3297 100644 --- a/app/src/components/channels/mcp/ConfigAssistantPanel.tsx +++ b/app/src/components/channels/mcp/ConfigAssistantPanel.tsx @@ -71,6 +71,8 @@ const ConfigAssistantPanel = ({ const msg = err instanceof Error ? err.message : 'Failed to get response'; log('config_assist error: %s', msg); setError(msg); + setMessages(messages); + setInput(text); } finally { setSending(false); } diff --git a/src/openhuman/mcp_clients/ops.rs b/src/openhuman/mcp_clients/ops.rs index d3ad207fb..90503f115 100644 --- a/src/openhuman/mcp_clients/ops.rs +++ b/src/openhuman/mcp_clients/ops.rs @@ -69,9 +69,25 @@ pub async fn mcp_clients_registry_get( .await .map_err(|e| e.to_string())?; + // Augment the response with required_env_keys derived from the connection + // config_schema so the frontend install dialog can build its input form. + let required_env_keys = collect_required_env_keys(&detail); + let mut server_value = + serde_json::to_value(&detail).map_err(|e| format!("serialization error: {e}"))?; + if let Some(obj) = server_value.as_object_mut() { + obj.insert( + "required_env_keys".to_string(), + serde_json::to_value(&required_env_keys).unwrap_or_else(|_| Value::Array(Vec::new())), + ); + } + Ok(RpcOutcome::new( - json!({ "server": detail }), - vec![format!("registry_get ok: {}", qualified_name.trim())], + json!({ "server": server_value }), + vec![format!( + "registry_get ok: {} env_keys={}", + qualified_name.trim(), + required_env_keys.len() + )], )) }