Inject SKILL.md when it's explicitly mentioned. #7763

xl-openai · 2025-12-09T06:40:46Z

Skills load once in core at session start; the cached outcome is reused across core and surfaced to TUI via SessionConfigured.
TUI detects explicit skill selections, and core injects the matching SKILL.md content into the turn when a selected skill is present.

chatgpt-codex-connector

💡 Codex Review

Lines 2159 to 2163 in a98bfdf

    
           // Note that pending_input would be something like a message the user 
        
           // submitted through the UI while the model was running. Though the UI 
        
           // may support this, the model might not. 
        
           let pending_input = sess 
        
               .get_pending_input()

Pending user input bypasses SKILL injection

Skill contents are injected only once from the initial input (line 2139), but follow-up messages queued in pending_input inside the turn loop are never scanned for $<skill> mentions. If a user sends an additional message while the model is running that names a skill, that SKILL.md is never inlined, so the model responds without the requested instructions. Consider running inject_skills on pending_input before adding it to history so mid-turn skill mentions get the same treatment as the initial message.

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

pakrym-oai · 2025-12-09T19:13:24Z

codex-rs/core/src/event_mapping.rs

 fn parse_user_message(message: &[ContentItem]) -> Option<UserMessageItem> {
-    if UserInstructions::is_user_instructions(message) {
+    if UserInstructions::is_user_instructions(message)
+        || SkillInstructions::is_skill_instructions(message)


needs a test. see below in this file

pakrym-oai · 2025-12-09T19:14:34Z

codex-rs/core/src/user_instructions.rs


 pub const USER_INSTRUCTIONS_OPEN_TAG_LEGACY: &str = "<user_instructions>";
 pub const USER_INSTRUCTIONS_PREFIX: &str = "# AGENTS.md instructions for ";
+pub const SKILL_INSTRUCTIONS_PREFIX: &str = "# SKILL.md instructions for ";


We should just do the xml tag without the prefix.

pakrym-oai · 2025-12-09T19:15:00Z

codex-rs/core/src/user_instructions.rs

+}
+
+impl From<SkillInstructions> for ResponseItem {
+    fn from(si: SkillInstructions) -> Self {


Needs a test. See examples below.

pakrym-oai · 2025-12-09T19:37:08Z

codex-rs/core/src/codex.rs

        }
    }

+    async fn inject_skills(


Not sure if re-parsing user message in core is the right approach.

It might be better to pass selected skills as one of UserInputs or as an explicit parameter on UserTurn similar to how we handle images.

The other problem with the current approach is that new skills added during the session won't be mentionable. And the contents of the skills won't be reloaded.

pakrym-oai · 2025-12-09T19:48:00Z

codex-rs/core/src/project_doc.rs

 /// Combines `Config::instructions` and `AGENTS.md` (if present) into a single
 /// string of instructions.
-pub(crate) async fn get_user_instructions(config: &Config) -> Option<String> {
+pub(crate) async fn get_user_instructions(


now what we have a separate skills instruction element should we render skills into it instead of injecting skills into user instructions?

The skills list in project docs is for discovery/implicit triggers only. SKILL instructions are explicit “use this skill” signals that loop in the full SKILL.md context.

codex-rs/core/src/codex.rs

pakrym-oai

PR needs integration tests, that send user messages with skills and assert that correct information is sent to responses api.

xl-openai · 2025-12-10T03:27:32Z

@codex review again except for the issue with follow-up messages queued in pending_input

chatgpt-codex-connector · 2025-12-10T03:34:36Z

Codex Review: Didn't find any major issues. Breezy!

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

pakrym-oai · 2025-12-10T18:50:39Z

codex-rs/core/src/state/session.rs

    pub(crate) session_configuration: SessionConfiguration,
    pub(crate) history: ContextManager,
    pub(crate) latest_rate_limits: Option<RateLimitSnapshot>,
+    pub(crate) skills: Option<SkillLoadOutcome>,


Let's put this onto SessionServices.

Session state contains frequently mutated state.

pakrym-oai · 2025-12-10T18:52:44Z

codex-rs/core/src/codex.rs

+    selected
+}
+
+fn paths_match(a: &Path, b: &Path) -> bool {


Why do we need this check?

pakrym-oai · 2025-12-10T18:53:06Z

codex-rs/core/src/codex.rs

        }
    }

+    async fn inject_skills(


can we please mode this and related skill helpers into skill.rs?

We can also take session as a reference instead of having an instance method on it.

This is to keep the size of codex.rs sane

pakrym-oai · 2025-12-10T18:55:19Z

codex-rs/core/src/codex.rs

+
+fn skill_load_outcome_for_client(
+    outcome: Option<&SkillLoadOutcome>,
+) -> Option<SkillLoadOutcomeInfo> {


Do you think we can move SkillLoadOutcome to the protocol directly and avoid double copies?

Using the protocol type inside core would couple the loader/cache to the external serde/TS model. The current split keeps boundaries clean, and the mapping is trivial. wdyt?

pakrym-oai · 2025-12-10T18:56:25Z

codex-rs/core/src/user_instructions.rs


 pub const USER_INSTRUCTIONS_OPEN_TAG_LEGACY: &str = "<user_instructions>";
 pub const USER_INSTRUCTIONS_PREFIX: &str = "# AGENTS.md instructions for ";
+pub const SKILL_INSTRUCTIONS_PREFIX: &str = "<SKILL";


can we please do lower case, mostly to be consistent with other elements

pakrym-oai · 2025-12-10T18:57:02Z

codex-rs/core/src/user_instructions.rs

+            role: "user".to_string(),
+            content: vec![ContentItem::InputText {
+                text: format!(
+                    "<SKILL name=\"{name}\" path=\"{path}\">\n{contents}\n</SKILL>",


nit: for env context we used sub tags instead of attributes. I don't have a strong opinion but might be good to stay consistent.

pakrym-oai · 2025-12-10T18:58:10Z

codex-rs/core/tests/suite/skills.rs

+}
+
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn user_turn_includes_skill_instructions() -> Result<()> {


very nice!!

consider a test for when skills fail to load.

pakrym-oai · 2025-12-10T18:59:18Z

codex-rs/protocol/src/models.rs

                            }
                        }
                    },
+                    UserInput::Skill { .. } => ContentItem::InputText {


Is this correct ? Should we be creating an extra empty content item?

pakrym-oai · 2025-12-10T19:00:01Z

codex-rs/core/tests/common/test_codex.rs

        Ok(())
    }
+
+    pub async fn submit_items(


most of our tests just call self.codex.submit(Op::UserTurn directly with whatever parameters they need.

Otherwise we'll end up with too many of these overloads.

pakrym-oai · 2025-12-10T19:05:01Z

codex-rs/core/src/codex.rs

+
+        let user_instructions = get_user_instructions(
+            &config,
+            skills_outcome.as_ref().and_then(|outcome| {


why aren't we passin outcome.skills directly?

xl-openai · 2025-12-10T21:25:02Z

@codex review again

chatgpt-codex-connector · 2025-12-10T21:33:52Z

Codex Review: Didn't find any major issues. Nice work!

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

xl-openai requested review from gverma-openai and pakrym-oai December 9, 2025 06:40

chatgpt-codex-connector bot reviewed Dec 9, 2025

View reviewed changes

xl-openai force-pushed the xl/skills branch from a98bfdf to 88fd4a1 Compare December 9, 2025 08:02

pakrym-oai reviewed Dec 9, 2025

View reviewed changes

codex-rs/core/src/codex.rs Show resolved Hide resolved

pakrym-oai requested changes Dec 9, 2025

View reviewed changes

Inject SKILL.md when it's explicitly mentioned.

ece3915

xl-openai force-pushed the xl/skills branch 3 times, most recently from 630a6f1 to 55777bf Compare December 10, 2025 02:02

xl-openai requested a review from pakrym-oai December 10, 2025 02:24

update skill loading + add tests.

85ff32b

xl-openai force-pushed the xl/skills branch from 55777bf to 85ff32b Compare December 10, 2025 02:32

pakrym-oai reviewed Dec 10, 2025

View reviewed changes

pakrym-oai approved these changes Dec 10, 2025

View reviewed changes

address comments.

5f7a5d1

xl-openai force-pushed the xl/skills branch from 9c10521 to 5f7a5d1 Compare December 10, 2025 20:39

xl-openai merged commit b36ecb6 into main Dec 10, 2025
26 checks passed

xl-openai deleted the xl/skills branch December 10, 2025 21:59

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

	// Note that pending_input would be something like a message the user
	// submitted through the UI while the model was running. Though the UI
	// may support this, the model might not.
	let pending_input = sess
	.get_pending_input()

Inject SKILL.md when it's explicitly mentioned. #7763

Inject SKILL.md when it's explicitly mentioned. #7763

Conversation

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

Uh oh!

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

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 Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pakrym-oai left a comment

Choose a reason for hiding this comment

Uh oh!

xl-openai commented Dec 10, 2025

Uh oh!

chatgpt-codex-connector bot commented Dec 10, 2025

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 10, 2025

Uh oh!

chatgpt-codex-connector bot commented Dec 10, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

xl-openai commented Dec 9, 2025 •

edited

Loading

xl-openai Dec 10, 2025 •

edited

Loading