mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[codex] Describe uninstalled cross-repo plugin reads (#18449)
## Summary - Populate `PluginDetail.description` in core for uninstalled cross-repo plugins when detailed fields are unavailable until install. - Include the source Git URL plus optional path/ref/sha details in that fallback description. - Keep `details_unavailable_reason` as the structured signal while app-server forwards the description normally. - Add plugin-read coverage proving the response does not clone the remote source just to show the message. ## Why Uninstalled cross-repo plugins intentionally return sparse detail data so listing/reading does not clone the plugin source. Without a description, Desktop and TUI detail pages look like an ordinary empty plugin. This gives users a concrete explanation and source pointer while keeping the existing structured reason available for callers. ## Validation - `just fmt` - `cargo test -p codex-core read_plugin_for_config_uninstalled_git_source_requires_install_without_cloning` - `cargo test -p codex-app-server plugin_read --test all` - `just fix -p codex-core` - `just fix -p codex-app-server` Note: `cargo test -p codex-app-server` was also attempted before the latest refactor and failed broadly in unrelated v2 thread/realtime/review/skills suites; the new plugin-read test passed in that run as well.
This commit is contained in:
committed by
GitHub
Unverified
parent
3f7222ec76
commit
def6467d2b
@@ -511,6 +511,76 @@ async fn plugin_read_accepts_legacy_string_default_prompt() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn plugin_read_describes_uninstalled_git_source_without_cloning() -> Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let repo_root = TempDir::new()?;
|
||||
let missing_remote_repo = repo_root.path().join("missing-remote-plugin-repo");
|
||||
let missing_remote_repo_url = url::Url::from_directory_path(&missing_remote_repo)
|
||||
.unwrap()
|
||||
.to_string();
|
||||
std::fs::create_dir_all(repo_root.path().join(".git"))?;
|
||||
std::fs::create_dir_all(repo_root.path().join(".agents/plugins"))?;
|
||||
std::fs::write(
|
||||
repo_root.path().join(".agents/plugins/marketplace.json"),
|
||||
format!(
|
||||
r#"{{
|
||||
"name": "debug",
|
||||
"plugins": [
|
||||
{{
|
||||
"name": "toolkit",
|
||||
"source": {{
|
||||
"source": "git-subdir",
|
||||
"url": "{missing_remote_repo_url}",
|
||||
"path": "plugins/toolkit"
|
||||
}}
|
||||
}}
|
||||
]
|
||||
}}"#
|
||||
),
|
||||
)?;
|
||||
write_plugins_enabled_config(&codex_home)?;
|
||||
|
||||
let mut mcp = McpProcess::new(codex_home.path()).await?;
|
||||
timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??;
|
||||
|
||||
let request_id = mcp
|
||||
.send_plugin_read_request(PluginReadParams {
|
||||
marketplace_path: Some(AbsolutePathBuf::try_from(
|
||||
repo_root.path().join(".agents/plugins/marketplace.json"),
|
||||
)?),
|
||||
remote_marketplace_name: None,
|
||||
plugin_name: "toolkit".to_string(),
|
||||
})
|
||||
.await?;
|
||||
|
||||
let response: JSONRPCResponse = timeout(
|
||||
DEFAULT_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(request_id)),
|
||||
)
|
||||
.await??;
|
||||
let response: PluginReadResponse = to_response(response)?;
|
||||
|
||||
let expected_description = format!(
|
||||
"This is a cross-repo plugin. Install it to view more detailed information. The source of the plugin is {missing_remote_repo_url}, path `plugins/toolkit`."
|
||||
);
|
||||
assert_eq!(
|
||||
response.plugin.description.as_deref(),
|
||||
Some(expected_description.as_str())
|
||||
);
|
||||
assert!(!response.plugin.summary.installed);
|
||||
assert!(response.plugin.skills.is_empty());
|
||||
assert!(response.plugin.apps.is_empty());
|
||||
assert!(response.plugin.mcp_servers.is_empty());
|
||||
assert!(
|
||||
!codex_home
|
||||
.path()
|
||||
.join("plugins/.marketplace-plugin-source-staging")
|
||||
.exists()
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn plugin_read_returns_invalid_request_when_plugin_is_missing() -> Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
|
||||
@@ -1058,10 +1058,11 @@ impl PluginsManager {
|
||||
})?;
|
||||
let plugin_key = plugin_id.as_key();
|
||||
if matches!(plugin.source, MarketplacePluginSource::Git { .. }) && !plugin.installed {
|
||||
let description = remote_plugin_install_required_description(&plugin.source);
|
||||
return Ok(PluginDetail {
|
||||
id: plugin_key,
|
||||
name: plugin.name,
|
||||
description: None,
|
||||
description: Some(description),
|
||||
source: plugin.source,
|
||||
policy: plugin.policy,
|
||||
interface: plugin.interface,
|
||||
@@ -1495,6 +1496,34 @@ impl PluginsManager {
|
||||
}
|
||||
}
|
||||
|
||||
fn remote_plugin_install_required_description(source: &MarketplacePluginSource) -> String {
|
||||
let source_description = match source {
|
||||
MarketplacePluginSource::Git {
|
||||
url,
|
||||
path,
|
||||
ref_name,
|
||||
sha,
|
||||
} => {
|
||||
let mut parts = vec![url.clone()];
|
||||
if let Some(path) = path {
|
||||
parts.push(format!("path `{path}`"));
|
||||
}
|
||||
if let Some(ref_name) = ref_name {
|
||||
parts.push(format!("ref `{ref_name}`"));
|
||||
}
|
||||
if let Some(sha) = sha {
|
||||
parts.push(format!("sha `{sha}`"));
|
||||
}
|
||||
parts.join(", ")
|
||||
}
|
||||
MarketplacePluginSource::Local { path } => path.as_path().display().to_string(),
|
||||
};
|
||||
|
||||
format!(
|
||||
"This is a cross-repo plugin. Install it to view more detailed information. The source of the plugin is {source_description}."
|
||||
)
|
||||
}
|
||||
|
||||
#[derive(Debug, thiserror::Error)]
|
||||
pub enum PluginInstallError {
|
||||
#[error("{0}")]
|
||||
|
||||
@@ -1623,7 +1623,13 @@ plugins = true
|
||||
Some(PluginDetailsUnavailableReason::InstallRequiredForRemoteSource)
|
||||
);
|
||||
assert!(!outcome.plugin.installed);
|
||||
assert!(outcome.plugin.description.is_none());
|
||||
let expected_description = format!(
|
||||
"This is a cross-repo plugin. Install it to view more detailed information. The source of the plugin is {missing_remote_repo_url}, path `plugins/toolkit`."
|
||||
);
|
||||
assert_eq!(
|
||||
outcome.plugin.description.as_deref(),
|
||||
Some(expected_description.as_str())
|
||||
);
|
||||
assert!(outcome.plugin.skills.is_empty());
|
||||
assert!(outcome.plugin.apps.is_empty());
|
||||
assert!(outcome.plugin.mcp_server_names.is_empty());
|
||||
|
||||
Reference in New Issue
Block a user