[codex] Support marketplace plugin manifest fallback#28789
Conversation
ee33893 to
d177c44
Compare
charlesgong-openai
left a comment
There was a problem hiding this comment.
Inline walkthrough of the rebuilt marketplace-manifest fallback diff.
| pub display_name: Option<String>, | ||
| } | ||
|
|
||
| #[derive(Debug, Clone, PartialEq, Eq)] |
There was a problem hiding this comment.
This value carries the synthesized plugin manifest alongside the resolved marketplace entry. has_metadata deliberately distinguishes metadata-rich entries from bare name/source entries, so a marketplace entry does not silently become installable unless it actually supplies plugin metadata.
| plugin_root.join(".codex-plugin/plugin.json") | ||
| } | ||
|
|
||
| fn marketplace_plugin_manifest_fallback( |
There was a problem hiding this comment.
This is the compatibility boundary. Flattened marketplace fields are preserved wholesale, including skills arrays, commands, agents, strict, and future manifest fields. The helper below only bridges Claude-style top-level presentation fields into Codex interface fields when the nested value is absent.
| } | ||
|
|
||
| plugin_sources.insert(plugin_key, plugin.source); | ||
| let manifest_fallback = find_marketplace_plugin(&marketplace.path, &plugin.name) |
There was a problem hiding this comment.
Non-curated cache refresh now retains fallback JSON with each source. That matters because refresh runs later than marketplace discovery and otherwise loses the metadata needed to compute the fallback version and reinstall a manifest-less plugin.
| }; | ||
| let store = self.store.clone(); | ||
| let codex_home = self.codex_home.clone(); | ||
| let manifest_fallback_contents = resolved |
There was a problem hiding this comment.
Install passes fallback contents directly into PluginStore. There is no prepare-source copy here anymore; Store owns validation and injection inside its existing atomic cache copy, while the four match arms preserve explicit-version and inferred-version behavior.
| plugin_version_for_install_manifest(source_path, manifest) | ||
| } | ||
|
|
||
| fn resolve_install_manifest<'a>( |
There was a problem hiding this comment.
This centralizes precedence: an on-disk plugin.json always wins, including when it is malformed. The fallback is only selected when no supported manifest path exists, matching the previous behavior and avoiding accidental override of a plugin-owned manifest.
| .replace('\\', "/"); | ||
| assert_eq!(logs.matches("ignoring interface.defaultPrompt").count(), 8); | ||
| assert_eq!(logs.matches("gmail/.codex-plugin/plugin.json").count(), 4); | ||
| assert_eq!(logs.matches("ignoring interface.defaultPrompt").count(), 4); |
There was a problem hiding this comment.
The expected warning counts are halved because app and MCP detail loading no longer performs a second manifest read. The test still guards the original invariant: marketplace discovery is not reloaded per plugin.
| } | ||
|
|
||
| #[test] | ||
| fn find_marketplace_plugin_builds_manifest_fallback_from_entry() { |
There was a problem hiding this comment.
This broad fixture is the compatibility contract. It verifies parsed Codex behavior and raw JSON preservation separately, including skills arrays, MCP objects, URL spelling, author, agents, commands, strict, homepage, and category.
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn install_plugin_writes_marketplace_manifest_fallback_when_missing_plugin_json() { |
There was a problem hiding this comment.
This end-to-end install test proves three ownership guarantees together: the source receives no plugin.json, no extra fallback staging root is created, and the installed cache receives a usable synthesized manifest.
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn read_plugin_for_config_uses_marketplace_manifest_fallback_paths_for_local_source() { |
There was a problem hiding this comment.
This is the regression test for the local detail-read gap. It uses a non-default app path and inline MCP declaration so both would disappear if any loader reread only an on-disk manifest.
|
|
||
| #[test] | ||
| fn install_uses_manifest_version_when_present() { | ||
| fn install_prefers_on_disk_manifest_version_over_fallback() { |
There was a problem hiding this comment.
This existing version test now supplies a conflicting fallback version. Keeping the installed path at 1.2.3-beta+7 proves real on-disk manifest precedence over fallback metadata.
a9c17a5 to
9e6d728
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9e6d728ae3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
e453af0 to
13e476c
Compare
Summary
Support marketplace plugins whose source directory does not include a discoverable plugin manifest. Metadata-rich
marketplace.jsonentries now act as fallback plugin manifests for listing, local detail reads, install, and non-curated cache refresh.The fallback preserves marketplace-entry plugin fields wholesale, then adds the small Codex-facing compatibility bridge for presentation metadata. A real source
plugin.jsonalways wins when present.Details
MarketplacePluginManifestFallback, preserving fields such asversion,description,skills,mcpServers,apps,hooks,agents,commands,strict,author, and future manifest fields without a per-field translation list.displayName,author.name,homepage, and marketplacecategoryinto Codex's nestedinterfacefields only when the nested values are absent.nameandsource; existing missing-manifest behavior remains for metadata-free entries.PluginStore, which validates them and injects.codex-plugin/plugin.jsoninto Store's existing atomic copy. Local marketplace source directories are never mutated, and the fallback path no longer needs an additional staging directory.