Use plugin/list to get list of plugins for mentions#22375
Conversation
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
fcoury-oai
left a comment
There was a problem hiding this comment.
I did a smoke test with a workspace where plugin/list returned a name, while the installed cache copy used different metadata and only provided the skill capability and it worked as expected.
One this wasn't clear to me: is this supposed to be a gradual migration to use app-server? What happens if we run this connected to an app-server right now? Is there a mismatch between the list and the individual plugin information?
The code looks good, I just need to understand the intention behind this PR 🙂
|
@fcoury-oai The main issue this fixes is that the older TUI-local PluginsManager path cannot correctly surface some cloud/remote plugins, so mentions now use plugin/list for the plugin inventory and display metadata. This will be relevant to some of the upcoming changes to plugins, so this is preemptive to avoid missing plugins in the mentions menu once those changes are live. There is still a temporary split: plugin/list does not expose the capability summary mentions need, so we still use codex_plugin::PluginCapabilitySummary from the local load path to decide whether a plugin has enough capabilities to show in mentions. In the normal local app-server case this works as expected. In a true remote app-server case, remote-only plugins without a matching local capability summary would be omitted from @/$ mentions, so we will need to follow up on the app-server side. |
fcoury-oai
left a comment
There was a problem hiding this comment.
@fcoury-oai The main issue this fixes is that the older TUI-local PluginsManager path cannot correctly surface some cloud/remote plugins, so mentions now use plugin/list for the plugin inventory and display metadata. This will be relevant to some of the upcoming changes to plugins, so this is preemptive to avoid missing plugins in the mentions menu once those changes are live.
I understand now, thank you! Approved.
This switches TUI plugin mentions to use app-server
plugin/listfor plugin inventory and metadata instead ofPluginManager, while keeping the same mention-eligibility filters as before.Same filters as before:
plugin/listfor the mention names/descriptions