feat: persist and restore codex app's tools after search (#11780)
### What changed 1. Removed per-turn MCP selection reset in `core/src/tasks/mod.rs`. 2. Added `SessionState::set_mcp_tool_selection(Vec<String>)` in `core/src/state/session.rs` for authoritative restore behavior (deduped, order-preserving, empty clears). 3. Added rollout parsing in `core/src/codex.rs` to recover `active_selected_tools` from prior `search_tool_bm25` outputs: - tracks matching `call_id`s - parses function output text JSON - extracts `active_selected_tools` - latest valid payload wins - malformed/non-matching payloads are ignored 4. Applied restore logic to resumed and forked startup paths in `core/src/codex.rs`. 5. Updated instruction text to session/thread scope in `core/templates/search_tool/tool_description.md`. 6. Expanded tests in `core/tests/suite/search_tool.rs`, plus unit coverage in: - `core/src/codex.rs` - `core/src/state/session.rs` ### Behavior after change 1. Search activates matched tools. 2. Additional searches union into active selection. 3. Selection survives new turns in the same thread. 4. Resume/fork restores selection from rollout history. 5. Separate threads do not inherit selection unless forked.
This commit is contained in:
parent
060a320e7d
commit
02abd9a8ea
12 changed files with 1201 additions and 201 deletions
|
|
@ -150,6 +150,7 @@ use crate::mcp::with_codex_apps_mcp;
|
|||
use crate::mcp_connection_manager::McpConnectionManager;
|
||||
use crate::mcp_connection_manager::filter_codex_apps_mcp_tools_only;
|
||||
use crate::mcp_connection_manager::filter_mcp_tools_by_name;
|
||||
use crate::mcp_connection_manager::filter_non_codex_apps_mcp_tools_only;
|
||||
use crate::memories;
|
||||
use crate::mentions::build_connector_slug_counts;
|
||||
use crate::mentions::build_skill_name_counts;
|
||||
|
|
@ -223,6 +224,7 @@ use crate::tasks::SessionTask;
|
|||
use crate::tasks::SessionTaskContext;
|
||||
use crate::tools::ToolRouter;
|
||||
use crate::tools::context::SharedTurnDiffTracker;
|
||||
use crate::tools::handlers::SEARCH_TOOL_BM25_TOOL_NAME;
|
||||
use crate::tools::js_repl::JsReplHandle;
|
||||
use crate::tools::network_approval::NetworkApprovalService;
|
||||
use crate::tools::network_approval::build_blocked_request_observer;
|
||||
|
|
@ -1489,6 +1491,11 @@ impl Session {
|
|||
state.merge_mcp_tool_selection(tool_names)
|
||||
}
|
||||
|
||||
pub(crate) async fn set_mcp_tool_selection(&self, tool_names: Vec<String>) {
|
||||
let mut state = self.state.lock().await;
|
||||
state.set_mcp_tool_selection(tool_names);
|
||||
}
|
||||
|
||||
pub(crate) async fn get_mcp_tool_selection(&self) -> Option<Vec<String>> {
|
||||
let state = self.state.lock().await;
|
||||
state.get_mcp_tool_selection()
|
||||
|
|
@ -1522,6 +1529,7 @@ impl Session {
|
|||
|
||||
async fn record_initial_history(&self, conversation_history: InitialHistory) {
|
||||
let turn_context = self.new_default_turn().await;
|
||||
self.clear_mcp_tool_selection().await;
|
||||
match conversation_history {
|
||||
InitialHistory::New => {
|
||||
// Build and record initial items (user instructions + environment context)
|
||||
|
|
@ -1537,6 +1545,8 @@ impl Session {
|
|||
}
|
||||
InitialHistory::Resumed(resumed_history) => {
|
||||
let rollout_items = resumed_history.history;
|
||||
let restored_tool_selection =
|
||||
Self::extract_mcp_tool_selection_from_rollout(&rollout_items);
|
||||
let previous_model = Self::last_rollout_model_name(&rollout_items)
|
||||
.map(std::string::ToString::to_string);
|
||||
{
|
||||
|
|
@ -1578,12 +1588,17 @@ impl Session {
|
|||
let mut state = self.state.lock().await;
|
||||
state.set_token_info(Some(info));
|
||||
}
|
||||
if let Some(selected_tools) = restored_tool_selection {
|
||||
self.set_mcp_tool_selection(selected_tools).await;
|
||||
}
|
||||
|
||||
// Defer seeding the session's initial context until the first turn starts so
|
||||
// turn/start overrides can be merged before we write to the rollout.
|
||||
self.flush_rollout().await;
|
||||
}
|
||||
InitialHistory::Forked(rollout_items) => {
|
||||
let restored_tool_selection =
|
||||
Self::extract_mcp_tool_selection_from_rollout(&rollout_items);
|
||||
let previous_model = Self::last_rollout_model_name(&rollout_items)
|
||||
.map(std::string::ToString::to_string);
|
||||
self.set_previous_model(previous_model).await;
|
||||
|
|
@ -1603,6 +1618,9 @@ impl Session {
|
|||
let mut state = self.state.lock().await;
|
||||
state.set_token_info(Some(info));
|
||||
}
|
||||
if let Some(selected_tools) = restored_tool_selection {
|
||||
self.set_mcp_tool_selection(selected_tools).await;
|
||||
}
|
||||
|
||||
// If persisting, persist all rollout items as-is (recorder filters)
|
||||
if !rollout_items.is_empty() {
|
||||
|
|
@ -1644,6 +1662,54 @@ impl Session {
|
|||
})
|
||||
}
|
||||
|
||||
fn extract_mcp_tool_selection_from_rollout(
|
||||
rollout_items: &[RolloutItem],
|
||||
) -> Option<Vec<String>> {
|
||||
let mut search_call_ids = HashSet::new();
|
||||
let mut active_selected_tools: Option<Vec<String>> = None;
|
||||
|
||||
for item in rollout_items {
|
||||
let RolloutItem::ResponseItem(response_item) = item else {
|
||||
continue;
|
||||
};
|
||||
match response_item {
|
||||
ResponseItem::FunctionCall { name, call_id, .. } => {
|
||||
if name == SEARCH_TOOL_BM25_TOOL_NAME {
|
||||
search_call_ids.insert(call_id.clone());
|
||||
}
|
||||
}
|
||||
ResponseItem::FunctionCallOutput { call_id, output } => {
|
||||
if !search_call_ids.contains(call_id) {
|
||||
continue;
|
||||
}
|
||||
let Some(content) = output.body.to_text() else {
|
||||
continue;
|
||||
};
|
||||
let Ok(payload) = serde_json::from_str::<Value>(&content) else {
|
||||
continue;
|
||||
};
|
||||
let Some(selected_tools) = payload
|
||||
.get("active_selected_tools")
|
||||
.and_then(Value::as_array)
|
||||
else {
|
||||
continue;
|
||||
};
|
||||
let Some(selected_tools) = selected_tools
|
||||
.iter()
|
||||
.map(|value| value.as_str().map(str::to_string))
|
||||
.collect::<Option<Vec<_>>>()
|
||||
else {
|
||||
continue;
|
||||
};
|
||||
active_selected_tools = Some(selected_tools);
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
|
||||
active_selected_tools
|
||||
}
|
||||
|
||||
async fn previous_model(&self) -> Option<String> {
|
||||
let state = self.state.lock().await;
|
||||
state.previous_model()
|
||||
|
|
@ -4899,6 +4965,10 @@ async fn built_tools(
|
|||
None
|
||||
};
|
||||
|
||||
let app_tools = connectors
|
||||
.as_ref()
|
||||
.map(|connectors| filter_codex_apps_mcp_tools(&mcp_tools, connectors));
|
||||
|
||||
if let Some(connectors) = connectors.as_ref() {
|
||||
let skill_name_counts_lower = skills_outcome.map_or_else(HashMap::new, |outcome| {
|
||||
build_skill_name_counts(&outcome.skills, &outcome.disabled_paths).1
|
||||
|
|
@ -4911,24 +4981,20 @@ async fn built_tools(
|
|||
&skill_name_counts_lower,
|
||||
);
|
||||
|
||||
let mut selected_mcp_tools =
|
||||
if let Some(selected_tools) = sess.get_mcp_tool_selection().await {
|
||||
filter_mcp_tools_by_name(&mcp_tools, &selected_tools)
|
||||
} else {
|
||||
HashMap::new()
|
||||
};
|
||||
let mut selected_mcp_tools = filter_non_codex_apps_mcp_tools_only(&mcp_tools);
|
||||
|
||||
let apps_mcp_tools =
|
||||
filter_codex_apps_mcp_tools_only(&mcp_tools, explicitly_enabled.as_ref());
|
||||
selected_mcp_tools.extend(apps_mcp_tools);
|
||||
if let Some(selected_tools) = sess.get_mcp_tool_selection().await {
|
||||
selected_mcp_tools.extend(filter_mcp_tools_by_name(&mcp_tools, &selected_tools));
|
||||
}
|
||||
|
||||
selected_mcp_tools.extend(filter_codex_apps_mcp_tools_only(
|
||||
&mcp_tools,
|
||||
explicitly_enabled.as_ref(),
|
||||
));
|
||||
|
||||
mcp_tools = selected_mcp_tools;
|
||||
}
|
||||
|
||||
let app_tools = connectors
|
||||
.as_ref()
|
||||
.map(|connectors| filter_codex_apps_mcp_tools(&mcp_tools, connectors));
|
||||
|
||||
Ok(Arc::new(ToolRouter::from_config(
|
||||
&turn_context.tools_config,
|
||||
Some(
|
||||
|
|
@ -5789,6 +5855,22 @@ mod tests {
|
|||
}
|
||||
}
|
||||
|
||||
fn function_call_rollout_item(name: &str, call_id: &str) -> RolloutItem {
|
||||
RolloutItem::ResponseItem(ResponseItem::FunctionCall {
|
||||
id: None,
|
||||
name: name.to_string(),
|
||||
arguments: "{}".to_string(),
|
||||
call_id: call_id.to_string(),
|
||||
})
|
||||
}
|
||||
|
||||
fn function_call_output_rollout_item(call_id: &str, output: &str) -> RolloutItem {
|
||||
RolloutItem::ResponseItem(ResponseItem::FunctionCallOutput {
|
||||
call_id: call_id.to_string(),
|
||||
output: FunctionCallOutputPayload::from_text(output.to_string()),
|
||||
})
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn get_base_instructions_no_user_content() {
|
||||
let prompt_with_apply_patch_instructions =
|
||||
|
|
@ -5978,6 +6060,46 @@ mod tests {
|
|||
assert_eq!(connector_ids, HashSet::<String>::new());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn non_app_mcp_tools_remain_visible_without_search_selection() {
|
||||
let mcp_tools = HashMap::from([
|
||||
(
|
||||
"mcp__codex_apps__calendar_create_event".to_string(),
|
||||
make_mcp_tool(
|
||||
CODEX_APPS_MCP_SERVER_NAME,
|
||||
"calendar_create_event",
|
||||
Some("calendar"),
|
||||
Some("Calendar"),
|
||||
),
|
||||
),
|
||||
(
|
||||
"mcp__rmcp__echo".to_string(),
|
||||
make_mcp_tool("rmcp", "echo", None, None),
|
||||
),
|
||||
]);
|
||||
|
||||
let mut selected_mcp_tools = mcp_tools
|
||||
.iter()
|
||||
.filter(|(_, tool)| tool.server_name != CODEX_APPS_MCP_SERVER_NAME)
|
||||
.map(|(name, tool)| (name.clone(), tool.clone()))
|
||||
.collect::<HashMap<_, _>>();
|
||||
|
||||
let connectors = connectors::accessible_connectors_from_mcp_tools(&mcp_tools);
|
||||
let explicitly_enabled_connectors = HashSet::new();
|
||||
let connectors = filter_connectors_for_input(
|
||||
&connectors,
|
||||
&[user_message("run echo")],
|
||||
&explicitly_enabled_connectors,
|
||||
&HashMap::new(),
|
||||
);
|
||||
let apps_mcp_tools = filter_codex_apps_mcp_tools_only(&mcp_tools, &connectors);
|
||||
selected_mcp_tools.extend(apps_mcp_tools);
|
||||
|
||||
let mut tool_names: Vec<String> = selected_mcp_tools.into_keys().collect();
|
||||
tool_names.sort();
|
||||
assert_eq!(tool_names, vec!["mcp__rmcp__echo".to_string()]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn search_tool_selection_keeps_codex_apps_tools_without_mentions() {
|
||||
let selected_tool_names = vec![
|
||||
|
|
@ -6065,6 +6187,106 @@ mod tests {
|
|||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn extract_mcp_tool_selection_from_rollout_reads_search_tool_output() {
|
||||
let rollout_items = vec![
|
||||
function_call_rollout_item(SEARCH_TOOL_BM25_TOOL_NAME, "search-1"),
|
||||
function_call_output_rollout_item(
|
||||
"search-1",
|
||||
&json!({
|
||||
"active_selected_tools": [
|
||||
"mcp__codex_apps__calendar_create_event",
|
||||
"mcp__codex_apps__calendar_list_events",
|
||||
],
|
||||
})
|
||||
.to_string(),
|
||||
),
|
||||
];
|
||||
|
||||
let selected = Session::extract_mcp_tool_selection_from_rollout(&rollout_items);
|
||||
assert_eq!(
|
||||
selected,
|
||||
Some(vec![
|
||||
"mcp__codex_apps__calendar_create_event".to_string(),
|
||||
"mcp__codex_apps__calendar_list_events".to_string(),
|
||||
])
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn extract_mcp_tool_selection_from_rollout_latest_valid_payload_wins() {
|
||||
let rollout_items = vec![
|
||||
function_call_rollout_item(SEARCH_TOOL_BM25_TOOL_NAME, "search-1"),
|
||||
function_call_output_rollout_item(
|
||||
"search-1",
|
||||
&json!({
|
||||
"active_selected_tools": ["mcp__codex_apps__calendar_create_event"],
|
||||
})
|
||||
.to_string(),
|
||||
),
|
||||
function_call_rollout_item(SEARCH_TOOL_BM25_TOOL_NAME, "search-2"),
|
||||
function_call_output_rollout_item(
|
||||
"search-2",
|
||||
&json!({
|
||||
"active_selected_tools": ["mcp__codex_apps__calendar_delete_event"],
|
||||
})
|
||||
.to_string(),
|
||||
),
|
||||
];
|
||||
|
||||
let selected = Session::extract_mcp_tool_selection_from_rollout(&rollout_items);
|
||||
assert_eq!(
|
||||
selected,
|
||||
Some(vec!["mcp__codex_apps__calendar_delete_event".to_string(),])
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn extract_mcp_tool_selection_from_rollout_ignores_non_search_and_malformed_payloads() {
|
||||
let rollout_items = vec![
|
||||
function_call_rollout_item("shell", "shell-1"),
|
||||
function_call_output_rollout_item(
|
||||
"shell-1",
|
||||
&json!({
|
||||
"active_selected_tools": ["mcp__codex_apps__should_be_ignored"],
|
||||
})
|
||||
.to_string(),
|
||||
),
|
||||
function_call_rollout_item(SEARCH_TOOL_BM25_TOOL_NAME, "search-1"),
|
||||
function_call_output_rollout_item("search-1", "{not-json"),
|
||||
function_call_output_rollout_item(
|
||||
"unknown-search-call",
|
||||
&json!({
|
||||
"active_selected_tools": ["mcp__codex_apps__also_ignored"],
|
||||
})
|
||||
.to_string(),
|
||||
),
|
||||
function_call_output_rollout_item(
|
||||
"search-1",
|
||||
&json!({
|
||||
"active_selected_tools": ["mcp__codex_apps__calendar_list_events"],
|
||||
})
|
||||
.to_string(),
|
||||
),
|
||||
];
|
||||
|
||||
let selected = Session::extract_mcp_tool_selection_from_rollout(&rollout_items);
|
||||
assert_eq!(
|
||||
selected,
|
||||
Some(vec!["mcp__codex_apps__calendar_list_events".to_string(),])
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn extract_mcp_tool_selection_from_rollout_returns_none_without_valid_search_output() {
|
||||
let rollout_items = vec![function_call_rollout_item(
|
||||
SEARCH_TOOL_BM25_TOOL_NAME,
|
||||
"search-1",
|
||||
)];
|
||||
let selected = Session::extract_mcp_tool_selection_from_rollout(&rollout_items);
|
||||
assert_eq!(selected, None);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn reconstruct_history_matches_live_compactions() {
|
||||
let (session, turn_context) = make_session_and_context().await;
|
||||
|
|
|
|||
|
|
@ -908,6 +908,16 @@ pub(crate) fn filter_codex_apps_mcp_tools_only(
|
|||
.collect()
|
||||
}
|
||||
|
||||
pub(crate) fn filter_non_codex_apps_mcp_tools_only(
|
||||
mcp_tools: &HashMap<String, ToolInfo>,
|
||||
) -> HashMap<String, ToolInfo> {
|
||||
mcp_tools
|
||||
.iter()
|
||||
.filter(|(_, tool)| tool.server_name != CODEX_APPS_MCP_SERVER_NAME)
|
||||
.map(|(name, tool)| (name.clone(), tool.clone()))
|
||||
.collect()
|
||||
}
|
||||
|
||||
pub(crate) fn filter_mcp_tools_by_name(
|
||||
mcp_tools: &HashMap<String, ToolInfo>,
|
||||
selected_tools: &[String],
|
||||
|
|
|
|||
|
|
@ -170,6 +170,27 @@ impl SessionState {
|
|||
merged
|
||||
}
|
||||
|
||||
pub(crate) fn set_mcp_tool_selection(&mut self, tool_names: Vec<String>) {
|
||||
if tool_names.is_empty() {
|
||||
self.active_mcp_tool_selection = None;
|
||||
return;
|
||||
}
|
||||
|
||||
let mut selected = Vec::new();
|
||||
let mut seen = HashSet::new();
|
||||
for tool_name in tool_names {
|
||||
if seen.insert(tool_name.clone()) {
|
||||
selected.push(tool_name);
|
||||
}
|
||||
}
|
||||
|
||||
self.active_mcp_tool_selection = if selected.is_empty() {
|
||||
None
|
||||
} else {
|
||||
Some(selected)
|
||||
};
|
||||
}
|
||||
|
||||
pub(crate) fn get_mcp_tool_selection(&self) -> Option<Vec<String>> {
|
||||
self.active_mcp_tool_selection.clone()
|
||||
}
|
||||
|
|
@ -293,6 +314,40 @@ mod tests {
|
|||
assert_eq!(state.get_mcp_tool_selection(), None);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn set_mcp_tool_selection_deduplicates_and_preserves_order() {
|
||||
let session_configuration = make_session_configuration_for_tests().await;
|
||||
let mut state = SessionState::new(session_configuration);
|
||||
state.merge_mcp_tool_selection(vec!["mcp__rmcp__old".to_string()]);
|
||||
|
||||
state.set_mcp_tool_selection(vec![
|
||||
"mcp__rmcp__echo".to_string(),
|
||||
"mcp__rmcp__image".to_string(),
|
||||
"mcp__rmcp__echo".to_string(),
|
||||
"mcp__rmcp__search".to_string(),
|
||||
]);
|
||||
|
||||
assert_eq!(
|
||||
state.get_mcp_tool_selection(),
|
||||
Some(vec![
|
||||
"mcp__rmcp__echo".to_string(),
|
||||
"mcp__rmcp__image".to_string(),
|
||||
"mcp__rmcp__search".to_string(),
|
||||
])
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn set_mcp_tool_selection_empty_input_clears_selection() {
|
||||
let session_configuration = make_session_configuration_for_tests().await;
|
||||
let mut state = SessionState::new(session_configuration);
|
||||
state.merge_mcp_tool_selection(vec!["mcp__rmcp__echo".to_string()]);
|
||||
|
||||
state.set_mcp_tool_selection(Vec::new());
|
||||
|
||||
assert_eq!(state.get_mcp_tool_selection(), None);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
// Verifies connector merging deduplicates repeated IDs.
|
||||
async fn merge_connector_selection_deduplicates_entries() {
|
||||
|
|
|
|||
|
|
@ -120,7 +120,6 @@ impl Session {
|
|||
task: T,
|
||||
) {
|
||||
self.abort_all_tasks(TurnAbortReason::Replaced).await;
|
||||
self.clear_mcp_tool_selection().await;
|
||||
self.clear_connector_selection().await;
|
||||
self.seed_initial_context_if_needed(turn_context.as_ref())
|
||||
.await;
|
||||
|
|
|
|||
|
|
@ -33,6 +33,7 @@ pub use read_file::ReadFileHandler;
|
|||
pub use request_user_input::RequestUserInputHandler;
|
||||
pub(crate) use request_user_input::request_user_input_tool_description;
|
||||
pub(crate) use search_tool_bm25::DEFAULT_LIMIT as SEARCH_TOOL_BM25_DEFAULT_LIMIT;
|
||||
pub(crate) use search_tool_bm25::SEARCH_TOOL_BM25_TOOL_NAME;
|
||||
pub use search_tool_bm25::SearchToolBm25Handler;
|
||||
pub use shell::ShellCommandHandler;
|
||||
pub use shell::ShellHandler;
|
||||
|
|
|
|||
|
|
@ -10,7 +10,6 @@ use std::collections::HashMap;
|
|||
use std::collections::HashSet;
|
||||
|
||||
use crate::connectors;
|
||||
use crate::features::Feature;
|
||||
use crate::function_tool::FunctionCallError;
|
||||
use crate::mcp::CODEX_APPS_MCP_SERVER_NAME;
|
||||
use crate::mcp_connection_manager::ToolInfo;
|
||||
|
|
@ -23,6 +22,7 @@ use crate::tools::registry::ToolKind;
|
|||
|
||||
pub struct SearchToolBm25Handler;
|
||||
|
||||
pub(crate) const SEARCH_TOOL_BM25_TOOL_NAME: &str = "search_tool_bm25";
|
||||
pub(crate) const DEFAULT_LIMIT: usize = 8;
|
||||
|
||||
fn default_limit() -> usize {
|
||||
|
|
@ -42,7 +42,6 @@ struct ToolEntry {
|
|||
server_name: String,
|
||||
title: Option<String>,
|
||||
description: Option<String>,
|
||||
connector_id: Option<String>,
|
||||
connector_name: Option<String>,
|
||||
input_keys: Vec<String>,
|
||||
search_text: String,
|
||||
|
|
@ -66,7 +65,6 @@ impl ToolEntry {
|
|||
.tool
|
||||
.description
|
||||
.map(|description| description.to_string()),
|
||||
connector_id: info.connector_id,
|
||||
connector_name: info.connector_name,
|
||||
input_keys,
|
||||
search_text,
|
||||
|
|
@ -91,9 +89,9 @@ impl ToolHandler for SearchToolBm25Handler {
|
|||
let arguments = match payload {
|
||||
ToolPayload::Function { arguments } => arguments,
|
||||
_ => {
|
||||
return Err(FunctionCallError::Fatal(
|
||||
"search_tool_bm25 handler received unsupported payload".to_string(),
|
||||
));
|
||||
return Err(FunctionCallError::Fatal(format!(
|
||||
"{SEARCH_TOOL_BM25_TOOL_NAME} handler received unsupported payload"
|
||||
)));
|
||||
}
|
||||
};
|
||||
|
||||
|
|
@ -120,15 +118,12 @@ impl ToolHandler for SearchToolBm25Handler {
|
|||
.await
|
||||
.list_all_tools()
|
||||
.await;
|
||||
let mcp_tools = if turn.config.features.enabled(Feature::Apps) {
|
||||
let connectors = connectors::with_app_enabled_state(
|
||||
connectors::accessible_connectors_from_mcp_tools(&mcp_tools),
|
||||
&turn.config,
|
||||
);
|
||||
filter_codex_apps_mcp_tools(mcp_tools, &connectors)
|
||||
} else {
|
||||
mcp_tools
|
||||
};
|
||||
|
||||
let connectors = connectors::with_app_enabled_state(
|
||||
connectors::accessible_connectors_from_mcp_tools(&mcp_tools),
|
||||
&turn.config,
|
||||
);
|
||||
let mcp_tools = filter_codex_apps_mcp_tools(mcp_tools, &connectors);
|
||||
|
||||
let mut entries: Vec<ToolEntry> = mcp_tools
|
||||
.into_iter()
|
||||
|
|
@ -172,7 +167,6 @@ impl ToolHandler for SearchToolBm25Handler {
|
|||
"server": entry.server_name.clone(),
|
||||
"title": entry.title.clone(),
|
||||
"description": entry.description.clone(),
|
||||
"connector_id": entry.connector_id.clone(),
|
||||
"connector_name": entry.connector_name.clone(),
|
||||
"input_keys": entry.input_keys.clone(),
|
||||
"score": result.score,
|
||||
|
|
@ -243,12 +237,6 @@ fn build_search_text(name: &str, info: &ToolInfo, input_keys: &[String]) -> Stri
|
|||
parts.push(connector_name.to_string());
|
||||
}
|
||||
|
||||
if let Some(connector_id) = info.connector_id.as_deref()
|
||||
&& !connector_id.trim().is_empty()
|
||||
{
|
||||
parts.push(connector_id.to_string());
|
||||
}
|
||||
|
||||
if !input_keys.is_empty() {
|
||||
parts.extend(input_keys.iter().cloned());
|
||||
}
|
||||
|
|
|
|||
|
|
@ -8,6 +8,7 @@ use crate::features::Features;
|
|||
use crate::mcp_connection_manager::ToolInfo;
|
||||
use crate::tools::handlers::PLAN_TOOL;
|
||||
use crate::tools::handlers::SEARCH_TOOL_BM25_DEFAULT_LIMIT;
|
||||
use crate::tools::handlers::SEARCH_TOOL_BM25_TOOL_NAME;
|
||||
use crate::tools::handlers::apply_patch::create_apply_patch_freeform_tool;
|
||||
use crate::tools::handlers::apply_patch::create_apply_patch_json_tool;
|
||||
use crate::tools::handlers::collab::DEFAULT_WAIT_TIMEOUT_MS;
|
||||
|
|
@ -913,7 +914,7 @@ fn create_search_tool_bm25_tool(app_tools: &HashMap<String, ToolInfo>) -> ToolSp
|
|||
SEARCH_TOOL_BM25_DESCRIPTION_TEMPLATE.replace("{{app_names}}", app_names.as_str());
|
||||
|
||||
ToolSpec::Function(ResponsesApiTool {
|
||||
name: "search_tool_bm25".to_string(),
|
||||
name: SEARCH_TOOL_BM25_TOOL_NAME.to_string(),
|
||||
description,
|
||||
strict: false,
|
||||
parameters: JsonSchema::Object {
|
||||
|
|
@ -1507,7 +1508,7 @@ pub(crate) fn build_specs(
|
|||
&& let Some(app_tools) = app_tools
|
||||
{
|
||||
builder.push_spec_with_parallel_support(create_search_tool_bm25_tool(&app_tools), true);
|
||||
builder.register_handler("search_tool_bm25", search_tool_handler);
|
||||
builder.register_handler(SEARCH_TOOL_BM25_TOOL_NAME, search_tool_handler);
|
||||
}
|
||||
|
||||
if let Some(apply_patch_tool_type) = &config.apply_patch_tool_type {
|
||||
|
|
@ -2579,7 +2580,7 @@ mod tests {
|
|||
)
|
||||
.build();
|
||||
|
||||
let search_tool = find_tool(&tools, "search_tool_bm25");
|
||||
let search_tool = find_tool(&tools, SEARCH_TOOL_BM25_TOOL_NAME);
|
||||
let ToolSpec::Function(ResponsesApiTool { description, .. }) = &search_tool.spec else {
|
||||
panic!("expected function tool");
|
||||
};
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@
|
|||
|
||||
Searches over apps tool metadata with BM25 and exposes matching tools for the next model call.
|
||||
|
||||
When `search_tool_bm25` is available, apps tools (`mcp__codex_apps__...`) are hidden until you search for them.
|
||||
MCP tools of the apps ({{app_names}}) are hidden until you search for them with this tool (`search_tool_bm25`).
|
||||
|
||||
Follow this workflow:
|
||||
|
||||
|
|
@ -10,8 +10,8 @@ Follow this workflow:
|
|||
- `query` (required): focused terms that describe the capability you need.
|
||||
- `limit` (optional): maximum number of tools to return (default `8`).
|
||||
2. Use the returned `tools` list to decide which Apps tools are relevant.
|
||||
3. Matching tools are added to available `tools` and available for the remainder of the current turn.
|
||||
4. Repeated searches in the same turn are additive: new matches are unioned into `tools`.
|
||||
3. Matching tools are added to available `tools` and available for the remainder of the current session/thread.
|
||||
4. Repeated searches in the same session/thread are additive: new matches are unioned into `tools`.
|
||||
|
||||
Notes:
|
||||
- Core tools remain available without searching.
|
||||
|
|
@ -23,8 +23,6 @@ Notes:
|
|||
- `title`
|
||||
- `description`
|
||||
- `connector_name`
|
||||
- `connector_id`
|
||||
- input schema property keys (`input_keys`)
|
||||
- Use `search_tool_bm25` when the user asks to work with one of apps ({{app_names}}) and the exact tool name is not already known.
|
||||
- If the needed app is already explicit in the prompt (for example an `apps://...` mention) or already present in the current `tools` list, you can call that tool directly.
|
||||
- Do not use `search_tool_bm25` for non-apps/local tasks (filesystem, repo search, or shell-only workflows).
|
||||
- Do not use `search_tool_bm25` for non-apps/local tasks (filesystem, repo search, or shell-only workflows) or anything not related to {{app_names}}.
|
||||
|
|
|
|||
158
codex-rs/core/tests/common/apps_test_server.rs
Normal file
158
codex-rs/core/tests/common/apps_test_server.rs
Normal file
|
|
@ -0,0 +1,158 @@
|
|||
use anyhow::Result;
|
||||
use serde_json::Value;
|
||||
use serde_json::json;
|
||||
use wiremock::Mock;
|
||||
use wiremock::MockServer;
|
||||
use wiremock::Request;
|
||||
use wiremock::Respond;
|
||||
use wiremock::ResponseTemplate;
|
||||
use wiremock::matchers::method;
|
||||
use wiremock::matchers::path;
|
||||
use wiremock::matchers::path_regex;
|
||||
|
||||
const CONNECTOR_ID: &str = "calendar";
|
||||
const CONNECTOR_NAME: &str = "Calendar";
|
||||
const PROTOCOL_VERSION: &str = "2025-11-25";
|
||||
const SERVER_NAME: &str = "codex-apps-test";
|
||||
const SERVER_VERSION: &str = "1.0.0";
|
||||
|
||||
#[derive(Clone)]
|
||||
pub struct AppsTestServer {
|
||||
pub chatgpt_base_url: String,
|
||||
}
|
||||
|
||||
impl AppsTestServer {
|
||||
pub async fn mount(server: &MockServer) -> Result<Self> {
|
||||
mount_oauth_metadata(server).await;
|
||||
mount_streamable_http_json_rpc(server).await;
|
||||
Ok(Self {
|
||||
chatgpt_base_url: server.uri(),
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
async fn mount_oauth_metadata(server: &MockServer) {
|
||||
Mock::given(method("GET"))
|
||||
.and(path("/.well-known/oauth-authorization-server/mcp"))
|
||||
.respond_with(ResponseTemplate::new(200).set_body_json(json!({
|
||||
"authorization_endpoint": format!("{}/oauth/authorize", server.uri()),
|
||||
"token_endpoint": format!("{}/oauth/token", server.uri()),
|
||||
"scopes_supported": [""],
|
||||
})))
|
||||
.mount(server)
|
||||
.await;
|
||||
}
|
||||
|
||||
async fn mount_streamable_http_json_rpc(server: &MockServer) {
|
||||
Mock::given(method("POST"))
|
||||
.and(path_regex("^/api/codex/apps/?$"))
|
||||
.respond_with(CodexAppsJsonRpcResponder)
|
||||
.mount(server)
|
||||
.await;
|
||||
}
|
||||
|
||||
struct CodexAppsJsonRpcResponder;
|
||||
|
||||
impl Respond for CodexAppsJsonRpcResponder {
|
||||
fn respond(&self, request: &Request) -> ResponseTemplate {
|
||||
let body: Value = match serde_json::from_slice(&request.body) {
|
||||
Ok(body) => body,
|
||||
Err(error) => {
|
||||
return ResponseTemplate::new(400).set_body_json(json!({
|
||||
"error": format!("invalid JSON-RPC body: {error}"),
|
||||
}));
|
||||
}
|
||||
};
|
||||
|
||||
let Some(method) = body.get("method").and_then(Value::as_str) else {
|
||||
return ResponseTemplate::new(400).set_body_json(json!({
|
||||
"error": "missing method in JSON-RPC request",
|
||||
}));
|
||||
};
|
||||
|
||||
match method {
|
||||
"initialize" => {
|
||||
let id = body.get("id").cloned().unwrap_or(Value::Null);
|
||||
let protocol_version = body
|
||||
.pointer("/params/protocolVersion")
|
||||
.and_then(Value::as_str)
|
||||
.unwrap_or(PROTOCOL_VERSION);
|
||||
ResponseTemplate::new(200).set_body_json(json!({
|
||||
"jsonrpc": "2.0",
|
||||
"id": id,
|
||||
"result": {
|
||||
"protocolVersion": protocol_version,
|
||||
"capabilities": {
|
||||
"tools": {
|
||||
"listChanged": true
|
||||
}
|
||||
},
|
||||
"serverInfo": {
|
||||
"name": SERVER_NAME,
|
||||
"version": SERVER_VERSION
|
||||
}
|
||||
}
|
||||
}))
|
||||
}
|
||||
"notifications/initialized" => ResponseTemplate::new(202),
|
||||
"tools/list" => {
|
||||
let id = body.get("id").cloned().unwrap_or(Value::Null);
|
||||
ResponseTemplate::new(200).set_body_json(json!({
|
||||
"jsonrpc": "2.0",
|
||||
"id": id,
|
||||
"result": {
|
||||
"tools": [
|
||||
{
|
||||
"name": "calendar_create_event",
|
||||
"description": "Create a calendar event.",
|
||||
"inputSchema": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"title": { "type": "string" },
|
||||
"starts_at": { "type": "string" },
|
||||
"timezone": { "type": "string" }
|
||||
},
|
||||
"required": ["title", "starts_at"],
|
||||
"additionalProperties": false
|
||||
},
|
||||
"_meta": {
|
||||
"connector_id": CONNECTOR_ID,
|
||||
"connector_name": CONNECTOR_NAME
|
||||
}
|
||||
},
|
||||
{
|
||||
"name": "calendar_list_events",
|
||||
"description": "List calendar events.",
|
||||
"inputSchema": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"query": { "type": "string" },
|
||||
"limit": { "type": "integer" }
|
||||
},
|
||||
"additionalProperties": false
|
||||
},
|
||||
"_meta": {
|
||||
"connector_id": CONNECTOR_ID,
|
||||
"connector_name": CONNECTOR_NAME
|
||||
}
|
||||
}
|
||||
],
|
||||
"nextCursor": null
|
||||
}
|
||||
}))
|
||||
}
|
||||
method if method.starts_with("notifications/") => ResponseTemplate::new(202),
|
||||
_ => {
|
||||
let id = body.get("id").cloned().unwrap_or(Value::Null);
|
||||
ResponseTemplate::new(200).set_body_json(json!({
|
||||
"jsonrpc": "2.0",
|
||||
"id": id,
|
||||
"error": {
|
||||
"code": -32601,
|
||||
"message": format!("method not found: {method}")
|
||||
}
|
||||
}))
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -12,6 +12,7 @@ use codex_utils_absolute_path::AbsolutePathBuf;
|
|||
use regex_lite::Regex;
|
||||
use std::path::PathBuf;
|
||||
|
||||
pub mod apps_test_server;
|
||||
pub mod context_snapshot;
|
||||
pub mod process;
|
||||
pub mod responses;
|
||||
|
|
|
|||
|
|
@ -34,6 +34,7 @@ use codex_protocol::models::ReasoningItemReasoningSummary;
|
|||
use codex_protocol::models::WebSearchAction;
|
||||
use codex_protocol::openai_models::ReasoningEffort;
|
||||
use codex_protocol::user_input::UserInput;
|
||||
use core_test_support::apps_test_server::AppsTestServer;
|
||||
use core_test_support::load_default_config_for_test;
|
||||
use core_test_support::responses::ev_completed;
|
||||
use core_test_support::responses::ev_completed_with_tokens;
|
||||
|
|
@ -671,6 +672,10 @@ async fn includes_user_instructions_message_in_request() {
|
|||
async fn includes_apps_guidance_as_developer_message_when_enabled() {
|
||||
skip_if_no_network!();
|
||||
let server = MockServer::start().await;
|
||||
let apps_server = AppsTestServer::mount(&server)
|
||||
.await
|
||||
.expect("mount apps MCP mock");
|
||||
let apps_base_url = apps_server.chatgpt_base_url.clone();
|
||||
|
||||
let resp_mock = mount_sse_once(
|
||||
&server,
|
||||
|
|
@ -680,8 +685,10 @@ async fn includes_apps_guidance_as_developer_message_when_enabled() {
|
|||
|
||||
let mut builder = test_codex()
|
||||
.with_auth(CodexAuth::from_api_key("Test API Key"))
|
||||
.with_config(|config| {
|
||||
.with_config(move |config| {
|
||||
config.features.enable(Feature::Apps);
|
||||
config.features.disable(Feature::AppsMcpGateway);
|
||||
config.chatgpt_base_url = apps_base_url;
|
||||
});
|
||||
let codex = builder
|
||||
.build(&server)
|
||||
|
|
|
|||
File diff suppressed because it is too large
Load diff
Loading…
Add table
Reference in a new issue