feat(v1.0): remove legacy MCP mode and complete Phase 10 migration
This commit completes the Phase 10 migration to MCP-only architecture by removing all legacy mode code paths and configuration options. **Breaking Changes:** - Removed `McpMode` enum from configuration system - Removed `mode` setting from `[mcp]` config section - MCP architecture is now always enabled (no option to disable) **Code Changes:** - Simplified `McpSettings` struct (now a placeholder for future options) - Updated `McpClientFactory` to remove legacy mode branching - Always use MCP architecture with automatic fallback to local client - Added test infrastructure: `MockProvider` and `MockMcpClient` in test_utils **Documentation:** - Created comprehensive v0.x → v1.0 migration guide - Added CHANGELOG_v1.0.md with detailed technical changes - Documented common issues (cloud model 404s, timeouts, API key setup) - Included rollback procedures and troubleshooting steps **Testing:** - All 29 tests passing - Fixed agent tests to use new mock implementations - Updated factory test to reflect new behavior This completes the 10-phase migration plan documented in .agents/new_phases.md, establishing Owlen as a production-ready MCP-only TUI application. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -364,12 +364,14 @@ impl AgentExecutor {
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::mcp::test_utils::MockMcpClient;
|
||||
use crate::provider::test_utils::MockProvider;
|
||||
|
||||
#[test]
|
||||
fn test_parse_tool_call() {
|
||||
let executor = AgentExecutor {
|
||||
llm_client: Arc::new(crate::provider::MockProvider::new()),
|
||||
tool_client: Arc::new(crate::mcp::MockMcpClient::new()),
|
||||
llm_client: Arc::new(MockProvider::new()),
|
||||
tool_client: Arc::new(MockMcpClient::new()),
|
||||
config: AgentConfig::default(),
|
||||
};
|
||||
|
||||
@@ -397,8 +399,8 @@ ACTION_INPUT: {"query": "Rust programming language"}
|
||||
#[test]
|
||||
fn test_parse_final_answer() {
|
||||
let executor = AgentExecutor {
|
||||
llm_client: Arc::new(crate::provider::MockProvider::new()),
|
||||
tool_client: Arc::new(crate::mcp::MockMcpClient::new()),
|
||||
llm_client: Arc::new(MockProvider::new()),
|
||||
tool_client: Arc::new(MockMcpClient::new()),
|
||||
config: AgentConfig::default(),
|
||||
};
|
||||
|
||||
|
||||
@@ -242,16 +242,8 @@ impl Default for GeneralSettings {
|
||||
/// MCP (Multi-Client-Provider) settings
|
||||
#[derive(Debug, Clone, Serialize, Deserialize, Default)]
|
||||
pub struct McpSettings {
|
||||
#[serde(default)]
|
||||
pub mode: McpMode,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, Default)]
|
||||
#[serde(rename_all = "lowercase")]
|
||||
pub enum McpMode {
|
||||
#[default]
|
||||
Legacy,
|
||||
Enabled,
|
||||
// MCP is now always enabled in v1.0+
|
||||
// Kept as a struct for future configuration options
|
||||
}
|
||||
|
||||
/// Privacy controls governing network access and storage
|
||||
|
||||
@@ -143,3 +143,45 @@ impl McpClient for LocalMcpClient {
|
||||
self.server.call_tool(call).await
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
pub mod test_utils {
|
||||
use super::*;
|
||||
|
||||
/// Mock MCP client for testing
|
||||
pub struct MockMcpClient;
|
||||
|
||||
impl MockMcpClient {
|
||||
pub fn new() -> Self {
|
||||
Self
|
||||
}
|
||||
}
|
||||
|
||||
#[async_trait]
|
||||
impl McpClient for MockMcpClient {
|
||||
async fn list_tools(&self) -> Result<Vec<McpToolDescriptor>> {
|
||||
Ok(vec![McpToolDescriptor {
|
||||
name: "mock_tool".to_string(),
|
||||
description: "A mock tool for testing".to_string(),
|
||||
input_schema: serde_json::json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"query": {"type": "string"}
|
||||
}
|
||||
}),
|
||||
requires_network: false,
|
||||
requires_filesystem: vec![],
|
||||
}])
|
||||
}
|
||||
|
||||
async fn call_tool(&self, call: McpToolCall) -> Result<McpToolResponse> {
|
||||
Ok(McpToolResponse {
|
||||
name: call.name,
|
||||
success: true,
|
||||
output: serde_json::json!({"result": "mock result"}),
|
||||
metadata: HashMap::new(),
|
||||
duration_ms: 10,
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -4,7 +4,7 @@
|
||||
/// Supports switching between local (in-process) and remote (STDIO) execution modes.
|
||||
use super::client::McpClient;
|
||||
use super::{remote_client::RemoteMcpClient, LocalMcpClient};
|
||||
use crate::config::{Config, McpMode};
|
||||
use crate::config::Config;
|
||||
use crate::tools::registry::ToolRegistry;
|
||||
use crate::validation::SchemaValidator;
|
||||
use crate::Result;
|
||||
@@ -31,37 +31,29 @@ impl McpClientFactory {
|
||||
}
|
||||
|
||||
/// Create an MCP client based on the current configuration
|
||||
///
|
||||
/// In v1.0+, MCP architecture is always enabled. If MCP servers are configured,
|
||||
/// uses the first server; otherwise falls back to local in-process client.
|
||||
pub fn create(&self) -> Result<Box<dyn McpClient>> {
|
||||
match self.config.mcp.mode {
|
||||
McpMode::Legacy => {
|
||||
// Use local in-process client
|
||||
Ok(Box::new(LocalMcpClient::new(
|
||||
self.registry.clone(),
|
||||
self.validator.clone(),
|
||||
)))
|
||||
}
|
||||
McpMode::Enabled => {
|
||||
// Use the first configured MCP server, if any.
|
||||
if let Some(server_cfg) = self.config.mcp_servers.first() {
|
||||
match RemoteMcpClient::new_with_config(server_cfg) {
|
||||
Ok(client) => Ok(Box::new(client)),
|
||||
Err(e) => {
|
||||
eprintln!("Warning: Failed to start remote MCP client '{}': {}. Falling back to local mode.", server_cfg.name, e);
|
||||
Ok(Box::new(LocalMcpClient::new(
|
||||
self.registry.clone(),
|
||||
self.validator.clone(),
|
||||
)))
|
||||
}
|
||||
}
|
||||
} else {
|
||||
// No servers configured – fall back to local client.
|
||||
eprintln!("Warning: No MCP servers defined in config. Using local client.");
|
||||
// Use the first configured MCP server, if any.
|
||||
if let Some(server_cfg) = self.config.mcp_servers.first() {
|
||||
match RemoteMcpClient::new_with_config(server_cfg) {
|
||||
Ok(client) => Ok(Box::new(client)),
|
||||
Err(e) => {
|
||||
eprintln!("Warning: Failed to start remote MCP client '{}': {}. Falling back to local mode.", server_cfg.name, e);
|
||||
Ok(Box::new(LocalMcpClient::new(
|
||||
self.registry.clone(),
|
||||
self.validator.clone(),
|
||||
)))
|
||||
}
|
||||
}
|
||||
} else {
|
||||
// No servers configured – fall back to local client.
|
||||
eprintln!("Warning: No MCP servers defined in config. Using local client.");
|
||||
Ok(Box::new(LocalMcpClient::new(
|
||||
self.registry.clone(),
|
||||
self.validator.clone(),
|
||||
)))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -76,9 +68,8 @@ mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn test_factory_creates_local_client_in_legacy_mode() {
|
||||
let mut config = Config::default();
|
||||
config.mcp.mode = McpMode::Legacy;
|
||||
fn test_factory_creates_local_client_when_no_servers_configured() {
|
||||
let config = Config::default();
|
||||
|
||||
let ui = Arc::new(crate::ui::NoOpUiController);
|
||||
let registry = Arc::new(ToolRegistry::new(
|
||||
@@ -89,7 +80,7 @@ mod tests {
|
||||
|
||||
let factory = McpClientFactory::new(Arc::new(config), registry, validator);
|
||||
|
||||
// Should create without error
|
||||
// Should create without error and fall back to local client
|
||||
let result = factory.create();
|
||||
assert!(result.is_ok());
|
||||
}
|
||||
|
||||
@@ -174,3 +174,54 @@ impl Default for ProviderRegistry {
|
||||
Self::new()
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
pub mod test_utils {
|
||||
use super::*;
|
||||
use crate::types::{ChatRequest, ChatResponse, Message, ModelInfo, Role};
|
||||
|
||||
/// Mock provider for testing
|
||||
pub struct MockProvider;
|
||||
|
||||
impl MockProvider {
|
||||
pub fn new() -> Self {
|
||||
Self
|
||||
}
|
||||
}
|
||||
|
||||
#[async_trait::async_trait]
|
||||
impl Provider for MockProvider {
|
||||
fn name(&self) -> &str {
|
||||
"mock"
|
||||
}
|
||||
|
||||
async fn list_models(&self) -> Result<Vec<ModelInfo>> {
|
||||
Ok(vec![ModelInfo {
|
||||
id: "mock-model".to_string(),
|
||||
provider: "mock".to_string(),
|
||||
name: "mock-model".to_string(),
|
||||
description: None,
|
||||
context_window: None,
|
||||
capabilities: vec![],
|
||||
supports_tools: false,
|
||||
}])
|
||||
}
|
||||
|
||||
async fn chat(&self, _request: ChatRequest) -> Result<ChatResponse> {
|
||||
Ok(ChatResponse {
|
||||
message: Message::new(Role::Assistant, "Mock response".to_string()),
|
||||
usage: None,
|
||||
is_streaming: false,
|
||||
is_final: true,
|
||||
})
|
||||
}
|
||||
|
||||
async fn chat_stream(&self, _request: ChatRequest) -> Result<ChatStream> {
|
||||
unimplemented!("MockProvider does not support streaming")
|
||||
}
|
||||
|
||||
async fn health_check(&self) -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user