From aa36a15f9f473370b82d1046197ae6910b79c4a5 Mon Sep 17 00:00:00 2001
From: Michael Bolin <mbolin@openai.com>
Date: Tue, 6 May 2025 16:21:35 -0700
Subject: [PATCH] fix: make all fields of Session struct private again (#840)

https://github.com/openai/codex/pull/829 noted it introduced a circular
dep between `codex.rs` and `mcp_tool_call.rs`. This attempts to clean
things up: the circular dep still exists, but at least all the fields of
`Session` are private again.
---
 codex-rs/core/src/codex.rs         | 23 ++++++++++--
 codex-rs/core/src/mcp_tool_call.rs | 56 +++++++++++++-----------------
 2 files changed, 45 insertions(+), 34 deletions(-)

diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs
index 250cbfdd..b5c04ddd 100644
--- a/codex-rs/core/src/codex.rs
+++ b/codex-rs/core/src/codex.rs
@@ -195,7 +195,7 @@ impl Recorder {
 /// A session has at most 1 running task at a time, and can be interrupted by user input.
 pub(crate) struct Session {
     client: ModelClient,
-    pub(crate) tx_event: Sender<Event>,
+    tx_event: Sender<Event>,
     ctrl_c: Arc<Notify>,
 
     /// The session's current working directory. All relative paths provided by
@@ -208,7 +208,7 @@ pub(crate) struct Session {
     writable_roots: Mutex<Vec<PathBuf>>,
 
     /// Manager for external MCP servers/tools.
-    pub(crate) mcp_connection_manager: McpConnectionManager,
+    mcp_connection_manager: McpConnectionManager,
 
     /// External notifier command (will be passed as args to exec()). When
     /// `None` this feature is disabled.
@@ -253,6 +253,14 @@ impl Session {
         }
     }
 
+    /// Sends the given event to the client and swallows the send event, if
+    /// any, logging it as an error.
+    pub(crate) async fn send_event(&self, event: Event) {
+        if let Err(e) = self.tx_event.send(event).await {
+            error!("failed to send tool call event: {e}");
+        }
+    }
+
     pub async fn request_command_approval(
         &self,
         sub_id: String,
@@ -383,6 +391,17 @@ impl Session {
         }
     }
 
+    pub async fn call_tool(
+        &self,
+        server: &str,
+        tool: &str,
+        arguments: Option<serde_json::Value>,
+    ) -> anyhow::Result<mcp_types::CallToolResult> {
+        self.mcp_connection_manager
+            .call_tool(server, tool, arguments)
+            .await
+    }
+
     pub fn abort(&self) {
         info!("Aborting existing session");
         let mut state = self.state.lock().unwrap();
diff --git a/codex-rs/core/src/mcp_tool_call.rs b/codex-rs/core/src/mcp_tool_call.rs
index 9967271a..0b6401f7 100644
--- a/codex-rs/core/src/mcp_tool_call.rs
+++ b/codex-rs/core/src/mcp_tool_call.rs
@@ -45,28 +45,25 @@ pub(crate) async fn handle_mcp_tool_call(
     notify_mcp_tool_call_event(sess, sub_id, tool_call_begin_event).await;
 
     // Perform the tool call.
-    let (tool_call_end_event, tool_call_err) = match sess
-        .mcp_connection_manager
-        .call_tool(&server, &tool_name, arguments_value)
-        .await
-    {
-        Ok(result) => (
-            EventMsg::McpToolCallEnd {
-                call_id,
-                success: !result.is_error.unwrap_or(false),
-                result: Some(result),
-            },
-            None,
-        ),
-        Err(e) => (
-            EventMsg::McpToolCallEnd {
-                call_id,
-                success: false,
-                result: None,
-            },
-            Some(e),
-        ),
-    };
+    let (tool_call_end_event, tool_call_err) =
+        match sess.call_tool(&server, &tool_name, arguments_value).await {
+            Ok(result) => (
+                EventMsg::McpToolCallEnd {
+                    call_id,
+                    success: !result.is_error.unwrap_or(false),
+                    result: Some(result),
+                },
+                None,
+            ),
+            Err(e) => (
+                EventMsg::McpToolCallEnd {
+                    call_id,
+                    success: false,
+                    result: None,
+                },
+                Some(e),
+            ),
+        };
 
     notify_mcp_tool_call_event(sess, sub_id, tool_call_end_event.clone()).await;
     let EventMsg::McpToolCallEnd {
@@ -94,14 +91,9 @@ pub(crate) async fn handle_mcp_tool_call(
 }
 
 async fn notify_mcp_tool_call_event(sess: &Session, sub_id: &str, event: EventMsg) {
-    if let Err(e) = sess
-        .tx_event
-        .send(Event {
-            id: sub_id.to_string(),
-            msg: event,
-        })
-        .await
-    {
-        error!("failed to send tool call event: {e}");
-    }
+    sess.send_event(Event {
+        id: sub_id.to_string(),
+        msg: event,
+    })
+    .await;
 }