mirror of
https://github.com/openai/codex.git
synced 2026-09-20 12:47:38 +00:00
Adds additive dark-mode plugin logo metadata across manifests, remote catalogs, and the app-server protocol while keeping uninstalled Git listings free of synthetic local paths. Supersedes #28945. This replacement uses an upstream branch so trusted CI can use the repository-provided remote Bazel configuration. ## Current state Plugin interfaces expose only the default logo asset. Clients therefore cannot select a dedicated dark-mode logo even when a plugin provides one. ## What this PR changes - Adds nullable `logoDark` and `logoUrlDark` fields to `PluginInterface`. - Resolves local `interface.logoDark` assets and maps remote `logo_url_dark` values. - Removes path-backed interface assets, including `logoDark`, from uninstalled Git fallback listings until the plugin has a real local root. - Updates the bundled plugin validator and manifest reference. - Regenerates the app-server JSON schemas and TypeScript types. Local manifests expose `interface.logoDark` as a package-relative asset path. Remote catalog responses expose `logo_url_dark`. These values map into separate app-server fields so clients can preserve local-path and remote-URL handling. ## Risk The fields are additive and nullable, so existing clients retain their current logo behavior. The main risks are an incomplete mapping path or exposing a synthetic local path for an uninstalled Git plugin. Local-manifest, remote-catalog, fallback-listing, protocol serialization, and app-server integration tests cover those paths. Spiciness: 2/5 ## Testing - `just write-app-server-schema` - `just fmt` - Regression test first failed with `logo_dark` resolved to `/assets/logo-dark.png`, then passed after the fallback-listing fix. - `just test -p codex-core-plugins` (267 tests passed) - `just test -p codex-app-server 'suite::v2::plugin'` (114 tests passed) - `just test -p codex-app-server-protocol -p codex-core-plugins -p codex-plugin -p codex-skills` (517 tests passed before the follow-up) - `just test -p codex-tui plugin` (47 tests passed) - Validated a local plugin manifest containing `interface.logoDark` with the bundled validator. ## Manual verification Create a local plugin with both `interface.logo` and `interface.logoDark`, then call `plugin/list` or `plugin/read`. Confirm the response contains separate `logo` and `logoDark` paths. For a remote catalog entry, confirm `logoUrlDark` is populated from `logo_url_dark`. For an uninstalled Git marketplace entry, confirm path-backed interface assets remain absent until installation. Issue: N/A - coordinated maintainer change.
297 lines
9.6 KiB
Rust
297 lines
9.6 KiB
Rust
use super::*;
|
|
use pretty_assertions::assert_eq;
|
|
|
|
#[test]
|
|
fn build_remote_marketplace_preserves_directory_order_and_appends_installed_only_plugins() {
|
|
let directory_plugins = vec![
|
|
directory_plugin("plugin-z", "zulu"),
|
|
directory_plugin("plugin-m", "mike"),
|
|
];
|
|
let installed_plugins = vec![RemotePluginInstalledItem {
|
|
plugin: directory_plugin("plugin-a", "alpha"),
|
|
enabled: true,
|
|
disabled_skill_names: Vec::new(),
|
|
}];
|
|
|
|
let marketplace = build_remote_marketplace(
|
|
"marketplace",
|
|
"Marketplace",
|
|
directory_plugins,
|
|
installed_plugins,
|
|
/*include_installed_only*/ true,
|
|
)
|
|
.expect("marketplace should be valid")
|
|
.expect("marketplace should not be empty");
|
|
|
|
assert_eq!(
|
|
marketplace
|
|
.plugins
|
|
.into_iter()
|
|
.map(|plugin| plugin.remote_plugin_id)
|
|
.collect::<Vec<_>>(),
|
|
vec!["plugin-z", "plugin-m", "plugin-a"]
|
|
);
|
|
}
|
|
|
|
fn directory_plugin(id: &str, name: &str) -> RemotePluginDirectoryItem {
|
|
RemotePluginDirectoryItem {
|
|
id: id.to_string(),
|
|
name: name.to_string(),
|
|
scope: RemotePluginScope::Global,
|
|
discoverability: None,
|
|
creator_account_user_id: None,
|
|
creator_name: None,
|
|
share_url: None,
|
|
share_principals: None,
|
|
installation_policy: PluginInstallPolicy::Available,
|
|
authentication_policy: PluginAuthPolicy::OnUse,
|
|
availability: PluginAvailability::Available,
|
|
release: RemotePluginReleaseResponse {
|
|
version: None,
|
|
display_name: name.to_string(),
|
|
description: String::new(),
|
|
bundle_download_url: None,
|
|
app_ids: Vec::new(),
|
|
app_manifest: None,
|
|
app_templates: Vec::new(),
|
|
keywords: Vec::new(),
|
|
interface: RemotePluginReleaseInterfaceResponse {
|
|
short_description: None,
|
|
long_description: None,
|
|
developer_name: None,
|
|
category: None,
|
|
capabilities: Vec::new(),
|
|
website_url: None,
|
|
privacy_policy_url: None,
|
|
terms_of_service_url: None,
|
|
brand_color: None,
|
|
default_prompt: None,
|
|
default_prompts: None,
|
|
composer_icon_url: None,
|
|
logo_url: None,
|
|
logo_url_dark: None,
|
|
screenshot_urls: Vec::new(),
|
|
},
|
|
skills: Vec::new(),
|
|
mcp_servers: Vec::new(),
|
|
},
|
|
}
|
|
}
|
|
|
|
#[test]
|
|
fn remote_plugin_interface_maps_dark_logo_url() {
|
|
let mut plugin = directory_plugin("plugin-linear", "linear");
|
|
plugin.release.interface.logo_url_dark =
|
|
Some("https://example.com/linear/logo-dark.png".to_string());
|
|
|
|
assert_eq!(
|
|
remote_plugin_interface_to_info(&plugin)
|
|
.expect("plugin interface")
|
|
.logo_url_dark,
|
|
Some("https://example.com/linear/logo-dark.png".to_string())
|
|
);
|
|
}
|
|
fn item(name: &str, display_name: &str) -> RecommendedPluginItem {
|
|
RecommendedPluginItem {
|
|
id: format!("plugin_{name}"),
|
|
name: name.to_string(),
|
|
status: None,
|
|
installation_policy: None,
|
|
release: RecommendedPluginRelease {
|
|
display_name: display_name.to_string(),
|
|
app_ids: Vec::new(),
|
|
},
|
|
}
|
|
}
|
|
|
|
#[test]
|
|
fn recommended_plugins_enabled_flag_selects_endpoint_or_legacy_mode() {
|
|
let disabled: RecommendedPluginsResponse = serde_json::from_value(serde_json::json!({
|
|
"enabled": false,
|
|
"plugins": [{"id": "plugin_github", "name": "github", "release": {"display_name": "GitHub"}}]
|
|
}))
|
|
.expect("response should deserialize");
|
|
assert_eq!(
|
|
recommended_plugins_mode(disabled),
|
|
RecommendedPluginsMode::Legacy
|
|
);
|
|
|
|
for response in [
|
|
serde_json::json!({"plugins": []}),
|
|
serde_json::json!({"enabled": null, "plugins": []}),
|
|
] {
|
|
let response: RecommendedPluginsResponse =
|
|
serde_json::from_value(response).expect("response should deserialize");
|
|
assert_eq!(
|
|
recommended_plugins_mode(response),
|
|
RecommendedPluginsMode::Legacy
|
|
);
|
|
}
|
|
|
|
let enabled: RecommendedPluginsResponse = serde_json::from_value(serde_json::json!({
|
|
"enabled": true,
|
|
"plugins": []
|
|
}))
|
|
.expect("response should deserialize");
|
|
assert_eq!(
|
|
recommended_plugins_mode(enabled),
|
|
RecommendedPluginsMode::Endpoint {
|
|
plugins: Vec::new()
|
|
}
|
|
);
|
|
}
|
|
|
|
#[test]
|
|
fn recommended_plugins_require_remote_install_identity() {
|
|
let response = serde_json::from_value::<RecommendedPluginsResponse>(serde_json::json!({
|
|
"enabled": true,
|
|
"plugins": [{
|
|
"name": "github",
|
|
"release": {"display_name": "GitHub"}
|
|
}]
|
|
}));
|
|
|
|
assert!(response.is_err());
|
|
}
|
|
|
|
#[test]
|
|
fn recommended_plugins_are_validated_deduplicated_sorted_and_capped() {
|
|
let mut plugins = (0..=52)
|
|
.rev()
|
|
.map(|index| item(&format!("plugin-{index:02}"), &format!("Plugin {index:02}")))
|
|
.collect::<Vec<_>>();
|
|
plugins.push(item("plugin-00", "Duplicate"));
|
|
plugins.push(item("not/a/plugin", "Invalid"));
|
|
plugins.push(RecommendedPluginItem {
|
|
id: "plugin_disabled".to_string(),
|
|
name: "disabled".to_string(),
|
|
status: Some(PluginAvailability::DisabledByAdmin),
|
|
installation_policy: Some(PluginInstallPolicy::Available),
|
|
release: RecommendedPluginRelease {
|
|
display_name: "Disabled".to_string(),
|
|
app_ids: Vec::new(),
|
|
},
|
|
});
|
|
plugins.push(RecommendedPluginItem {
|
|
id: "plugin_not_available".to_string(),
|
|
name: "not-available".to_string(),
|
|
status: Some(PluginAvailability::Available),
|
|
installation_policy: Some(PluginInstallPolicy::NotAvailable),
|
|
release: RecommendedPluginRelease {
|
|
display_name: "Not Available".to_string(),
|
|
app_ids: Vec::new(),
|
|
},
|
|
});
|
|
|
|
let mode = recommended_plugins_mode(RecommendedPluginsResponse {
|
|
enabled: Some(true),
|
|
plugins,
|
|
});
|
|
let RecommendedPluginsMode::Endpoint { plugins } = mode else {
|
|
panic!("expected endpoint mode");
|
|
};
|
|
|
|
assert_eq!(plugins.len(), MAX_RECOMMENDED_PLUGINS);
|
|
assert_eq!(
|
|
plugins.first(),
|
|
Some(&RecommendedPlugin {
|
|
config_id: "plugin-00@openai-curated-remote".to_string(),
|
|
remote_plugin_id: "plugin_plugin-00".to_string(),
|
|
display_name: "Plugin 00".to_string(),
|
|
app_connector_ids: Vec::new(),
|
|
})
|
|
);
|
|
assert_eq!(
|
|
plugins.last(),
|
|
Some(&RecommendedPlugin {
|
|
config_id: "plugin-49@openai-curated-remote".to_string(),
|
|
remote_plugin_id: "plugin_plugin-49".to_string(),
|
|
display_name: "Plugin 49".to_string(),
|
|
app_connector_ids: Vec::new(),
|
|
})
|
|
);
|
|
}
|
|
|
|
#[test]
|
|
fn recommended_plugins_bound_model_visible_fields() {
|
|
let overlong_name = "n".repeat(MAX_RECOMMENDED_PLUGIN_NAME_LEN + 1);
|
|
let overlong_display_name = "D".repeat(MAX_RECOMMENDED_PLUGIN_DISPLAY_NAME_LEN + 1);
|
|
let mode = recommended_plugins_mode(RecommendedPluginsResponse {
|
|
enabled: Some(true),
|
|
plugins: vec![
|
|
item(&overlong_name, "Ignored"),
|
|
item("bounded", &overlong_display_name),
|
|
],
|
|
});
|
|
|
|
assert_eq!(
|
|
mode,
|
|
RecommendedPluginsMode::Endpoint {
|
|
plugins: vec![RecommendedPlugin {
|
|
config_id: "bounded@openai-curated-remote".to_string(),
|
|
remote_plugin_id: "plugin_bounded".to_string(),
|
|
display_name: "D".repeat(MAX_RECOMMENDED_PLUGIN_DISPLAY_NAME_LEN),
|
|
app_connector_ids: Vec::new(),
|
|
}],
|
|
}
|
|
);
|
|
}
|
|
|
|
#[test]
|
|
fn recommended_plugins_preserve_install_identity_and_normalize_app_ids() {
|
|
let mode = recommended_plugins_mode(RecommendedPluginsResponse {
|
|
enabled: Some(true),
|
|
plugins: vec![RecommendedPluginItem {
|
|
id: "plugin_connector_sample".to_string(),
|
|
name: "sample".to_string(),
|
|
status: Some(PluginAvailability::Available),
|
|
installation_policy: Some(PluginInstallPolicy::Available),
|
|
release: RecommendedPluginRelease {
|
|
display_name: "Sample".to_string(),
|
|
app_ids: vec![
|
|
"connector_one".to_string(),
|
|
String::new(),
|
|
"connector_two".to_string(),
|
|
"connector_one".to_string(),
|
|
],
|
|
},
|
|
}],
|
|
});
|
|
|
|
assert_eq!(
|
|
mode,
|
|
RecommendedPluginsMode::Endpoint {
|
|
plugins: vec![RecommendedPlugin {
|
|
config_id: "sample@openai-curated-remote".to_string(),
|
|
remote_plugin_id: "plugin_connector_sample".to_string(),
|
|
display_name: "Sample".to_string(),
|
|
app_connector_ids: vec!["connector_one".to_string(), "connector_two".to_string(),],
|
|
}],
|
|
}
|
|
);
|
|
}
|
|
|
|
#[test]
|
|
fn recommended_plugins_ignore_invalid_remote_plugin_ids() {
|
|
let mode = recommended_plugins_mode(RecommendedPluginsResponse {
|
|
enabled: Some(true),
|
|
plugins: vec![RecommendedPluginItem {
|
|
id: "not/a/plugin".to_string(),
|
|
name: "sample".to_string(),
|
|
status: None,
|
|
installation_policy: None,
|
|
release: RecommendedPluginRelease {
|
|
display_name: "Sample".to_string(),
|
|
app_ids: Vec::new(),
|
|
},
|
|
}],
|
|
});
|
|
|
|
assert_eq!(
|
|
mode,
|
|
RecommendedPluginsMode::Endpoint {
|
|
plugins: Vec::new(),
|
|
}
|
|
);
|
|
}
|