skills: decouple the skills extension from core - #27413
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6498d78837
ℹ️ 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".
| pub(crate) struct AvailableSkillsInstructions { | ||
| skill_lines: Vec<String>, | ||
| } |
There was a problem hiding this comment.
These injected context fragments live in ext/skills instead of core/context (extension.rs:120-121 sends the available-skills fragment; extension.rs:212-217 sends the skill-body fragment). That breaks the context-fragment guidance; move/reuse the structs under core/context.
Useful? React with 👍 / 👎.
1191e92 to
68dc3b7
Compare
6498d78 to
9ff580b
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9ff580bf7e
ℹ️ 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".
# Conflicts: # codex-rs/ext/skills/src/render.rs # codex-rs/ext/skills/tests/skills_extension.rs
Why
ext/skillscurrently depends oncodex-corefor two host concerns: reading the concreteConfigtype and borrowing core-owned model-context fragment types. That coupling prevents the extension from being assembled independently above core and leaves context that belongs to the skills feature owned by core.This stacked PR introduces the host boundary needed for the broader extension migration while intentionally preserving existing skills behavior. It is stacked on #27404.
What changed
SkillsExtensionConfigview and makes skills installation generic over the host config type.Configvalues.ext/skills, preserving their roles, markers, and rendered bytes.codex-coredependency fromcodex-skills-extension.codex-core-skillscompatibility types unchanged for later staged PRs.Behavior
This adds no capability and is intended to have no user-visible or model-visible behavior change. The install API and ownership boundary change internally; emitted skills context remains byte-for-byte compatible.
Validation
just bazel-lock-check