mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-30 23:14:37 +00:00
fix(mcp): roll back user message and restore input on config_assist error
## 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
<!-- This is an auto-generated comment: release notes by coderabbit.ai -->
## 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_stack_entry_start -->
[](https://app.coderabbit.ai/change-stack/tinyhumansai/openhuman/pull/2280?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack)
<!-- review_stack_entry_end -->
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Co-authored-by: Steven Enamakel <enamakel@tinyhumans.ai>
Co-authored-by: antfleet-ops <285575208+antfleet-ops@users.noreply.github.com>
Co-authored-by: cyrus <cyrus@tinyhumans.ai>
This commit is contained in:
co-authored by
Steven Enamakel
antfleet-ops
cyrus
parent
2ba9d06784
commit
e38bfad67d
@@ -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: () => <div data-testid="mcp-servers-tab">MCP Servers Tab</div>,
|
||||
}));
|
||||
|
||||
// Mock channel-specific config panels to keep tests lightweight.
|
||||
vi.mock('../TelegramConfig', () => ({
|
||||
default: () => <div data-testid="telegram-config">Telegram Config</div>,
|
||||
}));
|
||||
|
||||
vi.mock('../DiscordConfig', () => ({
|
||||
default: () => <div data-testid="discord-config">Discord Config</div>,
|
||||
}));
|
||||
|
||||
vi.mock('../WebChannelConfig', () => ({
|
||||
default: () => <div data-testid="web-config">Web Config</div>,
|
||||
}));
|
||||
|
||||
vi.mock('../ChannelCapabilities', () => ({
|
||||
default: () => <div data-testid="channel-capabilities">Capabilities</div>,
|
||||
}));
|
||||
|
||||
describe('ChannelConfigPanel', () => {
|
||||
it('renders McpServersTab when selectedChannel is "mcp"', () => {
|
||||
render(<ChannelConfigPanel selectedChannel="mcp" definitions={FALLBACK_DEFINITIONS} />);
|
||||
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(<ChannelConfigPanel selectedChannel="mcp" definitions={FALLBACK_DEFINITIONS} />);
|
||||
// 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(
|
||||
<ChannelConfigPanel selectedChannel="telegram" definitions={FALLBACK_DEFINITIONS} />
|
||||
);
|
||||
expect(screen.getByTestId('telegram-config')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('renders DiscordConfig when selectedChannel is "discord"', () => {
|
||||
renderWithProviders(
|
||||
<ChannelConfigPanel selectedChannel="discord" definitions={FALLBACK_DEFINITIONS} />
|
||||
);
|
||||
expect(screen.getByTestId('discord-config')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('renders channel display_name and description for a matched definition', () => {
|
||||
renderWithProviders(
|
||||
<ChannelConfigPanel selectedChannel="telegram" definitions={FALLBACK_DEFINITIONS} />
|
||||
);
|
||||
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).
|
||||
<ChannelConfigPanel selectedChannel={'unknown' as never} definitions={[]} />
|
||||
);
|
||||
expect(container.firstChild).toBeNull();
|
||||
});
|
||||
});
|
||||
@@ -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(
|
||||
<ChannelSelector
|
||||
definitions={FALLBACK_DEFINITIONS}
|
||||
selectedChannel="telegram"
|
||||
onSelectChannel={onSelect}
|
||||
/>
|
||||
);
|
||||
expect(screen.getByRole('button', { name: /mcp servers/i })).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('calls onSelectChannel with "mcp" when MCP tab is clicked', () => {
|
||||
const handleSelect = vi.fn();
|
||||
renderWithProviders(
|
||||
<ChannelSelector
|
||||
definitions={FALLBACK_DEFINITIONS}
|
||||
selectedChannel="telegram"
|
||||
onSelectChannel={handleSelect}
|
||||
/>
|
||||
);
|
||||
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(
|
||||
<ChannelSelector
|
||||
definitions={FALLBACK_DEFINITIONS}
|
||||
selectedChannel="mcp"
|
||||
onSelectChannel={onSelect}
|
||||
/>
|
||||
);
|
||||
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(
|
||||
<ChannelSelector
|
||||
definitions={FALLBACK_DEFINITIONS}
|
||||
selectedChannel="telegram"
|
||||
onSelectChannel={onSelect}
|
||||
/>
|
||||
);
|
||||
const mcpBtn = screen.getByRole('button', { name: /mcp servers/i });
|
||||
expect(mcpBtn.className).not.toContain('bg-primary-50');
|
||||
expect(mcpBtn.className).toContain('bg-stone-50');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
@@ -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()
|
||||
)],
|
||||
))
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user