Reimplement skills loading using SkillsManager + skills/list op. #7914

xl-openai · 2025-12-12T04:56:28Z

refactor the way we load and manage skills:

Move skill discovery/caching into SkillsManager and reuse it across sessions.
Add the skills/list API (Op::ListSkills/SkillsListResponse) to fetch skills for one or more cwds. Also update app-server for VSCE/App;
Trigger skills/list during session startup so UIs preload skills and handle errors immediately.

voctory · 2025-12-12T12:31:05Z

codex-rs/core/src/skills/manager.rs

+    pub fn skills_for_cwd(&self, cwd: &Path) -> SkillLoadOutcome {
+        let cached = match self.cache_by_cwd.read() {
+            Ok(cache) => cache.get(cwd).cloned(),
+            Err(err) => err.into_inner().get(cwd).cloned(),
+        };
+        if let Some(outcome) = cached {
+            return outcome;
+        }
+
+        let mut roots = vec![user_skills_root(&self.codex_home)];
+        if let Some(repo_root) = repo_skills_root(cwd) {
+            roots.push(repo_root);
+        }
+        let outcome = load_skills_from_roots(roots);
+        match self.cache_by_cwd.write() {
+            Ok(mut cache) => {
+                cache.insert(cwd.to_path_buf(), outcome.clone());
+            }


Is it necessary for SkillsManager to cache SkillLoadOutcome for the lifetime of the host process (keyed by cwd) rather than caching per session/conversation (e.g. load once during Codex::spawn)?

As it is, edits/additions to a skill won’t be reflected by skills/list or by starting a “new chat” inside the same long-lived backend, e.g.:

VS Code “New chat” creates a new conversation but reuses the same backend process + SkillsManager cache for that cwd.

TUI /new starts a new session in the same process, so it also reuses the cached skills for that cwd.

If skill hot-refreshing is planned as a direct follow-up, this may be fine, but otherwise I feel like users may expect “new chat” to reflect the latest skills.

Thanks for the feedback. This change is just the first step toward the dynamic loading you’re envisioning. skills/list can be called more frequently, with optional forced reloads.

ambrosino-oai

I can't speak to the rust code, but from an interface perspective for App/VSCE this LGTM

ambrosino-oai · 2025-12-12T17:06:42Z

codex-rs/app-server-protocol/src/protocol/v2.rs

+    pub name: String,
+    pub description: String,
+    pub path: PathBuf,
+    pub scope: SkillScope,


we'll want the icon fields too (cc @gverma-openai @edward-bayes)

We could extend the API later. This is just a minimum set to start with.

pakrym-oai · 2025-12-12T20:54:11Z

codex-rs/app-server-protocol/src/protocol/v2.rs

+#[ts(export_to = "v2/")]
+pub struct SkillsListParams {
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub cwds: Option<Vec<PathBuf>>,


Does UI need to associate individual skill with CWD it came from? Are we better served by asking for a single CWD at a time?

I see, we return combinations of skills and errors. Hmm.

Why is this Optional?

I was thinking about a case where you might want to query the default skills without tying them to a specific CWD, but yeah, maybe that’s not a valid use case.

pakrym-oai · 2025-12-12T20:54:55Z

codex-rs/app-server-protocol/src/protocol/v2.rs

+#[serde(rename_all = "camelCase")]
+#[ts(export_to = "v2/")]
+pub struct SkillsListResponse {
+    pub data: Vec<SkillsListEntry>,


consider adding pagination like other endpoints (models/list)

Consider having many directories open, do we want to wait for all of them to load skills before this method returns?

Good call! We can treat pagination/streaming as a follow-up optimization once we see more skills enabled in practice. For now, loading everything in one go keeps it simple, and clients that need finer-grained control can already split requests by cwd.

pakrym-oai · 2025-12-12T20:59:23Z

codex-rs/app-server/src/codex_message_processor.rs

+            Some(cwds) if !cwds.is_empty() => cwds,
+            _ => vec![self.config.cwd.clone()],
+        };
+        let data = if self.config.features.enabled(Feature::Skills) {


no strong feelings but consider encapsulating this check in skills_manager

I’m inclined to keep the Feature::Skills guard at the call site. SkillsManager doesn’t have access to config/session state, so pushing the check down would either require passing Config in (extra coupling) or just moving the same if behind another method without changing behavior.

pakrym-oai · 2025-12-12T20:59:49Z

codex-rs/app-server/README.md

 - `review/start` — kick off Codex’s automated reviewer for a thread; responds like `turn/start` and emits `item/started`/`item/completed` notifications with `enteredReviewMode` and `exitedReviewMode` items, plus a final assistant `agentMessage` containing the review.
 - `command/exec` — run a single command under the server sandbox without starting a thread/turn (handy for utilities and validation).
 - `model/list` — list available models (with reasoning effort options).
+- `skills/list` — list skills for one or more `cwd` values; each skill includes a `scope` of `user` or `repo` (not thread-scoped).


(not thread-scoped). ? Prompt leftovers?

pakrym-oai · 2025-12-12T21:53:09Z

codex-rs/core/src/conversation_manager.rs

+    #[cfg(any(test, feature = "test-support"))]
+    /// Construct with a dummy AuthManager containing the provided CodexAuth and codex home.
+    /// Used for integration tests: should not be used by ordinary business logic.
+    pub fn with_models_provider_and_home(


let's have with_models_provider a call to this new method to have fewer copies of logic

pakrym-oai · 2025-12-12T21:53:30Z

codex-rs/core/src/codex.rs

    });
    sess.send_event(&turn_context, event).await;

+    let skills_outcome = if sess.enabled(Feature::Skills) {


Consider encapsulating in skills_manager

pakrym-oai · 2025-12-12T21:54:01Z

codex-rs/core/src/auth.rs


+    #[cfg(any(test, feature = "test-support"))]
+    /// Create an AuthManager with a specific CodexAuth and codex home, for testing only.
+    pub fn from_auth_for_testing_with_home(auth: CodexAuth, codex_home: PathBuf) -> Arc<Self> {


Do you care which home is set on auth?

pakrym-oai · 2025-12-12T21:56:02Z

codex-rs/protocol/src/protocol.rs

+    ListSkills {
+        /// Optional working directories to scope repo skills discovery.
+        #[serde(skip_serializing_if = "Option::is_none")]
+        cwds: Option<Vec<PathBuf>>,


consider required

xl-openai force-pushed the xl/skills branch 2 times, most recently from 44f6068 to ba5a045 Compare December 12, 2025 05:53

Reimplement skills loading using SkillsManager + skills/list op.

b1adbc0

xl-openai force-pushed the xl/skills branch from ba5a045 to b1adbc0 Compare December 12, 2025 06:26

xl-openai requested review from ambrosino-oai, gverma-openai and pakrym-oai December 12, 2025 08:24

voctory reviewed Dec 12, 2025

View reviewed changes

ambrosino-oai reviewed Dec 12, 2025

View reviewed changes

pakrym-oai reviewed Dec 12, 2025

View reviewed changes

pakrym-oai approved these changes Dec 12, 2025

View reviewed changes

Address comments.

83c7bd3

Reimplement skills loading using SkillsManager + skills/list op. #7914

Are you sure you want to change the base?

Reimplement skills loading using SkillsManager + skills/list op. #7914

Conversation

xl-openai commented Dec 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ambrosino-oai left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

xl-openai commented Dec 12, 2025 •

edited

Loading