From df9f537a6e105ac7e457f2e4ebc7431d8614393e Mon Sep 17 00:00:00 2001
From: jif <jif@openai.com>
Date: Thu, 27 Aug 2026 14:11:16 +0000
Subject: [PATCH] Remove locking from trusted skill collection (#41150)

## What changed

- Store per-turn trusted skill paths directly in a `BTreeSet` instead of a
  mutex-protected set.
- Mutably record paths and consume the collection when producing the final
  ordered path list.
- Strengthen the bounded-deduplication test to verify the exact collected paths.

GitOrigin-RevId: 69e87fc7783738d85ed204f18a3968d0837c3693
---
 .../guardian-v2/src/async_scorer/extension.rs |  4 ++--
 .../src/async_scorer/trusted_skills.rs        | 19 +++++--------------
 .../src/async_scorer/trusted_skills_tests.rs  | 14 +++++++++-----
 3 files changed, 16 insertions(+), 21 deletions(-)

diff --git a/codex-rs/ext/guardian-v2/src/async_scorer/extension.rs b/codex-rs/ext/guardian-v2/src/async_scorer/extension.rs
index 0355c76c11..eeb4b5b3e6 100644
--- a/codex-rs/ext/guardian-v2/src/async_scorer/extension.rs
+++ b/codex-rs/ext/guardian-v2/src/async_scorer/extension.rs
@@ -678,7 +678,7 @@ impl GuardianV2Extension {
                 None => None,
             };
             let root_snapshot = thread.guardian_root_snapshot().await;
-            let trusted_skills = TrustedSkillInvocations::default();
+            let mut trusted_skills = TrustedSkillInvocations::default();
             for path in local_trusted_skill_paths.iter().chain(
                 root_snapshot
                     .as_ref()
@@ -687,7 +687,7 @@ impl GuardianV2Extension {
             ) {
                 trusted_skills.record(path.clone());
             }
-            let trusted_skill_paths = trusted_skills.snapshot();
+            let trusted_skill_paths = trusted_skills.into_paths();
             let root_authorization_version = root_snapshot
                 .as_ref()
                 .map(|snapshot| snapshot.authorization_version);
diff --git a/codex-rs/ext/guardian-v2/src/async_scorer/trusted_skills.rs b/codex-rs/ext/guardian-v2/src/async_scorer/trusted_skills.rs
index 623c0d7349..5fe92b3035 100644
--- a/codex-rs/ext/guardian-v2/src/async_scorer/trusted_skills.rs
+++ b/codex-rs/ext/guardian-v2/src/async_scorer/trusted_skills.rs
@@ -3,7 +3,6 @@
 use std::collections::BTreeSet;
 use std::path::Path;
 use std::path::PathBuf;
-use std::sync::Mutex;
 
 use codex_core::config::Config;
 use codex_extension_api::ContentItemKind;
@@ -104,14 +103,11 @@ impl TrustedSkillRoots {
 
 /// Bounded, deduplicated user-owned skill paths observed in one turn.
 #[derive(Default)]
-pub(crate) struct TrustedSkillInvocations(Mutex<BTreeSet<String>>);
+pub(crate) struct TrustedSkillInvocations(BTreeSet<String>);
 
 impl TrustedSkillInvocations {
-    pub(crate) fn record(&self, path: String) {
-        let mut skills = self
-            .0
-            .lock()
-            .unwrap_or_else(std::sync::PoisonError::into_inner);
+    pub(crate) fn record(&mut self, path: String) {
+        let skills = &mut self.0;
         if skills.contains(&path)
             || skills.len() >= MAX_TRUSTED_SKILLS
             || skills
@@ -126,13 +122,8 @@ impl TrustedSkillInvocations {
         skills.insert(path);
     }
 
-    pub(crate) fn snapshot(&self) -> Vec<String> {
-        self.0
-            .lock()
-            .unwrap_or_else(std::sync::PoisonError::into_inner)
-            .iter()
-            .cloned()
-            .collect()
+    pub(crate) fn into_paths(self) -> Vec<String> {
+        self.0.into_iter().collect()
     }
 }
 
diff --git a/codex-rs/ext/guardian-v2/src/async_scorer/trusted_skills_tests.rs b/codex-rs/ext/guardian-v2/src/async_scorer/trusted_skills_tests.rs
index c144ad2ae6..21b9b45119 100644
--- a/codex-rs/ext/guardian-v2/src/async_scorer/trusted_skills_tests.rs
+++ b/codex-rs/ext/guardian-v2/src/async_scorer/trusted_skills_tests.rs
@@ -137,21 +137,25 @@ async fn rejects_skills_that_escape_user_roots_through_symlinks() -> Result<()>
 
 #[test]
 fn invoked_skill_paths_are_deduplicated_and_bounded() {
-    let skills = TrustedSkillInvocations::default();
+    let mut skills = TrustedSkillInvocations::default();
     for index in 0..MAX_TRUSTED_SKILLS.saturating_mul(2) {
         let path = format!("/home/user/.codex/skills/{index:03}/SKILL.md");
         skills.record(path.clone());
         skills.record(path);
     }
 
-    let snapshot = skills.snapshot();
-    assert_eq!(snapshot.len(), MAX_TRUSTED_SKILLS);
+    assert_eq!(
+        skills.into_paths(),
+        (0..MAX_TRUSTED_SKILLS)
+            .map(|index| format!("/home/user/.codex/skills/{index:03}/SKILL.md"))
+            .collect::<Vec<_>>()
+    );
 
-    let bounded = TrustedSkillInvocations::default();
+    let mut bounded = TrustedSkillInvocations::default();
     for index in 0..MAX_TRUSTED_SKILLS {
         bounded.record(format!("{index:03}{}", "x".repeat(500)));
     }
-    let snapshot = bounded.snapshot();
+    let snapshot = bounded.into_paths();
     assert!(snapshot.len() < MAX_TRUSTED_SKILLS);
     assert!(snapshot.iter().map(String::len).sum::<usize>() <= MAX_TRUSTED_SKILL_PATHS_BYTES);
 }