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.
This commit is contained in:
Michael Bolin
2025-05-06 16:21:35 -07:00
committed by GitHub
parent 88e7ca5f2b
commit aa36a15f9f
2 changed files with 45 additions and 34 deletions

View File

@@ -195,7 +195,7 @@ impl Recorder {
/// A session has at most 1 running task at a time, and can be interrupted by user input. /// A session has at most 1 running task at a time, and can be interrupted by user input.
pub(crate) struct Session { pub(crate) struct Session {
client: ModelClient, client: ModelClient,
pub(crate) tx_event: Sender<Event>, tx_event: Sender<Event>,
ctrl_c: Arc<Notify>, ctrl_c: Arc<Notify>,
/// The session's current working directory. All relative paths provided by /// The session's current working directory. All relative paths provided by
@@ -208,7 +208,7 @@ pub(crate) struct Session {
writable_roots: Mutex<Vec<PathBuf>>, writable_roots: Mutex<Vec<PathBuf>>,
/// Manager for external MCP servers/tools. /// 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 /// External notifier command (will be passed as args to exec()). When
/// `None` this feature is disabled. /// `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( pub async fn request_command_approval(
&self, &self,
sub_id: String, 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) { pub fn abort(&self) {
info!("Aborting existing session"); info!("Aborting existing session");
let mut state = self.state.lock().unwrap(); let mut state = self.state.lock().unwrap();

View File

@@ -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; notify_mcp_tool_call_event(sess, sub_id, tool_call_begin_event).await;
// Perform the tool call. // Perform the tool call.
let (tool_call_end_event, tool_call_err) = match sess let (tool_call_end_event, tool_call_err) =
.mcp_connection_manager match sess.call_tool(&server, &tool_name, arguments_value).await {
.call_tool(&server, &tool_name, arguments_value) Ok(result) => (
.await EventMsg::McpToolCallEnd {
{ call_id,
Ok(result) => ( success: !result.is_error.unwrap_or(false),
EventMsg::McpToolCallEnd { result: Some(result),
call_id, },
success: !result.is_error.unwrap_or(false), None,
result: Some(result), ),
}, Err(e) => (
None, EventMsg::McpToolCallEnd {
), call_id,
Err(e) => ( success: false,
EventMsg::McpToolCallEnd { result: None,
call_id, },
success: false, Some(e),
result: None, ),
}, };
Some(e),
),
};
notify_mcp_tool_call_event(sess, sub_id, tool_call_end_event.clone()).await; notify_mcp_tool_call_event(sess, sub_id, tool_call_end_event.clone()).await;
let EventMsg::McpToolCallEnd { 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) { async fn notify_mcp_tool_call_event(sess: &Session, sub_id: &str, event: EventMsg) {
if let Err(e) = sess sess.send_event(Event {
.tx_event id: sub_id.to_string(),
.send(Event { msg: event,
id: sub_id.to_string(), })
msg: event, .await;
})
.await
{
error!("failed to send tool call event: {e}");
}
} }