mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[codex] [3/4] Activate endpoint plugin recommendations (#27704)
Summary\n- Await endpoint recommendation selection while constructing each authenticated turn, removing the first-turn cache race.\n- Snapshot and filter endpoint candidates once per turn, then use that same set for the bounded contextual user fragment, tool exposure, and exact install validation.\n- Keep recommendation selection ephemeral: do not persist recommendation state in or gate resumed threads on prior context.\n- Hide the legacy list tool in endpoint mode and preserve legacy discovery unchanged when the endpoint is disabled or unavailable.\n- Keep remote plugin and connector app identities out of model-visible context and attach them only to Codex-owned elicitation metadata.\n\nStack\n- 3/4, based on #28400.\n- Endpoint client and cache: #28399.\n- Generalized suggestion presentation: #28400.\n- Install-schema follow-up: #28403.\n\nValidation\n- \n- \n- \n- \n- Full : 2,649 passed and 88 environment-dependent tests failed because this sandbox cannot write , nest Seatbelt, or locate auxiliary test binaries.
This commit is contained in:
@@ -10,6 +10,7 @@ use super::InternalModelContextFragment;
|
||||
use super::LegacyApplyPatchExecCommandWarning;
|
||||
use super::LegacyModelMismatchWarning;
|
||||
use super::LegacyUnifiedExecProcessLimitWarning;
|
||||
use super::RecommendedPluginsInstructions;
|
||||
use super::SkillInstructions;
|
||||
use super::SubagentNotification;
|
||||
use super::TurnAborted;
|
||||
@@ -33,6 +34,8 @@ static SUBAGENT_NOTIFICATION_REGISTRATION: FragmentRegistrationProxy<SubagentNot
|
||||
static INTERNAL_MODEL_CONTEXT_REGISTRATION: FragmentRegistrationProxy<
|
||||
InternalModelContextFragment,
|
||||
> = FragmentRegistrationProxy::new();
|
||||
static RECOMMENDED_PLUGINS_REGISTRATION: FragmentRegistrationProxy<RecommendedPluginsInstructions> =
|
||||
FragmentRegistrationProxy::new();
|
||||
static LEGACY_UNIFIED_EXEC_PROCESS_LIMIT_WARNING_REGISTRATION: FragmentRegistrationProxy<
|
||||
LegacyUnifiedExecProcessLimitWarning,
|
||||
> = FragmentRegistrationProxy::new();
|
||||
@@ -52,6 +55,7 @@ static CONTEXTUAL_USER_FRAGMENTS: &[&dyn FragmentRegistration] = &[
|
||||
&TURN_ABORTED_REGISTRATION,
|
||||
&SUBAGENT_NOTIFICATION_REGISTRATION,
|
||||
&INTERNAL_MODEL_CONTEXT_REGISTRATION,
|
||||
&RECOMMENDED_PLUGINS_REGISTRATION,
|
||||
&LEGACY_UNIFIED_EXEC_PROCESS_LIMIT_WARNING_REGISTRATION,
|
||||
&LEGACY_APPLY_PATCH_EXEC_COMMAND_WARNING_REGISTRATION,
|
||||
&LEGACY_MODEL_MISMATCH_WARNING_REGISTRATION,
|
||||
|
||||
@@ -75,6 +75,14 @@ fn detects_internal_model_context_fragment() {
|
||||
}));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn detects_recommended_plugins_fragment() {
|
||||
assert!(is_contextual_user_fragment(&ContentItem::InputText {
|
||||
text: "<recommended_plugins>\n- Google Drive (google-drive@openai-curated-remote)\n</recommended_plugins>"
|
||||
.to_string(),
|
||||
}));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn detects_legacy_goal_context_fragment() {
|
||||
assert!(is_contextual_user_fragment(&ContentItem::InputText {
|
||||
|
||||
@@ -23,6 +23,7 @@ mod plugin_instructions;
|
||||
mod realtime_end_instructions;
|
||||
mod realtime_start_instructions;
|
||||
mod realtime_start_with_instructions;
|
||||
mod recommended_plugins_instructions;
|
||||
mod subagent_notification;
|
||||
mod token_budget_context;
|
||||
mod turn_aborted;
|
||||
@@ -62,6 +63,7 @@ pub(crate) use plugin_instructions::PluginInstructions;
|
||||
pub(crate) use realtime_end_instructions::RealtimeEndInstructions;
|
||||
pub(crate) use realtime_start_instructions::RealtimeStartInstructions;
|
||||
pub(crate) use realtime_start_with_instructions::RealtimeStartWithInstructions;
|
||||
pub(crate) use recommended_plugins_instructions::RecommendedPluginsInstructions;
|
||||
pub(crate) use subagent_notification::SubagentNotification;
|
||||
pub(crate) use token_budget_context::TokenBudgetContext;
|
||||
pub(crate) use token_budget_context::TokenBudgetRemainingContext;
|
||||
|
||||
@@ -0,0 +1,49 @@
|
||||
use super::ContextualUserFragment;
|
||||
use codex_tools::DiscoverableTool;
|
||||
|
||||
const RECOMMENDED_PLUGINS_INTRO: &str = "Here is a list of plugins that are available but not installed. If the user's query would benefit from one of these plugins, use the `request_plugin_install` tool to suggest that they install it. All entries have `tool_type: plugin`; pass `plugin` as `tool_type` and the parenthesized ID as `tool_id`. For example, suggest the Google Drive plugin if the query could possibly be better answered with access to Google Drive.";
|
||||
const MAX_RECOMMENDED_PLUGINS: usize = 50;
|
||||
|
||||
#[derive(Debug, Clone, PartialEq)]
|
||||
pub(crate) struct RecommendedPluginsInstructions {
|
||||
plugins: Vec<DiscoverableTool>,
|
||||
}
|
||||
|
||||
impl RecommendedPluginsInstructions {
|
||||
pub(crate) fn from_plugins(plugins: &[DiscoverableTool]) -> Option<Self> {
|
||||
if plugins.is_empty() {
|
||||
return None;
|
||||
}
|
||||
Some(Self {
|
||||
plugins: plugins
|
||||
.iter()
|
||||
.take(MAX_RECOMMENDED_PLUGINS)
|
||||
.cloned()
|
||||
.collect(),
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
impl ContextualUserFragment for RecommendedPluginsInstructions {
|
||||
fn role(&self) -> &'static str {
|
||||
"user"
|
||||
}
|
||||
|
||||
fn markers(&self) -> (&'static str, &'static str) {
|
||||
Self::type_markers()
|
||||
}
|
||||
|
||||
fn type_markers() -> (&'static str, &'static str) {
|
||||
("<recommended_plugins>", "</recommended_plugins>")
|
||||
}
|
||||
|
||||
fn body(&self) -> String {
|
||||
let plugins = self
|
||||
.plugins
|
||||
.iter()
|
||||
.map(|plugin| format!("- {} ({})", plugin.name(), plugin.id()))
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n");
|
||||
format!("\n{RECOMMENDED_PLUGINS_INTRO}\n\n{plugins}\n")
|
||||
}
|
||||
}
|
||||
@@ -30,6 +30,7 @@ use crate::context::ContextualUserFragment;
|
||||
use crate::context::NetworkRuleSaved;
|
||||
use crate::context::PermissionsInstructions;
|
||||
use crate::context::PersonalitySpecInstructions;
|
||||
use crate::context::RecommendedPluginsInstructions;
|
||||
use crate::default_skill_metadata_budget;
|
||||
use crate::environment_selection::TurnEnvironmentSnapshot;
|
||||
use crate::exec_policy::ExecPolicyManager;
|
||||
@@ -325,6 +326,7 @@ use crate::turn_timing::record_turn_ttfm_metric;
|
||||
use crate::unified_exec::UnifiedExecProcessManager;
|
||||
use crate::windows_sandbox::WindowsSandboxLevelExt;
|
||||
use codex_core_plugins::PluginsManager;
|
||||
use codex_core_plugins::RecommendedPluginCandidatesInput;
|
||||
use codex_git_utils::get_git_repo_root;
|
||||
use codex_mcp::McpConfig;
|
||||
use codex_mcp::compute_auth_statuses;
|
||||
@@ -2975,6 +2977,29 @@ impl Session {
|
||||
.plugins_manager
|
||||
.plugins_for_config(&turn_context.config.plugins_config_input())
|
||||
.await;
|
||||
let recommended_plugin_candidates =
|
||||
if crate::tools::spec_plan::tool_suggest_enabled(turn_context) {
|
||||
let auth = self.services.auth_manager.auth().await;
|
||||
let plugins_config = turn_context.config.plugins_config_input();
|
||||
self.services
|
||||
.plugins_manager
|
||||
.recommended_plugin_candidates_for_config(RecommendedPluginCandidatesInput {
|
||||
plugins_config: &plugins_config,
|
||||
loaded_plugins: &loaded_plugins,
|
||||
auth: auth.as_ref(),
|
||||
disabled_tools: &turn_context.config.tool_suggest.disabled_tools,
|
||||
app_server_client_name: turn_context.app_server_client_name.as_deref(),
|
||||
})
|
||||
.await
|
||||
} else {
|
||||
None
|
||||
};
|
||||
if let Some(recommended_plugins) = recommended_plugin_candidates
|
||||
.as_deref()
|
||||
.and_then(RecommendedPluginsInstructions::from_plugins)
|
||||
{
|
||||
contextual_user_sections.push(recommended_plugins.render());
|
||||
}
|
||||
if let Some(plugin_instructions) =
|
||||
AvailablePluginsInstructions::from_plugins(loaded_plugins.capability_summaries())
|
||||
{
|
||||
|
||||
@@ -73,6 +73,7 @@ use codex_analytics::InvocationType;
|
||||
use codex_analytics::TurnResolvedConfigFact;
|
||||
use codex_analytics::build_track_events_context;
|
||||
use codex_async_utils::OrCancelExt;
|
||||
use codex_core_plugins::RecommendedPluginCandidatesInput;
|
||||
use codex_core_skills::injection::InjectedHostSkillPrompts;
|
||||
use codex_extension_api::TurnInputContext;
|
||||
use codex_extension_api::TurnInputEnvironment;
|
||||
@@ -1186,49 +1187,78 @@ pub(crate) async fn built_tools(
|
||||
} else {
|
||||
None
|
||||
};
|
||||
let auth = sess.services.auth_manager.auth().await;
|
||||
let loaded_plugin_app_connector_ids = loaded_plugins
|
||||
.effective_apps()
|
||||
.into_iter()
|
||||
.map(|connector_id| connector_id.0)
|
||||
.collect::<Vec<_>>();
|
||||
let tool_suggest_candidates = async {
|
||||
if apps_enabled && tool_suggest_enabled(turn_context) {
|
||||
if let Some(accessible_connectors) = accessible_connectors_with_enabled_state.as_ref() {
|
||||
match connectors::list_tool_suggest_discoverable_tools_with_auth(
|
||||
&turn_context.config,
|
||||
sess.services.plugins_manager.as_ref(),
|
||||
auth.as_ref(),
|
||||
accessible_connectors.as_slice(),
|
||||
&loaded_plugin_app_connector_ids,
|
||||
)
|
||||
.await
|
||||
.map(|discoverable_tools| {
|
||||
filter_request_plugin_install_discoverable_tools_for_client(
|
||||
discoverable_tools,
|
||||
turn_context.app_server_client_name.as_deref(),
|
||||
)
|
||||
}) {
|
||||
Ok(discoverable_tools) if discoverable_tools.is_empty() => None,
|
||||
Ok(discoverable_tools) => Some(ToolSuggestCandidates {
|
||||
tools: discoverable_tools,
|
||||
presentation: ToolSuggestPresentation::ListTool,
|
||||
}),
|
||||
Err(err) => {
|
||||
warn!("failed to load discoverable tool suggestions: {err:#}");
|
||||
let tool_suggest_is_enabled = tool_suggest_enabled(turn_context);
|
||||
let auth = if tool_suggest_is_enabled {
|
||||
sess.services.auth_manager.auth().await
|
||||
} else {
|
||||
None
|
||||
};
|
||||
let endpoint_recommended_plugin_candidates = if tool_suggest_is_enabled {
|
||||
let plugins_config = turn_context.config.plugins_config_input();
|
||||
sess.services
|
||||
.plugins_manager
|
||||
.recommended_plugin_candidates_for_config(RecommendedPluginCandidatesInput {
|
||||
plugins_config: &plugins_config,
|
||||
loaded_plugins: &loaded_plugins,
|
||||
auth: auth.as_ref(),
|
||||
disabled_tools: &turn_context.config.tool_suggest.disabled_tools,
|
||||
app_server_client_name: turn_context.app_server_client_name.as_deref(),
|
||||
})
|
||||
.await
|
||||
} else {
|
||||
None
|
||||
};
|
||||
let tool_suggest_candidates =
|
||||
if let Some(recommended_plugin_candidates) = endpoint_recommended_plugin_candidates {
|
||||
Some(ToolSuggestCandidates {
|
||||
tools: recommended_plugin_candidates,
|
||||
presentation: ToolSuggestPresentation::RecommendationContext,
|
||||
})
|
||||
} else {
|
||||
let loaded_plugin_app_connector_ids = loaded_plugins
|
||||
.effective_apps()
|
||||
.into_iter()
|
||||
.map(|connector_id| connector_id.0)
|
||||
.collect::<Vec<_>>();
|
||||
async {
|
||||
if apps_enabled && tool_suggest_is_enabled {
|
||||
if let Some(accessible_connectors) =
|
||||
accessible_connectors_with_enabled_state.as_ref()
|
||||
{
|
||||
match connectors::list_tool_suggest_discoverable_tools_with_auth(
|
||||
&turn_context.config,
|
||||
sess.services.plugins_manager.as_ref(),
|
||||
auth.as_ref(),
|
||||
accessible_connectors.as_slice(),
|
||||
&loaded_plugin_app_connector_ids,
|
||||
)
|
||||
.await
|
||||
.map(|discoverable_tools| {
|
||||
filter_request_plugin_install_discoverable_tools_for_client(
|
||||
discoverable_tools,
|
||||
turn_context.app_server_client_name.as_deref(),
|
||||
)
|
||||
}) {
|
||||
Ok(discoverable_tools) if discoverable_tools.is_empty() => None,
|
||||
Ok(discoverable_tools) => Some(ToolSuggestCandidates {
|
||||
tools: discoverable_tools,
|
||||
presentation: ToolSuggestPresentation::ListTool,
|
||||
}),
|
||||
Err(err) => {
|
||||
warn!("failed to load discoverable tool suggestions: {err:#}");
|
||||
None
|
||||
}
|
||||
}
|
||||
} else {
|
||||
None
|
||||
}
|
||||
} else {
|
||||
None
|
||||
}
|
||||
} else {
|
||||
None
|
||||
}
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
.instrument(trace_span!("built_tools.load_discoverable_tools"))
|
||||
.await;
|
||||
|
||||
.instrument(trace_span!("built_tools.load_discoverable_tools"))
|
||||
.await
|
||||
};
|
||||
let mcp_tool_exposure = build_mcp_tool_exposure(
|
||||
&all_mcp_tools,
|
||||
connectors.as_deref(),
|
||||
|
||||
@@ -130,8 +130,8 @@ impl RequestPluginInstallHandler {
|
||||
ToolSuggestPresentation::ListTool => format!(
|
||||
"the discoverable tools returned by {LIST_AVAILABLE_PLUGINS_TO_INSTALL_TOOL_NAME}"
|
||||
),
|
||||
ToolSuggestPresentation::DeveloperContext => {
|
||||
"the developer recommendations".to_string()
|
||||
ToolSuggestPresentation::RecommendationContext => {
|
||||
"the <recommended_plugins> list".to_string()
|
||||
}
|
||||
};
|
||||
FunctionCallError::RespondToModel(format!(
|
||||
|
||||
@@ -39,8 +39,8 @@ pub(crate) fn create_request_plugin_install_tool(
|
||||
ToolSuggestPresentation::ListTool => format!(
|
||||
"# Request plugin/connector install\n\nUse this tool only after `{LIST_AVAILABLE_PLUGINS_TO_INSTALL_TOOL_NAME}` returns a plugin or connector that exactly matches the user's explicit request.\n\nDo not use it for adjacent capabilities, broad recommendations, or tools that merely seem useful. Pass the returned `tool_type` through directly, and pass the returned `id` as `tool_id`.\n\nIMPORTANT: DO NOT call this tool in parallel with other tools."
|
||||
),
|
||||
ToolSuggestPresentation::DeveloperContext =>
|
||||
"# Suggest a recommended plugin installation\n\nSuggest installing a plugin from the developer `<recommended_plugins>` list when it would help with the user's current request. Briefly explain why in `suggest_reason`.".to_string(),
|
||||
ToolSuggestPresentation::RecommendationContext =>
|
||||
"# Suggest a recommended plugin installation\n\nSuggest installing a plugin from the `<recommended_plugins>` list when it would help with the user's current request. Briefly explain why in `suggest_reason`.".to_string(),
|
||||
};
|
||||
|
||||
ToolSpec::Function(ResponsesApiTool {
|
||||
@@ -126,15 +126,15 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn developer_recommendations_change_only_the_description() {
|
||||
fn recommendation_context_changes_only_the_description() {
|
||||
let mut expected = create_request_plugin_install_tool(ToolSuggestPresentation::ListTool);
|
||||
let recommendations =
|
||||
create_request_plugin_install_tool(ToolSuggestPresentation::DeveloperContext);
|
||||
create_request_plugin_install_tool(ToolSuggestPresentation::RecommendationContext);
|
||||
|
||||
let ToolSpec::Function(expected_function) = &mut expected else {
|
||||
panic!("expected function tool specs");
|
||||
};
|
||||
expected_function.description = "# Suggest a recommended plugin installation\n\nSuggest installing a plugin from the developer `<recommended_plugins>` list when it would help with the user's current request. Briefly explain why in `suggest_reason`.".to_string();
|
||||
expected_function.description = "# Suggest a recommended plugin installation\n\nSuggest installing a plugin from the `<recommended_plugins>` list when it would help with the user's current request. Briefly explain why in `suggest_reason`.".to_string();
|
||||
|
||||
assert_eq!(recommendations, expected);
|
||||
}
|
||||
|
||||
@@ -48,8 +48,7 @@ pub(crate) struct ToolRouterParams<'a> {
|
||||
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
|
||||
pub(crate) enum ToolSuggestPresentation {
|
||||
ListTool,
|
||||
#[allow(dead_code)]
|
||||
DeveloperContext,
|
||||
RecommendationContext,
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug)]
|
||||
|
||||
@@ -883,7 +883,7 @@ async fn request_plugin_install_requires_all_discovery_features() {
|
||||
None,
|
||||
Some(ToolSuggestCandidates {
|
||||
tools: Vec::new(),
|
||||
presentation: ToolSuggestPresentation::DeveloperContext,
|
||||
presentation: ToolSuggestPresentation::RecommendationContext,
|
||||
}),
|
||||
] {
|
||||
let plan = probe_with(
|
||||
@@ -959,7 +959,7 @@ async fn request_plugin_install_description_refers_to_recommended_plugins_hint()
|
||||
},
|
||||
ToolPlanInputs {
|
||||
tool_suggest_candidates: Some(plugin_candidates(
|
||||
ToolSuggestPresentation::DeveloperContext,
|
||||
ToolSuggestPresentation::RecommendationContext,
|
||||
)),
|
||||
..ToolPlanInputs::default()
|
||||
},
|
||||
@@ -973,7 +973,7 @@ async fn request_plugin_install_description_refers_to_recommended_plugins_hint()
|
||||
else {
|
||||
panic!("expected request_plugin_install function spec");
|
||||
};
|
||||
assert!(request_description.contains("developer `<recommended_plugins>` list"));
|
||||
assert!(request_description.contains("the `<recommended_plugins>` list"));
|
||||
assert!(!request_description.contains("list_available_plugins_to_install"));
|
||||
assert!(!request_description.contains("github"));
|
||||
plan.assert_visible_lacks(&["list_available_plugins_to_install"]);
|
||||
|
||||
Reference in New Issue
Block a user