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?

Yes as the load skills load from home directory.

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 · 2025-12-14T17:35:03Z

@codex review again

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ 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".

chatgpt-codex-connector · 2025-12-14T17:45:28Z

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

+        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 {


Refresh cached skills when files change

SkillsManager returns the cached SkillLoadOutcome for a cwd without any invalidation, so once a cwd is cached, later edits/additions to SKILL.md files (or fixes to parse errors) will never be reflected in Op::ListSkills or skill injections until the process restarts. This is a regression from the prior per-session reload behavior and can leave users stuck with stale skills/errors in long‑lived sessions. Consider adding an explicit refresh path (e.g., on ListSkills) or invalidating based on file mtimes.

Useful? React with 👍 / 👎.

xl-openai force-pushed the xl/skills branch 3 times, most recently 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

xl-openai added 2 commits December 14, 2025 00:42

Reimplement skills loading using SkillsManager + skills/list op.

95d2095

Address comments.

d2bf7e7

xl-openai force-pushed the xl/skills branch from 83c7bd3 to d2bf7e7 Compare December 14, 2025 08:44

chatgpt-codex-connector bot reviewed Dec 14, 2025

View reviewed changes

xl-openai merged commit 5d77d4d into main Dec 14, 2025
27 checks passed

xl-openai deleted the xl/skills branch December 14, 2025 17:58

github-actions bot locked and limited conversation to collaborators Dec 14, 2025

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

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

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

xl-openai commented Dec 14, 2025

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Dec 14, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

xl-openai commented Dec 12, 2025 •

edited

Loading