refactor: encapsulate leaked abstractions into owning modules (#778)

* refactor: encapsulate leaked abstractions from main.rs and app.rs into owning modules Move module-specific initialization logic out of main.rs (1222→665 lines, -46%) and app.rs (944→780 lines, -17%) into their respective owning modules as public factory functions. This enforces separation of concerns so that adding a new DB backend, MCP transport, or channel doesn't require editing main.rs/app.rs. Key changes: - Tracing init functions → src/tracing_fmt.rs - DB connection factory (connect_with_handles + DatabaseHandles) → src/db/mod.rs - Secrets store factory (create_secrets_store) → src/secrets/mod.rs - MCP transport dispatch factory (create_client_from_config) → src/tools/mcp/factory.rs - Orchestrator setup (setup_orchestrator + OrchestratorSetup) → src/orchestrator/mod.rs - WASM channel setup (setup_wasm_channels) → src/channels/wasm/setup.rs - Worker entry points (run_worker, run_claude_bridge) → src/worker/mod.rs - Shared CLI secrets init (init_secrets_store) → src/cli/mod.rs - Tunnel startup (start_managed_tunnel) → src/tunnel/mod.rs - Onboard check (check_onboard_needed) → src/setup/mod.rs - ExtensionManager unified MCP: uses create_client_from_config via McpProcessManager, enabling stdio/Unix transports for hot-activated MCP servers - Deduplicated ~130 lines of secrets store init across cli/mcp.rs and cli/tool.rs - CLAUDE.md updated with module-owned initialization guideline [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * refactor: address review feedback — deduplicate db factory, extract channel helper - connect_from_config() now delegates to connect_with_handles() to eliminate duplicated backend-matching logic (Copilot review feedback) - Extract register_channel() helper from setup_wasm_channels() loop body to improve readability (Gemini review feedback) [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * style: fix rustfmt line wrapping in setup_wasm_channels Co-Authored-By: Claude Opus 4.6 <[email protected]> * test: add integration test for module-owned initialization factories Exercises the full factory chain end-to-end to verify nothing was lost when initialization logic was moved from main.rs/app.rs into owning modules: - connect_with_handles returns Database + populated backend handles - connect_from_config delegates correctly (produces working Database) - secrets::create_secrets_store builds working store from DatabaseHandles - db::create_secrets_store standalone factory round-trips secrets - Both secrets factories produce compatible stores (cross-read works) - ExtensionManager constructs with McpProcessManager and is functional - DatabaseHandles default is empty All tests run without external services using libsql in-memory/tempfile. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: wire cli/mcp.rs and cli/tool.rs to shared init_secrets_store() Both files had inline implementations identical to cli::init_secrets_store(). Replace with delegation to complete the claimed deduplication. [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * style: fix rustfmt line wrapping in integration test Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix(review): remove unused Config import and deduplicate Error Handling section - Remove `#[allow(unused_imports)]` and unused `use crate::config::Config` from cli/tool.rs (no longer needed after delegating to shared `cli::init_secrets_store()`) - Remove duplicate Error Handling subsection from CLAUDE.md Key Patterns (all four bullets already exist in Code Style section and review-discipline.md) Addresses Copilot review comments. [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix(review): address remaining Copilot review comments - secrets/mod.rs: clarify docstring that None is a normal no-db condition - app.rs: add comment explaining the empty_handles fallback path - orchestrator/mod.rs: combine duplicated sandbox condition into single block - setup/mod.rs: document env var reads and thread-safety caveat [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> --------- Co-authored-by: Claude Opus 4.6 <[email protected]> Co-authored-by: Henry Park <[email protected]>
2026-08-31 16:49:34 +00:00 · 2026-03-10 04:39:51 +00:00
parent a868b14221
commit 94d101924e
21 changed files with 1167 additions and 815 deletions
@@ -777,6 +777,7 @@ mod tests {

        Arc::new(ExtensionManager::new(
            Arc::new(McpSessionManager::new()),
+            Arc::new(crate::tools::mcp::process::McpProcessManager::new()),
            Arc::new(InMemorySecretsStore::new(crypto)),
            Arc::new(ToolRegistry::new()),
            None,
@@ -0,0 +1,98 @@
+//! Factory for creating MCP clients from server configuration.
+//!
+//! Encapsulates the transport dispatch logic (stdio, Unix socket, HTTP)
+//! so that callers don't need to match on `EffectiveTransport` themselves.
+
+use std::sync::Arc;
+
+use crate::secrets::SecretsStore;
+use crate::tools::mcp::config::{EffectiveTransport, McpServerConfig};
+use crate::tools::mcp::{McpClient, McpProcessManager, McpSessionManager, McpTransport};
+
+/// Error returned when MCP client creation fails.
+#[derive(Debug, thiserror::Error)]
+pub enum McpFactoryError {
+    #[error("Failed to spawn stdio MCP server '{name}': {reason}")]
+    StdioSpawn { name: String, reason: String },
+    #[error("Failed to connect to Unix MCP server '{name}': {reason}")]
+    UnixConnect { name: String, reason: String },
+    #[error("Unix socket transport is not supported on this platform (server '{name}')")]
+    UnixNotSupported { name: String },
+}
+
+/// Create an `McpClient` from a server configuration, dispatching on the
+/// effective transport type.
+pub async fn create_client_from_config(
+    server: McpServerConfig,
+    session_manager: &Arc<McpSessionManager>,
+    process_manager: &Arc<McpProcessManager>,
+    secrets: Option<Arc<dyn SecretsStore + Send + Sync>>,
+    user_id: &str,
+) -> Result<McpClient, McpFactoryError> {
+    let server_name = server.name.clone();
+
+    match server.effective_transport() {
+        EffectiveTransport::Stdio { command, args, env } => {
+            let transport = process_manager
+                .spawn_stdio(&server_name, command, args.to_vec(), env.clone())
+                .await
+                .map_err(|e| McpFactoryError::StdioSpawn {
+                    name: server_name.clone(),
+                    reason: e.to_string(),
+                })?;
+
+            Ok(McpClient::new_with_transport(
+                &server_name,
+                transport as Arc<dyn McpTransport>,
+                None,
+                secrets,
+                user_id,
+                Some(server),
+            ))
+        }
+        #[cfg(unix)]
+        EffectiveTransport::Unix { socket_path } => {
+            let transport = crate::tools::mcp::unix_transport::UnixMcpTransport::connect(
+                &server_name,
+                socket_path,
+            )
+            .await
+            .map_err(|e| McpFactoryError::UnixConnect {
+                name: server_name.clone(),
+                reason: e.to_string(),
+            })?;
+
+            Ok(McpClient::new_with_transport(
+                &server_name,
+                Arc::new(transport) as Arc<dyn McpTransport>,
+                None,
+                secrets,
+                user_id,
+                Some(server),
+            ))
+        }
+        #[cfg(not(unix))]
+        EffectiveTransport::Unix { .. } => {
+            Err(McpFactoryError::UnixNotSupported { name: server_name })
+        }
+        EffectiveTransport::Http => {
+            if let Some(ref secrets) = secrets {
+                let has_tokens =
+                    crate::tools::mcp::is_authenticated(&server, secrets, user_id).await;
+
+                if has_tokens || server.requires_auth() {
+                    Ok(McpClient::new_authenticated(
+                        server,
+                        Arc::clone(session_manager),
+                        Arc::clone(secrets),
+                        user_id,
+                    ))
+                } else {
+                    Ok(McpClient::new_with_config(server))
+                }
+            } else {
+                Ok(McpClient::new_with_config(server))
+            }
+        }
+    }
+}
@@ -31,6 +31,7 @@
 pub mod auth;
 mod client;
 pub mod config;
+pub mod factory;
 pub(crate) mod http_transport;
 pub(crate) mod process;
 mod protocol;
@@ -43,6 +44,7 @@ pub(crate) mod unix_transport;
 pub use auth::{is_authenticated, refresh_access_token};
 pub use client::McpClient;
 pub use config::{McpServerConfig, McpServersFile, OAuthConfig};
+pub use factory::{McpFactoryError, create_client_from_config};
 pub use process::McpProcessManager;
 pub use protocol::{InitializeResult, McpRequest, McpResponse, McpTool};
 pub use session::McpSessionManager;