Skip to content

[codex] Reuse parsed plugin skills during session startup - #28844

Merged
xl-openai merged 2 commits into
mainfrom
xl/plugin-skill-cache
Jun 18, 2026
Merged

[codex] Reuse parsed plugin skills during session startup#28844
xl-openai merged 2 commits into
mainfrom
xl/plugin-skill-cache

Conversation

@xl-openai

@xl-openai xl-openai commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Preserve raw plugin skill-root snapshots in the matching loaded-plugin cache entry, keyed by the effective plugin root identity including namespace.
  • Pass those snapshots through SkillsLoadInput as an optional preload, so session startup reuses plugin parsing while ordinary skill loads pass None.
  • Keep plugin skill loading cohesive: the existing loaders accept the optional snapshots directly, and uncached or marketplace-detail paths do not create a cache.

Why

Plugin discovery already parses plugin skills to determine available capabilities. Cold session startup then scanned and parsed the same roots again while building the skills snapshot.

This solves the same duplicate-work problem as #28623 while keeping ownership narrow: PluginsManager creates and owns PluginSkillSnapshots only for its loaded-plugin cache entry; SkillsService consumes an optional clone. Entry replacement or clearing naturally drops the snapshots, with no separate generation, capacity policy, or watcher coupling.

Validation

  • cargo clippy -p codex-core-skills --all-targets -- -D warnings
  • just test -p codex-core-plugins skills_service_reuses_skills_parsed_during_plugin_load
  • just test -p codex-core-skills namespaces_plugin_skills_using_provided_namespace
  • just fmt

@xl-openai
xl-openai force-pushed the xl/plugin-skill-cache branch from 34763b5 to 4edd999 Compare June 18, 2026 04:27
@xl-openai
xl-openai force-pushed the xl/plugin-skill-cache branch from 4edd999 to 242d739 Compare June 18, 2026 05:01
@xl-openai
xl-openai force-pushed the xl/plugin-skill-cache branch 3 times, most recently from 700c53a to 3bef94a Compare June 18, 2026 07:25
@xl-openai
xl-openai marked this pull request as ready for review June 18, 2026 07:28
@xl-openai
xl-openai requested a review from a team as a code owner June 18, 2026 07:28
};
let cached_snapshot = cache_key.as_ref().and_then(|cache_key| {
let plugin_skill_snapshots = plugin_skill_snapshots?;
plugin_skill_snapshots

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we invalidate these snapshots when SkillsService is cleared? The app-server watcher only clears the skills cache, so a plugin SKILL.md edit can still reuse stale metadata on the next turn after skills/changed fires.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. I took a closer look, and I think the issue is actually that plugin-owned skill roots shouldn’t be registered with SkillsWatcher in the first place. Plugin install/update/uninstall owns invalidating plugin state, including these snapshots. I pushed ce979d1 to exclude plugin roots from watcher registration rather than coupling SkillsService cache clears back into PluginsManager. Thanks for flagging this!

@xl-openai
xl-openai force-pushed the xl/plugin-skill-cache branch from 036f870 to ce979d1 Compare June 18, 2026 20:16
@xl-openai
xl-openai merged commit e83b784 into main Jun 18, 2026
46 of 47 checks passed
@xl-openai
xl-openai deleted the xl/plugin-skill-cache branch June 18, 2026 23:46
@github-actions github-actions Bot locked and limited conversation to collaborators Jun 18, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants