openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-18 17:18:52 +00:00
Code Issues Packages Projects Releases Wiki Activity

Make Apps guidance react to MCP availability (#30226)

Browse Source 
## Why

Generic Apps guidance is emitted only while building static initial
context. If the Apps MCP is unavailable then and recovers later in the
same turn, its tools can become usable without the model receiving the
guidance for using them.

## What

- move generic Apps guidance into a persisted `apps_instructions` World
State section
- derive availability from the request's MCP runtime while preserving
the existing feature, auth, orchestrator-MCP, and config gates
- recognize legacy and retained Apps fragments so resume and compaction
do not duplicate guidance
- register Apps guidance as rollback-trimmable context
- remove the old static injection path and its now-unused connector
helper
- keep tool construction on its existing independent `list_all_tools()`
read rather than adding a request-wide cache; a reconnect between the
two reads can differ for one request and reconciles on the next request

Apps and plugin guidance now render after the remaining static
host-skills block, with Apps before Plugins.

## Testing

- `just fmt`
- `just test -p codex-core apps_instructions`
- `just test -p codex-core
apps_guidance_appears_after_background_recovery_within_a_turn`
- `just test -p codex-core
drop_last_n_user_turns_trims_context_updates_above_rolled_back_turn`
This commit is contained in:
sayan-oai
2026-07-06 14:29:19 -07:00
committed by GitHub
parent c976741124
commit 3c2bfe7c5d
12 changed files with 293 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/core/src/apps/render.rs
+4 -1
View File
@@ -5,7 +5,10 @@ use codex_protocol::protocol::APPS_INSTRUCTIONS_CLOSE_TAG;
use codex_protocol::protocol::APPS_INSTRUCTIONS_OPEN_TAG;
pub(crate) fn render_apps_section(connectors: &[AppInfo]) -> Option<String> {
AppsInstructions::from_connectors(connectors).map(|instructions| instructions.render())
connectors
.iter()
.any(|connector| connector.is_accessible && connector.is_enabled)
.then(|| AppsInstructions.render())
}
#[cfg(test)]
codex-rs/core/src/connectors.rs
-13
View File
@@ -80,19 +80,6 @@ pub async fn list_accessible_connectors_from_mcp_tools(
)
}
pub(crate) async fn list_accessible_and_enabled_connectors_from_manager(
mcp_connection_manager: &McpConnectionManager,
config: &Config,
) -> Vec<AppInfo> {
with_app_enabled_state(
accessible_connectors_from_mcp_tools(&mcp_connection_manager.list_all_tools().await),
config,
)
.into_iter()
.filter(|connector| connector.is_accessible && connector.is_enabled)
.collect()
}
#[instrument(level = "trace", skip_all)]
pub(crate) async fn list_tool_suggest_discoverable_tools_with_auth(
config: &Config,
codex-rs/core/src/context/apps_instructions.rs
-10
View File
@@ -1,4 +1,3 @@
use crate::connectors::AppInfo;
use codex_mcp::CODEX_APPS_MCP_SERVER_NAME;
use codex_protocol::protocol::APPS_INSTRUCTIONS_CLOSE_TAG;
use codex_protocol::protocol::APPS_INSTRUCTIONS_OPEN_TAG;
@@ -8,15 +7,6 @@ use super::ContextualUserFragment;
#[derive(Debug, Clone, PartialEq)]
pub(crate) struct AppsInstructions;
impl AppsInstructions {
pub(crate) fn from_connectors(connectors: &[AppInfo]) -> Option<Self> {
connectors
.iter()
.any(|connector| connector.is_accessible && connector.is_enabled)
.then_some(Self)
}
}
impl ContextualUserFragment for AppsInstructions {
fn role(&self) -> &'static str {
"developer"
codex-rs/core/src/context/world_state/apps_instructions.rs
+55
View File
@@ -0,0 +1,55 @@
use super::PreviousSectionState;
use super::WorldStateSection;
use crate::context::AppsInstructions;
use crate::context::ContextualUserFragment;
/// Whether generic Apps usage guidance should be visible to the model.
#[derive(Clone, Copy, Debug, Default)]
pub(crate) struct AppsInstructionsState {
available: bool,
}
impl AppsInstructionsState {
pub(crate) fn new(available: bool) -> Self {
Self { available }
}
}
impl WorldStateSection for AppsInstructionsState {
const ID: &'static str = "apps_instructions";
type Snapshot = bool;
fn snapshot(&self) -> Self::Snapshot {
self.available
}
fn matches_legacy_fragment(role: &str, text: &str) -> bool {
role == "developer" && AppsInstructions::matches_text(text)
}
fn has_retained_fragment_matcher() -> bool {
true
}
fn matches_retained_fragment(role: &str, text: &str) -> bool {
Self::matches_legacy_fragment(role, text)
}
fn render_diff(
&self,
previous: PreviousSectionState<'_, Self::Snapshot>,
) -> Option<Box<dyn ContextualUserFragment>> {
if !self.available
|| matches!(previous, PreviousSectionState::Known(previous) if *previous)
|| matches!(previous, PreviousSectionState::Unknown)
{
return None;
}
Some(Box::new(AppsInstructions))
}
}
#[cfg(test)]
#[path = "apps_instructions_tests.rs"]
mod tests;
codex-rs/core/src/context/world_state/apps_instructions_tests.rs
+69
View File
@@ -0,0 +1,69 @@
use super::*;
use crate::context::ContextualUserFragment;
use codex_protocol::models::ResponseItem;
use pretty_assertions::assert_eq;
fn render(state: AppsInstructionsState, previous: PreviousSectionState<'_, bool>) -> Vec<String> {
state
.render_diff(previous)
.into_iter()
.map(|fragment| fragment.render())
.collect()
}
#[test]
fn renders_only_when_apps_become_available() {
let unavailable = AppsInstructionsState::new(/*available*/ false);
let available = AppsInstructionsState::new(/*available*/ true);
let false_snapshot = false;
let true_snapshot = true;
assert_eq!(
render(unavailable, PreviousSectionState::Absent),
Vec::<String>::new()
);
assert_eq!(render(available, PreviousSectionState::Absent).len(), 1);
assert_eq!(
render(available, PreviousSectionState::Known(&false_snapshot)).len(),
1
);
assert_eq!(
render(available, PreviousSectionState::Known(&true_snapshot)),
Vec::<String>::new()
);
assert_eq!(
render(unavailable, PreviousSectionState::Known(&true_snapshot)),
Vec::<String>::new()
);
}
#[test]
fn legacy_guidance_is_not_injected_again() {
let mut world_state = super::super::WorldState::default();
world_state.add_section(AppsInstructionsState::new(/*available*/ true));
let legacy: ResponseItem = ContextualUserFragment::into(AppsInstructions);
assert!(
world_state
.render_history_diff(/*previous*/ None, &[legacy])
.is_empty()
);
}
#[test]
fn persisted_guidance_is_restored_only_when_missing_from_history() {
let mut world_state = super::super::WorldState::default();
world_state.add_section(AppsInstructionsState::new(/*available*/ true));
let snapshot = world_state.snapshot();
let retained: ResponseItem = ContextualUserFragment::into(AppsInstructions);
assert_eq!(
world_state.render_history_diff(Some(&snapshot), &[]).len(),
1
);
assert!(
world_state
.render_history_diff(Some(&snapshot), &[retained])
.is_empty()
);
}
codex-rs/core/src/context/world_state/mod.rs
+2
View File
@@ -1,4 +1,5 @@
mod agents_md;
mod apps_instructions;
mod environment;
mod plugins_instructions;
@@ -17,6 +18,7 @@ use std::collections::BTreeMap;
use std::fmt;
pub(crate) use agents_md::AgentsMdState;
pub(crate) use apps_instructions::AppsInstructionsState;
pub(crate) use environment::EnvironmentsState;
pub(crate) use plugins_instructions::PluginsInstructionsState;
codex-rs/core/src/context_manager/history_tests.rs
+4
View File
@@ -20,6 +20,7 @@ use codex_protocol::models::ReasoningItemContent;
use codex_protocol::models::ReasoningItemReasoningSummary;
use codex_protocol::openai_models::InputModality;
use codex_protocol::openai_models::default_input_modalities;
use codex_protocol::protocol::APPS_INSTRUCTIONS_OPEN_TAG;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::InterAgentCommunication;
use codex_protocol::protocol::PLUGINS_INSTRUCTIONS_OPEN_TAG;
@@ -1022,6 +1023,9 @@ fn drop_last_n_user_turns_trims_context_updates_above_rolled_back_turn() {
user_input_text_msg("turn 1 user"),
assistant_msg("turn 1 assistant"),
developer_msg("Generated images are saved to /tmp as /tmp/image-1.png by default."),
developer_msg(&format!(
"{APPS_INSTRUCTIONS_OPEN_TAG}\nROLLED_BACK_APPS_INSTRUCTIONS"
)),
developer_msg(&format!(
"{PLUGINS_INSTRUCTIONS_OPEN_TAG}\nROLLED_BACK_PLUGIN_INSTRUCTIONS"
)),
codex-rs/core/src/event_mapping.rs
+2
View File
@@ -14,6 +14,7 @@ use codex_protocol::models::is_image_close_tag_text;
use codex_protocol::models::is_image_open_tag_text;
use codex_protocol::models::is_local_image_close_tag_text;
use codex_protocol::models::is_local_image_open_tag_text;
use codex_protocol::protocol::APPS_INSTRUCTIONS_OPEN_TAG;
use codex_protocol::protocol::COLLABORATION_MODE_OPEN_TAG;
use codex_protocol::protocol::CONTEXT_WINDOW_GUIDANCE_OPEN_TAG;
use codex_protocol::protocol::CONTEXT_WINDOW_OPEN_TAG;
@@ -32,6 +33,7 @@ use crate::web_search::web_search_action_detail;
const CONTEXTUAL_DEVELOPER_PREFIXES: &[&str] = &[
"<permissions instructions>",
"<model_switch>",
APPS_INSTRUCTIONS_OPEN_TAG,
COLLABORATION_MODE_OPEN_TAG,
MULTI_AGENT_MODE_OPEN_TAG,
PLUGINS_INSTRUCTIONS_OPEN_TAG,
codex-rs/core/src/session/mod.rs
-15
View File
@@ -21,9 +21,7 @@ use crate::build_available_skills;
use crate::compact;
use crate::config::ManagedFeatures;
use crate::config::resolve_tool_suggest_config_from_layer_stack;
use crate::connectors;
use crate::context::ApprovedCommandPrefixSaved;
use crate::context::AppsInstructions;
use crate::context::AvailableSkillsInstructions;
use crate::context::CollaborationModeInstructions;
use crate::context::ContextualUserFragment;
@@ -3235,19 +3233,6 @@ impl Session {
.push(PersonalitySpecInstructions::new(personality_message).render());
}
}
if turn_context.config.include_apps_instructions && turn_context.apps_enabled() {
let accessible_and_enabled_connectors =
connectors::list_accessible_and_enabled_connectors_from_manager(
mcp.manager(),
&turn_context.config,
)
.await;
if let Some(apps_instructions) =
AppsInstructions::from_connectors(&accessible_and_enabled_connectors)
{
developer_sections.push(apps_instructions.render());
}
}
if turn_context.config.include_skill_instructions {
let available_skills = build_available_skills(
turn_context.turn_skills.snapshot.outcome(),
codex-rs/core/src/session/world_state.rs
+15
View File
@@ -1,6 +1,8 @@
use super::session::Session;
use super::step_context::StepContext;
use crate::connectors;
use crate::context::world_state::AgentsMdState;
use crate::context::world_state::AppsInstructionsState;
use crate::context::world_state::EnvironmentsState;
use crate::context::world_state::PluginsInstructionsState;
use crate::context::world_state::WorldState;
@@ -37,6 +39,19 @@ impl Session {
.with_subagents(environment_subagents),
);
}
let apps_available =
if turn_context.config.include_apps_instructions && turn_context.apps_enabled() {
let tools = step_context.mcp.manager().list_all_tools().await;
connectors::with_app_enabled_state(
connectors::accessible_connectors_from_mcp_tools(&tools),
&turn_context.config,
)
.into_iter()
.any(|connector| connector.is_accessible && connector.is_enabled)
} else {
false
};
world_state.add_section(AppsInstructionsState::new(apps_available));
world_state.add_section(PluginsInstructionsState::new(
step_context.mcp.plugins_available(),
));
codex-rs/core/tests/suite/mcp_tool_exposure.rs
+140
View File
@@ -1,22 +1,32 @@
use anyhow::Result;
use codex_features::Feature;
use codex_mcp::CODEX_APPS_MCP_SERVER_NAME;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::McpServerRefreshConfig;
use codex_protocol::protocol::Op;
use codex_protocol::request_user_input::RequestUserInputAnswer;
use codex_protocol::request_user_input::RequestUserInputResponse;
use codex_protocol::user_input::UserInput;
use core_test_support::apps_test_server::AppsTestServer;
use core_test_support::apps_test_server::SEARCH_CALENDAR_CREATE_TOOL;
use core_test_support::apps_test_server::SEARCH_CALENDAR_NAMESPACE;
use core_test_support::apps_test_server::apps_enabled_builder;
use core_test_support::apps_test_server::search_capable_apps_builder;
use core_test_support::responses;
use core_test_support::responses::ev_assistant_message;
use core_test_support::responses::ev_completed;
use core_test_support::responses::ev_function_call;
use core_test_support::responses::ev_response_created;
use core_test_support::responses::mount_sse_sequence;
use core_test_support::responses::namespace_child_tool;
use core_test_support::responses::sse;
use core_test_support::skip_if_no_network;
use core_test_support::wait_for_event;
use core_test_support::wait_for_event_match;
use core_test_support::wait_for_mcp_server;
use serde_json::Value;
use serde_json::json;
use std::collections::HashMap;
use std::time::Duration;
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
@@ -86,6 +96,136 @@ async fn code_mode_only_exposes_direct_model_only_mcp_namespaces() -> Result<()>
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn apps_guidance_appears_after_background_recovery_within_a_turn() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = responses::start_mock_server().await;
let (apps_server, startup_control) =
AppsTestServer::mount_with_startup_control(&server).await?;
// Initial context is rendered twice before the first request, so keep both reads unavailable.
startup_control.fail_next_initialize_attempts(/*attempts*/ 2);
let call_id = "pause-for-apps";
let response = mount_sse_sequence(
&server,
vec![
sse(vec![
ev_response_created("resp-1"),
ev_function_call(
call_id,
"request_user_input",
&json!({
"questions": [{
"id": "continue",
"header": "Continue",
"question": "Continue after Apps recovers?",
"options": [{
"label": "Yes (Recommended)",
"description": "Continue the test."
}, {
"label": "No",
"description": "Stop the test."
}]
}]
})
.to_string(),
),
ev_completed("resp-1"),
]),
sse(vec![
ev_response_created("resp-2"),
ev_assistant_message("msg-2", "done"),
ev_completed("resp-2"),
]),
],
)
.await;
let mut builder =
apps_enabled_builder(apps_server.chatgpt_base_url.clone()).with_config(|config| {
config
.features
.enable(Feature::DeferredExecutor)
.expect("test config should allow feature update");
config
.features
.enable(Feature::DefaultModeRequestUserInput)
.expect("test config should allow feature update");
});
let test = builder.build(&server).await?;
let mcp_runtime = test.codex.current_mcp_runtime().await;
test.codex
.submit(Op::UserInput {
items: vec![UserInput::Text {
text: "use an app after it recovers".into(),
text_elements: Vec::new(),
}],
final_output_json_schema: None,
responsesapi_client_metadata: None,
additional_context: Default::default(),
thread_settings: Default::default(),
})
.await?;
let request = wait_for_event_match(&test.codex, |event| match event {
EventMsg::RequestUserInput(request) => Some(request.clone()),
_ => None,
})
.await;
let initial_requests = response.requests();
assert_eq!(initial_requests.len(), 1);
let initial_request = &initial_requests[0];
assert_eq!(
initial_request
.message_input_texts("developer")
.iter()
.filter(|text| text.contains("<apps_instructions>"))
.count(),
0
);
tokio::time::timeout(Duration::from_secs(3), async {
loop {
if !mcp_runtime.manager().list_all_tools().await.is_empty() {
break;
}
tokio::time::sleep(Duration::from_millis(10)).await;
}
})
.await
.expect("Apps MCP should recover while the turn is paused");
test.codex
.submit(Op::UserInputAnswer {
id: request.turn_id,
response: RequestUserInputResponse {
answers: HashMap::from([(
"continue".to_string(),
RequestUserInputAnswer {
answers: vec!["Yes (Recommended)".to_string()],
},
)]),
},
})
.await?;
wait_for_event(&test.codex, |event| {
matches!(event, EventMsg::TurnComplete(_))
})
.await;
let requests = response.requests();
assert_eq!(requests.len(), 2);
assert_eq!(
requests[1]
.message_input_texts("developer")
.iter()
.filter(|text| text.contains("<apps_instructions>"))
.count(),
1
);
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn later_follow_up_uses_background_recovered_apps_after_mid_thread_startup_failures()
-> Result<()> {
codex-rs/core/tests/suite/plugins.rs
+2 -2
View File
@@ -255,8 +255,8 @@ async fn capability_sections_render_in_developer_message_in_order() -> Result<()
.find("## Plugins")
.expect("expected plugins section in developer message");
assert!(
apps_pos < skills_pos && skills_pos < plugins_pos,
"expected Apps -> Skills -> Plugins order: {developer_messages:?}"
skills_pos < apps_pos && apps_pos < plugins_pos,
"expected Skills -> Apps -> Plugins order: {developer_messages:?}"
);
assert!(
!developer_text.contains("`sample`: inspect sample data"),