Skip to content

Commit 5d77d4d

Browse files
xl-openaixl-openai
authored
Reimplement skills loading using SkillsManager + skills/list op. (#7914)
refactor the way we load and manage skills: 1. Move skill discovery/caching into SkillsManager and reuse it across sessions. 2. Add the skills/list API (Op::ListSkills/SkillsListResponse) to fetch skills for one or more cwds. Also update app-server for VSCE/App; 3. Trigger skills/list during session startup so UIs preload skills and handle errors immediately.
1 parent a2c86e5 commit 5d77d4d

File tree

29 files changed

+579
-137
lines changed

29 files changed

+579
-137
lines changed

‎codex-rs/app-server-protocol/src/protocol/common.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,10 @@ client_request_definitions! {
121121
params: v2::ThreadCompactParams,
122122
response: v2::ThreadCompactResponse,
123123
},
124+
SkillsList => "skills/list" {
125+
params: v2::SkillsListParams,
126+
response: v2::SkillsListResponse,
127+
},
124128
TurnStart => "turn/start" {
125129
params: v2::TurnStartParams,
126130
response: v2::TurnStartResponse,

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

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@ use codex_protocol::protocol::CreditsSnapshot as CoreCreditsSnapshot;
2121
use codex_protocol::protocol::RateLimitSnapshot as CoreRateLimitSnapshot;
2222
use codex_protocol::protocol::RateLimitWindow as CoreRateLimitWindow;
2323
use codex_protocol::protocol::SessionSource as CoreSessionSource;
24+
use codex_protocol::protocol::SkillErrorInfo as CoreSkillErrorInfo;
25+
use codex_protocol::protocol::SkillMetadata as CoreSkillMetadata;
26+
use codex_protocol::protocol::SkillScope as CoreSkillScope;
2427
use codex_protocol::protocol::TokenUsage as CoreTokenUsage;
2528
use codex_protocol::protocol::TokenUsageInfo as CoreTokenUsageInfo;
2629
use codex_protocol::user_input::UserInput as CoreUserInput;
@@ -967,6 +970,87 @@ pub struct ThreadCompactParams {
967970
#[ts(export_to = "v2/")]
968971
pub struct ThreadCompactResponse {}
969972

973+
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
974+
#[serde(rename_all = "camelCase")]
975+
#[ts(export_to = "v2/")]
976+
pub struct SkillsListParams {
977+
/// When empty, defaults to the current session working directory.
978+
#[serde(default, skip_serializing_if = "Vec::is_empty")]
979+
pub cwds: Vec<PathBuf>,
980+
}
981+
982+
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
983+
#[serde(rename_all = "camelCase")]
984+
#[ts(export_to = "v2/")]
985+
pub struct SkillsListResponse {
986+
pub data: Vec<SkillsListEntry>,
987+
}
988+
989+
#[derive(Serialize, Deserialize, Debug, Clone, Copy, PartialEq, Eq, JsonSchema, TS)]
990+
#[serde(rename_all = "snake_case")]
991+
#[ts(rename_all = "snake_case")]
992+
#[ts(export_to = "v2/")]
993+
pub enum SkillScope {
994+
User,
995+
Repo,
996+
}
997+
998+
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
999+
#[serde(rename_all = "camelCase")]
1000+
#[ts(export_to = "v2/")]
1001+
pub struct SkillMetadata {
1002+
pub name: String,
1003+
pub description: String,
1004+
pub path: PathBuf,
1005+
pub scope: SkillScope,
1006+
}
1007+
1008+
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
1009+
#[serde(rename_all = "camelCase")]
1010+
#[ts(export_to = "v2/")]
1011+
pub struct SkillErrorInfo {
1012+
pub path: PathBuf,
1013+
pub message: String,
1014+
}
1015+
1016+
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
1017+
#[serde(rename_all = "camelCase")]
1018+
#[ts(export_to = "v2/")]
1019+
pub struct SkillsListEntry {
1020+
pub cwd: PathBuf,
1021+
pub skills: Vec<SkillMetadata>,
1022+
pub errors: Vec<SkillErrorInfo>,
1023+
}
1024+
1025+
impl From<CoreSkillMetadata> for SkillMetadata {
1026+
fn from(value: CoreSkillMetadata) -> Self {
1027+
Self {
1028+
name: value.name,
1029+
description: value.description,
1030+
path: value.path,
1031+
scope: value.scope.into(),
1032+
}
1033+
}
1034+
}
1035+
1036+
impl From<CoreSkillScope> for SkillScope {
1037+
fn from(value: CoreSkillScope) -> Self {
1038+
match value {
1039+
CoreSkillScope::User => Self::User,
1040+
CoreSkillScope::Repo => Self::Repo,
1041+
}
1042+
}
1043+
}
1044+
1045+
impl From<CoreSkillErrorInfo> for SkillErrorInfo {
1046+
fn from(value: CoreSkillErrorInfo) -> Self {
1047+
Self {
1048+
path: value.path,
1049+
message: value.message,
1050+
}
1051+
}
1052+
}
1053+
9701054
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
9711055
#[serde(rename_all = "camelCase")]
9721056
#[ts(export_to = "v2/")]

‎codex-rs/app-server/README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ Example (from OpenAI's official VSCode extension):
6565
- `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.
6666
- `command/exec` — run a single command under the server sandbox without starting a thread/turn (handy for utilities and validation).
6767
- `model/list` — list available models (with reasoning effort options).
68+
- `skills/list` — list skills for one or more `cwd` values.
6869
- `mcpServer/oauth/login` — start an OAuth login for a configured MCP server; returns an `authorization_url` and later emits `mcpServer/oauthLogin/completed` once the browser flow finishes.
6970
- `mcpServers/list` — enumerate configured MCP servers with their tools, resources, resource templates, and auth status; supports cursor+limit pagination.
7071
- `feedback/upload` — submit a feedback report (classification + optional reason/logs and conversation_id); returns the tracking thread id.

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

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,8 @@ use codex_app_server_protocol::ServerNotification;
8181
use codex_app_server_protocol::SessionConfiguredNotification;
8282
use codex_app_server_protocol::SetDefaultModelParams;
8383
use codex_app_server_protocol::SetDefaultModelResponse;
84+
use codex_app_server_protocol::SkillsListParams;
85+
use codex_app_server_protocol::SkillsListResponse;
8486
use codex_app_server_protocol::Thread;
8587
use codex_app_server_protocol::ThreadArchiveParams;
8688
use codex_app_server_protocol::ThreadArchiveResponse;
@@ -373,6 +375,9 @@ impl CodexMessageProcessor {
373375
self.send_unimplemented_error(request_id, "thread/compact")
374376
.await;
375377
}
378+
ClientRequest::SkillsList { request_id, params } => {
379+
self.skills_list(request_id, params).await;
380+
}
376381
ClientRequest::TurnStart { request_id, params } => {
377382
self.turn_start(request_id, params).await;
378383
}
@@ -2615,6 +2620,42 @@ impl CodexMessageProcessor {
26152620
.await;
26162621
}
26172622

2623+
async fn skills_list(&self, request_id: RequestId, params: SkillsListParams) {
2624+
let SkillsListParams { cwds } = params;
2625+
let cwds = if cwds.is_empty() {
2626+
vec![self.config.cwd.clone()]
2627+
} else {
2628+
cwds
2629+
};
2630+
2631+
let data = if self.config.features.enabled(Feature::Skills) {
2632+
let skills_manager = self.conversation_manager.skills_manager();
2633+
cwds.into_iter()
2634+
.map(|cwd| {
2635+
let outcome = skills_manager.skills_for_cwd(&cwd);
2636+
let errors = errors_to_info(&outcome.errors);
2637+
let skills = skills_to_info(&outcome.skills);
2638+
codex_app_server_protocol::SkillsListEntry {
2639+
cwd,
2640+
skills,
2641+
errors,
2642+
}
2643+
})
2644+
.collect()
2645+
} else {
2646+
cwds.into_iter()
2647+
.map(|cwd| codex_app_server_protocol::SkillsListEntry {
2648+
cwd,
2649+
skills: Vec::new(),
2650+
errors: Vec::new(),
2651+
})
2652+
.collect()
2653+
};
2654+
self.outgoing
2655+
.send_response(request_id, SkillsListResponse { data })
2656+
.await;
2657+
}
2658+
26182659
async fn interrupt_conversation(
26192660
&mut self,
26202661
request_id: RequestId,
@@ -3260,6 +3301,32 @@ impl CodexMessageProcessor {
32603301
}
32613302
}
32623303

3304+
fn skills_to_info(
3305+
skills: &[codex_core::skills::SkillMetadata],
3306+
) -> Vec<codex_app_server_protocol::SkillMetadata> {
3307+
skills
3308+
.iter()
3309+
.map(|skill| codex_app_server_protocol::SkillMetadata {
3310+
name: skill.name.clone(),
3311+
description: skill.description.clone(),
3312+
path: skill.path.clone(),
3313+
scope: skill.scope.into(),
3314+
})
3315+
.collect()
3316+
}
3317+
3318+
fn errors_to_info(
3319+
errors: &[codex_core::skills::SkillError],
3320+
) -> Vec<codex_app_server_protocol::SkillErrorInfo> {
3321+
errors
3322+
.iter()
3323+
.map(|err| codex_app_server_protocol::SkillErrorInfo {
3324+
path: err.path.clone(),
3325+
message: err.message.clone(),
3326+
})
3327+
.collect()
3328+
}
3329+
32633330
async fn derive_config_from_params(
32643331
overrides: ConfigOverrides,
32653332
cli_overrides: Option<std::collections::HashMap<String, serde_json::Value>>,

‎codex-rs/core/src/auth.rs‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ const REFRESH_TOKEN_UNKNOWN_MESSAGE: &str =
6464
const REFRESH_TOKEN_URL: &str = "https://auth.openai.com/oauth/token";
6565
pub const REFRESH_TOKEN_URL_OVERRIDE_ENV_VAR: &str = "CODEX_REFRESH_TOKEN_URL_OVERRIDE";
6666

67+
#[cfg(any(test, feature = "test-support"))]
6768
static TEST_AUTH_TEMP_DIRS: Lazy<Mutex<Vec<TempDir>>> = Lazy::new(|| Mutex::new(Vec::new()));
6869

6970
#[derive(Debug, Error)]
@@ -1111,6 +1112,18 @@ impl AuthManager {
11111112
})
11121113
}
11131114

1115+
#[cfg(any(test, feature = "test-support"))]
1116+
/// Create an AuthManager with a specific CodexAuth and codex home, for testing only.
1117+
pub fn from_auth_for_testing_with_home(auth: CodexAuth, codex_home: PathBuf) -> Arc<Self> {
1118+
let cached = CachedAuth { auth: Some(auth) };
1119+
Arc::new(Self {
1120+
codex_home,
1121+
inner: RwLock::new(cached),
1122+
enable_codex_api_key_env: false,
1123+
auth_credentials_store_mode: AuthCredentialsStoreMode::File,
1124+
})
1125+
}
1126+
11141127
/// Current cached auth (clone). May be `None` if not logged in or load failed.
11151128
pub fn auth(&self) -> Option<CodexAuth> {
11161129
self.inner.read().ok().and_then(|c| c.auth.clone())

0 commit comments

Comments
 (0)