chore: clarify plugin + app copy in model instructions (#14541)
- clarify app mentions are in user messages - clarify what it means for tools to be provided via `codex_apps` MCP - add plugin descriptions (with basic sanitization) to top-level `## Plugins` section alongside the corresponding plugin names - explain that skills from plugins are prefixed with `plugin_name:` in top-level `##Plugins` section changes to more logically organize `Apps`, `Skills`, and `Plugins` instructions will be in a separate PR, as that shuffles dev + user instructions in ways that change tests broadly. ### Tests confirmed in local rollout, some new tests.
This commit is contained in:
parent
59b588b8ec
commit
9f2da5a9ce
7 changed files with 121 additions and 5 deletions
|
|
@ -2,6 +2,6 @@ use crate::mcp::CODEX_APPS_MCP_SERVER_NAME;
|
|||
|
||||
pub(crate) fn render_apps_section() -> String {
|
||||
format!(
|
||||
"## Apps\nApps are mentioned in the prompt in the format `[$app-name](app://{{connector_id}})`.\nAn app is equivalent to a set of MCP tools within the `{CODEX_APPS_MCP_SERVER_NAME}` MCP.\nWhen you see an app mention, the app's MCP tools are either already provided in `{CODEX_APPS_MCP_SERVER_NAME}`, or do not exist because the user did not install it.\nDo not additionally call list_mcp_resources for apps that are already mentioned."
|
||||
"## Apps\nApps are mentioned in user messages in the format `[$app-name](app://{{connector_id}})`.\nAn app is equivalent to a set of MCP tools within the `{CODEX_APPS_MCP_SERVER_NAME}` MCP.\nWhen you see an app mention, the app's MCP tools are either available tools in the `{CODEX_APPS_MCP_SERVER_NAME}` MCP server, or the tools do not exist because the user has not installed the app.\nDo not additionally call list_mcp_resources for apps that are already mentioned."
|
||||
)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -66,6 +66,7 @@ const DEFAULT_APP_CONFIG_FILE: &str = ".app.json";
|
|||
const OPENAI_CURATED_MARKETPLACE_NAME: &str = "openai-curated";
|
||||
const REMOTE_PLUGIN_SYNC_TIMEOUT: Duration = Duration::from_secs(30);
|
||||
static CURATED_REPO_SYNC_STARTED: AtomicBool = AtomicBool::new(false);
|
||||
const MAX_CAPABILITY_SUMMARY_DESCRIPTION_LEN: usize = 1024;
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
|
||||
pub struct AppConnectorId(pub String);
|
||||
|
|
@ -191,7 +192,7 @@ impl PluginCapabilitySummary {
|
|||
.manifest_name
|
||||
.clone()
|
||||
.unwrap_or_else(|| plugin.config_name.clone()),
|
||||
description: plugin.manifest_description.clone(),
|
||||
description: prompt_safe_plugin_description(plugin.manifest_description.as_deref()),
|
||||
has_skills: !plugin.skill_roots.is_empty(),
|
||||
mcp_server_names,
|
||||
app_connector_ids: plugin.apps.clone(),
|
||||
|
|
@ -213,6 +214,23 @@ impl PluginCapabilitySummary {
|
|||
}
|
||||
}
|
||||
|
||||
fn prompt_safe_plugin_description(description: Option<&str>) -> Option<String> {
|
||||
let description = description?
|
||||
.split_whitespace()
|
||||
.collect::<Vec<_>>()
|
||||
.join(" ");
|
||||
if description.is_empty() {
|
||||
return None;
|
||||
}
|
||||
|
||||
Some(
|
||||
description
|
||||
.chars()
|
||||
.take(MAX_CAPABILITY_SUMMARY_DESCRIPTION_LEN)
|
||||
.collect(),
|
||||
)
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq)]
|
||||
pub struct PluginLoadOutcome {
|
||||
plugins: Vec<LoadedPlugin>,
|
||||
|
|
|
|||
|
|
@ -270,6 +270,73 @@ fn plugin_telemetry_metadata_uses_default_mcp_config_path() {
|
|||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn capability_summary_sanitizes_plugin_descriptions_to_one_line() {
|
||||
let codex_home = TempDir::new().unwrap();
|
||||
let plugin_root = codex_home
|
||||
.path()
|
||||
.join("plugins/cache")
|
||||
.join("test/sample/local");
|
||||
|
||||
write_file(
|
||||
&plugin_root.join(".codex-plugin/plugin.json"),
|
||||
r#"{
|
||||
"name": "sample",
|
||||
"description": "Plugin that\n includes the sample\tserver"
|
||||
}"#,
|
||||
);
|
||||
write_file(
|
||||
&plugin_root.join("skills/sample-search/SKILL.md"),
|
||||
"---\nname: sample-search\ndescription: search sample data\n---\n",
|
||||
);
|
||||
|
||||
let outcome = load_plugins_from_config(&plugin_config_toml(true, true), codex_home.path());
|
||||
|
||||
assert_eq!(
|
||||
outcome.plugins[0].manifest_description.as_deref(),
|
||||
Some("Plugin that\n includes the sample\tserver")
|
||||
);
|
||||
assert_eq!(
|
||||
outcome.capability_summaries()[0].description.as_deref(),
|
||||
Some("Plugin that includes the sample server")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn capability_summary_truncates_overlong_plugin_descriptions() {
|
||||
let codex_home = TempDir::new().unwrap();
|
||||
let plugin_root = codex_home
|
||||
.path()
|
||||
.join("plugins/cache")
|
||||
.join("test/sample/local");
|
||||
let too_long = "x".repeat(MAX_CAPABILITY_SUMMARY_DESCRIPTION_LEN + 1);
|
||||
|
||||
write_file(
|
||||
&plugin_root.join(".codex-plugin/plugin.json"),
|
||||
&format!(
|
||||
r#"{{
|
||||
"name": "sample",
|
||||
"description": "{too_long}"
|
||||
}}"#
|
||||
),
|
||||
);
|
||||
write_file(
|
||||
&plugin_root.join("skills/sample-search/SKILL.md"),
|
||||
"---\nname: sample-search\ndescription: search sample data\n---\n",
|
||||
);
|
||||
|
||||
let outcome = load_plugins_from_config(&plugin_config_toml(true, true), codex_home.path());
|
||||
|
||||
assert_eq!(
|
||||
outcome.plugins[0].manifest_description.as_deref(),
|
||||
Some(too_long.as_str())
|
||||
);
|
||||
assert_eq!(
|
||||
outcome.capability_summaries()[0].description,
|
||||
Some("x".repeat(MAX_CAPABILITY_SUMMARY_DESCRIPTION_LEN))
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn load_plugins_uses_manifest_configured_component_paths() {
|
||||
let codex_home = TempDir::new().unwrap();
|
||||
|
|
|
|||
|
|
@ -14,12 +14,16 @@ pub(crate) fn render_plugins_section(plugins: &[PluginCapabilitySummary]) -> Opt
|
|||
lines.extend(
|
||||
plugins
|
||||
.iter()
|
||||
.map(|plugin| format!("- `{}`", plugin.display_name)),
|
||||
.map(|plugin| match plugin.description.as_deref() {
|
||||
Some(description) => format!("- `{}`: {description}", plugin.display_name),
|
||||
None => format!("- `{}`", plugin.display_name),
|
||||
}),
|
||||
);
|
||||
|
||||
lines.push("### How to use plugins".to_string());
|
||||
lines.push(
|
||||
r###"- Discovery: The list above is the plugins available in this session.
|
||||
- Skill naming: If a plugin contributes skills, those skill entries are prefixed with `plugin_name:` in the Skills list.
|
||||
- Trigger rules: If the user explicitly names a plugin, prefer capabilities associated with that plugin for that turn.
|
||||
- Relationship to capabilities: Plugins are not invoked directly. Use their underlying skills, MCP tools, and app tools to help solve the task.
|
||||
- Preference: When a relevant plugin is available, prefer using capabilities associated with that plugin over standalone capabilities that provide similar functionality.
|
||||
|
|
|
|||
|
|
@ -5,3 +5,19 @@ use pretty_assertions::assert_eq;
|
|||
fn render_plugins_section_returns_none_for_empty_plugins() {
|
||||
assert_eq!(render_plugins_section(&[]), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn render_plugins_section_includes_descriptions_and_skill_naming_guidance() {
|
||||
let rendered = render_plugins_section(&[PluginCapabilitySummary {
|
||||
config_name: "sample@test".to_string(),
|
||||
display_name: "sample".to_string(),
|
||||
description: Some("inspect sample data".to_string()),
|
||||
has_skills: true,
|
||||
..PluginCapabilitySummary::default()
|
||||
}])
|
||||
.expect("plugin section should render");
|
||||
|
||||
let expected = "## Plugins\nA plugin is a local bundle of skills, MCP servers, and apps. Below is the list of plugins that are enabled and available in this session.\n### Available plugins\n- `sample`: inspect sample data\n### How to use plugins\n- Discovery: The list above is the plugins available in this session.\n- Skill naming: If a plugin contributes skills, those skill entries are prefixed with `plugin_name:` in the Skills list.\n- Trigger rules: If the user explicitly names a plugin, prefer capabilities associated with that plugin for that turn.\n- Relationship to capabilities: Plugins are not invoked directly. Use their underlying skills, MCP tools, and app tools to help solve the task.\n- Preference: When a relevant plugin is available, prefer using capabilities associated with that plugin over standalone capabilities that provide similar functionality.\n- Missing/blocked: If the user requests a plugin that is not listed above, or the plugin does not have relevant callable capabilities for the task, say so briefly and continue with the best fallback.";
|
||||
|
||||
assert_eq!(rendered, expected);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -971,7 +971,7 @@ async fn includes_apps_guidance_as_developer_message_for_chatgpt_auth() {
|
|||
let request = resp_mock.single_request();
|
||||
let request_body = request.body_json();
|
||||
let input = request_body["input"].as_array().expect("input array");
|
||||
let apps_snippet = "Apps are mentioned in the prompt in the format";
|
||||
let apps_snippet = "Apps are mentioned in user messages in the format";
|
||||
|
||||
let has_developer_apps_guidance = input.iter().any(|item| {
|
||||
item.get("role").and_then(|value| value.as_str()) == Some("developer")
|
||||
|
|
|
|||
|
|
@ -26,6 +26,7 @@ use wiremock::MockServer;
|
|||
|
||||
const SAMPLE_PLUGIN_CONFIG_NAME: &str = "sample@test";
|
||||
const SAMPLE_PLUGIN_DISPLAY_NAME: &str = "sample";
|
||||
const SAMPLE_PLUGIN_DESCRIPTION: &str = "inspect sample data";
|
||||
|
||||
fn sample_plugin_root(home: &TempDir) -> std::path::PathBuf {
|
||||
home.path().join("plugins/cache/test/sample/local")
|
||||
|
|
@ -36,7 +37,9 @@ fn write_sample_plugin_manifest_and_config(home: &TempDir) -> std::path::PathBuf
|
|||
std::fs::create_dir_all(plugin_root.join(".codex-plugin")).expect("create plugin manifest dir");
|
||||
std::fs::write(
|
||||
plugin_root.join(".codex-plugin/plugin.json"),
|
||||
format!(r#"{{"name":"{SAMPLE_PLUGIN_DISPLAY_NAME}"}}"#),
|
||||
format!(
|
||||
r#"{{"name":"{SAMPLE_PLUGIN_DISPLAY_NAME}","description":"{SAMPLE_PLUGIN_DESCRIPTION}"}}"#
|
||||
),
|
||||
)
|
||||
.expect("write plugin manifest");
|
||||
std::fs::write(
|
||||
|
|
@ -225,6 +228,14 @@ async fn plugin_skills_append_to_instructions() -> Result<()> {
|
|||
instructions_text.contains("`sample`"),
|
||||
"expected enabled plugin name in instructions"
|
||||
);
|
||||
assert!(
|
||||
instructions_text.contains("`sample`: inspect sample data"),
|
||||
"expected plugin description in instructions"
|
||||
);
|
||||
assert!(
|
||||
instructions_text.contains("skill entries are prefixed with `plugin_name:`"),
|
||||
"expected plugin skill naming guidance"
|
||||
);
|
||||
assert!(
|
||||
instructions_text.contains("sample:sample-search: inspect sample data"),
|
||||
"expected namespaced plugin skill summary"
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue