ext: ask permissions first approach

crates/forge_api/src/api.rs

@@ -3,7 +3,7 @@ use std::path::PathBuf;
 use anyhow::Result;
 use forge_app::dto::ToolsOverview;
 use forge_app::{User, UserUsage};
-use forge_domain::{AgentId, Effort, ModelId, PermissionOperation, ProviderModels};
+use forge_domain::{AgentId, Effort, ModelId, ProviderModels};
 use forge_stream::MpscStream;
 use futures::stream::BoxStream;
 use url::Url;
@@ -124,10 +124,14 @@ pub trait API: Sync + Send {
     /// project directory
     async fn write_mcp_config(&self, scope: &Scope, config: &McpConfig) -> Result<()>;
 
-    /// Unconditionally persists an allow policy for the given operation.
-    /// Use this when the user has explicitly opted in (e.g. via `mcp import`)
-    /// so no interactive confirmation is required on first use.
-    async fn allow_operation(&self, operation: &PermissionOperation) -> Result<()>;
+    /// Prompts for missing permissions for each enabled server in `cfg`.
+    /// Idempotent — servers with existing decisions are skipped.
+    /// Call this synchronously at startup before the REPL takes over stdin.
+    async fn request_mcp_permissions(&self, cfg: McpConfig) -> Result<()>;
+
+    /// Persist `Allow` decisions for the named servers without prompting.
+    /// Used by `mcp import` to record consent on the user's behalf.
+    async fn allow_mcp_servers(&self, names: &[ServerName]) -> Result<()>;
 
     /// Retrieves the provider configuration for the specified agent
     async fn get_agent_provider(&self, agent_id: AgentId) -> anyhow::Result<Provider<Url>>;

crates/forge_api/src/forge_api.rs

@@ -7,7 +7,7 @@ use forge_app::dto::ToolsOverview;
 use forge_app::{
     AgentProviderResolver, AgentRegistry, AppConfigService, AuthService, CommandInfra,
     CommandLoaderService, ConversationService, DataGenerationApp, EnvironmentInfra,
-    FileDiscoveryService, ForgeApp, GitApp, GrpcInfra, McpConfigManager, McpService, PolicyService,
+    FileDiscoveryService, ForgeApp, GitApp, GrpcInfra, McpApp, McpConfigManager, McpService,
     ProviderAuthService, ProviderService, Services, User, UserUsage, Walker, WorkspaceService,
 };
 use forge_config::ForgeConfig;
@@ -39,6 +39,15 @@ impl<A, F> ForgeAPI<A, F> {
     {
         ForgeApp::new(self.services.clone())
     }
+
+    /// Creates an McpApp instance for MCP permission and connection orchestration.
+    fn mcp_app(&self) -> McpApp<A>
+    where
+        A: Services + EnvironmentInfra<Config = forge_config::ForgeConfig>,
+        F: EnvironmentInfra<Config = forge_config::ForgeConfig>,
+    {
+        McpApp::new(self.services.clone())
+    }
 }
 
 impl ForgeAPI<ForgeServices<ForgeRepo<ForgeInfra>>, ForgeRepo<ForgeInfra>> {
@@ -227,8 +236,12 @@ impl<
             .map_err(|e| anyhow::anyhow!(e))
     }
 
-    async fn allow_operation(&self, operation: &PermissionOperation) -> Result<()> {
-        self.services.allow_operation(operation).await
+    async fn allow_mcp_servers(&self, names: &[ServerName]) -> Result<()> {
+        self.mcp_app().allow_mcp_servers(names).await
+    }
+
+    async fn request_mcp_permissions(&self, cfg: McpConfig) -> Result<()> {
+        self.mcp_app().request_mcp_permissions(cfg).await
     }
 
     async fn execute_shell_command_raw(

crates/forge_app/src/lib.rs

@@ -15,6 +15,7 @@ mod git_app;
 mod hooks;
 mod infra;
 mod init_conversation_metrics;
+mod mcp_app;
 mod mcp_executor;
 mod operation;
 mod orch;
@@ -47,6 +48,7 @@ pub use data_gen::*;
 pub use error::*;
 pub use git_app::*;
 pub use infra::*;
+pub use mcp_app::*;
 pub use services::*;
 pub use template_engine::*;
 pub use terminal_context::*;

crates/forge_app/src/mcp_app.rs

@@ -0,0 +1,110 @@
+use std::sync::Arc;
+
+use anyhow::Result;
+use forge_domain::*;
+use merge::Merge;
+
+use crate::services::{McpConfigManager, McpService, PolicyService};
+use crate::{EnvironmentInfra, Services};
+
+/// McpApp handles MCP permission reconciliation and policy-filtered
+/// connections, keeping `McpService` free of any policy awareness.
+pub struct McpApp<S> {
+    services: Arc<S>,
+}
+
+impl<S> McpApp<S> {
+    /// Creates a new McpApp instance with the provided services.
+    pub fn new(services: Arc<S>) -> Self {
+        Self { services }
+    }
+}
+
+impl<S: Services + EnvironmentInfra<Config = forge_config::ForgeConfig>> McpApp<S> {
+    /// Prompts for missing permissions for each enabled server in `cfg`.
+    /// Idempotent — servers that already have a recorded decision are skipped
+    /// silently.
+    ///
+    /// This is the only place a permission prompt can fire for MCP. Call this
+    /// synchronously at startup (before the REPL takes over stdin) so prompts
+    /// don't race with user input.
+    pub async fn request_mcp_permissions(&self, cfg: McpConfig) -> Result<()> {
+        let cwd = self.services.get_environment().cwd;
+        for (name, server) in cfg
+            .mcp_servers
+            .into_iter()
+            .filter(|(_, s)| !s.is_disabled())
+        {
+            let op = PermissionOperation::Mcp {
+                config: server,
+                cwd: cwd.clone(),
+                message: format!("Allow MCP server \"{name}\" to connect?"),
+            };
+            // check_operation_permission handles the prompt + persist.
+            // The return value is intentionally discarded here; the caller
+            // just needs all decisions to be recorded before connections start.
+            let _ = self.services.check_operation_permission(&op).await?;
+        }
+        Ok(())
+    }
+
+    /// Returns a merged MCP config where user-scope servers are trusted
+    /// unconditionally and local-scope servers are filtered to those with an
+    /// explicit `Allow` policy. Never prompts — call
+    /// [`Self::request_mcp_permissions`] first to ensure decisions exist.
+    pub async fn permitted_mcp_config(&self) -> Result<McpConfig> {
+        let mut user = self
+            .services
+            .read_mcp_config(Some(&Scope::User))
+            .await?;
+        let local = self
+            .services
+            .read_mcp_config(Some(&Scope::Local))
+            .await?;
+        let cwd = self.services.get_environment().cwd;
+
+        let mut filtered_local = McpConfig::default();
+        for (name, server) in local.mcp_servers {
+            if server.is_disabled() {
+                continue;
+            }
+            let op = PermissionOperation::Mcp {
+                config: server.clone(),
+                cwd: cwd.clone(),
+                message: String::new(),
+            };
+            if self.services.is_operation_permitted(&op).await? {
+                filtered_local.mcp_servers.insert(name, server);
+            }
+        }
+        user.merge(filtered_local);
+        Ok(user)
+    }
+
+    /// Lists MCP tools, connecting only servers that have an explicit `Allow`
+    /// policy for local-scope entries (user-scope are trusted unconditionally).
+    /// Never prompts.
+    pub async fn get_mcp_servers(&self) -> Result<McpServers> {
+        let cfg = self.permitted_mcp_config().await?;
+        self.services.get_mcp_servers(cfg).await
+    }
+
+    /// Persist `Allow` decisions for the named servers without prompting.
+    /// Used by `mcp import` to record consent on the user's behalf — importing
+    /// is itself an explicit opt-in.
+    pub async fn allow_mcp_servers(&self, names: &[ServerName]) -> Result<()> {
+        let cfg = self.services.read_mcp_config(None).await?;
+        let cwd = self.services.get_environment().cwd;
+        for name in names {
+            if let Some(server) = cfg.mcp_servers.get(name) {
+                let op = PermissionOperation::Mcp {
+                    config: server.clone(),
+                    cwd: cwd.clone(),
+                    message: format!("Connect to MCP server: {name}"),
+                };
+                self.services.allow_operation(&op).await?;
+            }
+        }
+        Ok(())
+    }
+}

crates/forge_app/src/mcp_executor.rs

@@ -2,13 +2,13 @@ use std::sync::Arc;
 
 use forge_domain::{TitleFormat, ToolCallContext, ToolCallFull, ToolName, ToolOutput};
 
-use crate::McpService;
+use crate::{EnvironmentInfra, McpApp, McpService, Services};
 
 pub struct McpExecutor<S> {
     services: Arc<S>,
 }
 
-impl<S: McpService> McpExecutor<S> {
+impl<S: Services + EnvironmentInfra<Config = forge_config::ForgeConfig>> McpExecutor<S> {
     pub fn new(services: Arc<S>) -> Self {
         Self { services }
     }
@@ -22,11 +22,13 @@ impl<S: McpService> McpExecutor<S> {
             .send_tool_input(TitleFormat::info("MCP").sub_title(input.name.as_str()))
             .await?;
 
-        self.services.execute_mcp(input).await
+        let mcp_app = McpApp::new(self.services.clone());
+        let cfg = mcp_app.permitted_mcp_config().await?;
+        self.services.execute_mcp(input, cfg).await
     }
 
     pub async fn contains_tool(&self, tool_name: &ToolName) -> anyhow::Result<bool> {
-        let mcp_servers = self.services.get_mcp_servers().await?;
+        let mcp_servers = McpApp::new(self.services.clone()).get_mcp_servers().await?;
         // Convert Claude Code format (mcp__{server}__{tool}) to the internal legacy
         // format (mcp_{server}_tool_{tool}) before checking, so both name styles match.
         let legacy = tool_name.to_legacy_mcp_name();

crates/forge_app/src/services.rs

@@ -218,8 +218,15 @@ pub trait McpConfigManager: Send + Sync {
 
 #[async_trait::async_trait]
 pub trait McpService: Send + Sync {
-    async fn get_mcp_servers(&self) -> anyhow::Result<McpServers>;
-    async fn execute_mcp(&self, call: ToolCallFull) -> anyhow::Result<ToolOutput>;
+    /// Connect to and list tools from the given MCP servers.
+    /// The caller is responsible for filtering `cfg` through any policy
+    /// gating before calling; this method connects every enabled server
+    /// in `cfg` without re-checking permissions.
+    async fn get_mcp_servers(&self, cfg: McpConfig) -> anyhow::Result<McpServers>;
+    /// Execute a tool call against an already-connected server. The caller is
+    /// responsible for supplying a pre-filtered `cfg` (same one used for
+    /// `get_mcp_servers`) so denied servers are never reconnected here.
+    async fn execute_mcp(&self, call: ToolCallFull, cfg: McpConfig) -> anyhow::Result<ToolOutput>;
     /// Refresh the MCP cache by fetching fresh data
     async fn reload_mcp(&self) -> anyhow::Result<()>;
 }
@@ -702,12 +709,12 @@ impl<I: Services> McpConfigManager for I {
 
 #[async_trait::async_trait]
 impl<I: Services> McpService for I {
-    async fn get_mcp_servers(&self) -> anyhow::Result<McpServers> {
-        self.mcp_service().get_mcp_servers().await
+    async fn get_mcp_servers(&self, cfg: McpConfig) -> anyhow::Result<McpServers> {
+        self.mcp_service().get_mcp_servers(cfg).await
     }
 
-    async fn execute_mcp(&self, call: ToolCallFull) -> anyhow::Result<ToolOutput> {
-        self.mcp_service().execute_mcp(call).await
+    async fn execute_mcp(&self, call: ToolCallFull, cfg: McpConfig) -> anyhow::Result<ToolOutput> {
+        self.mcp_service().execute_mcp(call, cfg).await
     }
 
     async fn reload_mcp(&self) -> anyhow::Result<()> {

crates/forge_app/src/tool_registry.rs

@@ -21,7 +21,7 @@ use crate::fmt::content::FormatContent;
 use crate::mcp_executor::McpExecutor;
 use crate::tool_executor::ToolExecutor;
 use crate::{
-    AgentRegistry, EnvironmentInfra, McpService, PolicyService, ProviderService, Services,
+    AgentRegistry, EnvironmentInfra, McpApp, PolicyService, ProviderService, Services,
     ToolResolver, WorkspaceService,
 };
 
@@ -241,7 +241,7 @@ impl<S: Services + EnvironmentInfra<Config = forge_config::ForgeConfig>> ToolReg
     }
 
     pub async fn tools_overview(&self) -> anyhow::Result<ToolsOverview> {
-        let mcp_tools = self.services.get_mcp_servers().await?;
+        let mcp_tools = McpApp::new(self.services.clone()).get_mcp_servers().await?;
         let agent_tools = self.agent_executor.agent_definitions().await?;
 
         // Get agents for template rendering in Task tool description

crates/forge_main/src/ui.rs

@@ -11,7 +11,7 @@ use convert_case::{Case, Casing};
 use forge_api::{
     API, AgentId, AnyProvider, ApiKeyRequest, AuthContextRequest, AuthContextResponse, ChatRequest,
     ChatResponse, CodeRequest, ConfigOperation, Conversation, ConversationId, DeviceCodeRequest,
-    Event, InterruptionReason, ModelId, Provider, ProviderId, TextMessage, UserPrompt,
+    Event, InterruptionReason, ModelId, Provider, ProviderId, Scope, TextMessage, UserPrompt,
 };
 use forge_app::utils::{format_display_path, truncate_key};
 use forge_app::{CommitResult, ToolResolver};
@@ -223,9 +223,7 @@ impl<A: API + ConsoleWriter + 'static, F: Fn(ForgeConfig) -> A + Send + Sync> UI
         self.display_banner()?;
         self.trace_user();
         self.hydrate_caches();
-        // Resolve MCP connections up front and surface any permission
-        // warnings before control returns to the caller.
-        let _ = self.api.get_tools().await;
+        self.request_local_mcp_permissions().await?;
         Ok(())
     }
 
@@ -370,10 +368,7 @@ impl<A: API + ConsoleWriter + 'static, F: Fn(ForgeConfig) -> A + Send + Sync> UI
         self.trace_user();
         self.hydrate_caches();
         self.init_conversation().await?;
-
-        // Initialise MCP connections and display any permission warnings
-        // before the REPL takes over stdin.
-        let _ = self.api.get_tools().await;
+        self.request_local_mcp_permissions().await?;
 
         // Check for dispatch flag first
         if let Some(dispatch_json) = self.cli.event.clone() {
@@ -450,11 +445,22 @@ impl<A: API + ConsoleWriter + 'static, F: Fn(ForgeConfig) -> A + Send + Sync> UI
         }
     }
 
+    /// Reads the local-scope MCP config and asks the user for permission for
+    /// each server that does not yet have a recorded decision. Call this
+    /// synchronously before the REPL takes over stdin so prompts don't race
+    /// with user input.
+    async fn request_local_mcp_permissions(&self) -> Result<()> {
+        let local_cfg = self.api.read_mcp_config(Some(&Scope::Local)).await?;
+        self.api.request_mcp_permissions(local_cfg).await
+    }
+
     // Improve startup time by hydrating caches
     fn hydrate_caches(&self) {
         let api = self.api.clone();
         tokio::spawn(async move { api.get_models().await });
         let api = self.api.clone();
+        tokio::spawn(async move { let _ = api.get_tools().await; });
+        let api = self.api.clone();
         tokio::spawn(async move { api.get_agent_infos().await });
         let api = self.api.clone();
         tokio::spawn(async move {
@@ -574,21 +580,9 @@ impl<A: API + ConsoleWriter + 'static, F: Fn(ForgeConfig) -> A + Send + Sync> UI
                     // Write back to the specific scope only
                     self.api.write_mcp_config(&scope, &scope_config).await?;
 
-                    let cwd = self.api.environment().cwd;
-
-                    // Grant allow permission for each imported server so the user
-                    // is not prompted again on first use — importing is itself an
-                    // explicit opt-in.
-                    for server_name in &added_servers {
-                        if let Some(server_config) = scope_config.mcp_servers.get(server_name) {
-                            let operation = forge_domain::PermissionOperation::Mcp {
-                                config: server_config.clone(),
-                                cwd: cwd.clone(),
-                                message: format!("Connect to MCP server: {server_name}"),
-                            };
-                            self.api.allow_operation(&operation).await?;
-                        }
-                    }
+                    // Importing is an explicit opt-in — persist Allow decisions so
+                    // the user is not prompted on first use.
+                    self.api.allow_mcp_servers(&added_servers).await?;
 
                     // Log each added server after successful write
                     for server_name in added_servers {

crates/forge_services/src/forge_services.rs

@@ -30,8 +30,7 @@ use crate::tool_services::{
     ForgeFsUndo, ForgeFsWrite, ForgeImageRead, ForgePlanCreate, ForgeShell, ForgeSkillFetch,
 };
 
-type McpService<F> =
-    ForgeMcpService<ForgeMcpManager<F>, F, <F as McpServerInfra>::Client, ForgePolicyService<F>>;
+type McpService<F> = ForgeMcpService<F, <F as McpServerInfra>::Client>;
 type AuthService<F> = ForgeAuthService<F>;
 
 /// ForgeApp is the main application container that implements the App trait.
@@ -110,13 +109,9 @@ impl<
 > ForgeServices<F>
 {
     pub fn new(infra: Arc<F>) -> Self {
+        let mcp_service = Arc::new(ForgeMcpService::new(infra.clone()));
         let mcp_manager = Arc::new(ForgeMcpManager::new(infra.clone()));
         let policy_service = ForgePolicyService::new(infra.clone());
-        let mcp_service = Arc::new(ForgeMcpService::new(
-            mcp_manager.clone(),
-            infra.clone(),
-            Arc::new(policy_service.clone()),
-        ));
         let template_service = Arc::new(ForgeTemplateService::new(infra.clone()));
         let attachment_service = Arc::new(ForgeChatRequest::new(infra.clone()));
         let suggestion_service = Arc::new(ForgeDiscoveryService::new(infra.clone()));