diff --git a/crates/ide-agent/src/native/codex/mod.rs b/crates/ide-agent/src/native/codex/mod.rs index 373305d3d..7219312e8 100644 --- a/crates/ide-agent/src/native/codex/mod.rs +++ b/crates/ide-agent/src/native/codex/mod.rs @@ -53,6 +53,7 @@ fn attach( client: OnceLock::new(), state: Mutex::new(State::default()), models: tokio::sync::Mutex::new(None), + skills: tokio::sync::Mutex::new(()), }); let codex = connect_codex(Arc::new(CodexEvents(Arc::downgrade(&shim))))?; let _ = shim.codex.set(codex.clone()); @@ -82,6 +83,8 @@ struct CodexShim { state: Mutex, /// `model/list`, fetched once. models: tokio::sync::Mutex>>>, + /// Keep skill snapshots in request order, including invalidations during session setup. + skills: tokio::sync::Mutex<()>, } #[derive(Default)] @@ -300,6 +303,20 @@ impl CodexShim { models } + async fn publish_skills(&self, session: &str) { + let _listing = self.skills.lock().await; + let cwd = self.state.lock().threads.get(session).and_then(|t| t.cwd.clone()); + let Some(cwd) = cwd else { return }; + let result = + tokio::time::timeout(Duration::from_secs(5), self.call("skills/list", json!({ "cwds": [cwd] }))).await; + if let Ok(Ok(result)) = result + && let Some(update) = translate::skill_commands(&result) + && self.state.lock().threads.contains_key(session) + { + self.emit(session, update); + } + } + /// Registers a thread codex opened for a session of its own; returns its `configOptions`. fn open_thread(&self, result: &Value, models: &[ModelInfo], settings: Option) -> (String, Value) { let thread = result.get("thread").unwrap_or(&Value::Null); @@ -326,6 +343,7 @@ impl CodexShim { if id.is_empty() { return Err(error(codes::INTERNAL_ERROR, "codex started no thread")); } + self.publish_skills(&id).await; Ok(json!({ "sessionId": id, "configOptions": options })) } @@ -344,6 +362,7 @@ impl CodexShim { for update in translate::history(thread, cwd.as_deref()) { self.emit(id, update); } + self.publish_skills(id).await; Ok(json!({ "configOptions": options })) } @@ -362,6 +381,7 @@ impl CodexShim { } let result = self.call("thread/fork", request).await?; let (id, options) = self.open_thread(&result, &models, settings); + self.publish_skills(&id).await; Ok(json!({ "sessionId": id, "configOptions": options })) } @@ -551,9 +571,25 @@ impl CodexShim { } } - fn on_codex_notification(&self, method: &str, params: &Value) { + fn on_codex_notification(self: &Arc, method: &str, params: &Value) { let thread = params.get("threadId").and_then(Value::as_str).unwrap_or_default(); match method { + "skills/changed" => { + let shim = self.clone(); + let sessions: Vec = self + .state + .lock() + .threads + .iter() + .filter(|(_, t)| t.settings.is_some() && t.parent.is_none()) + .map(|(id, _)| id.clone()) + .collect(); + self.runtime.spawn(async move { + for session in sessions { + shim.publish_skills(&session).await; + } + }); + } "turn/started" => { if let Some(turn) = params.pointer("/turn/id").and_then(Value::as_str) { self.turn_known(thread, turn, true); diff --git a/crates/ide-agent/src/native/codex/tests.rs b/crates/ide-agent/src/native/codex/tests.rs index ab406dc5d..c5456ff68 100644 --- a/crates/ide-agent/src/native/codex/tests.rs +++ b/crates/ide-agent/src/native/codex/tests.rs @@ -528,12 +528,21 @@ async fn play( let mut sent = Vec::new(); for (out, frame) in &t.frames { if *out { - let line = tokio::time::timeout(Duration::from_secs(10), lines.next_line()) - .await - .unwrap_or_else(|_| panic!("the shim didn't send {frame}")) - .unwrap() - .unwrap_or_else(|| panic!("the shim closed before {frame}")); - let actual: Value = serde_json::from_str(&line).unwrap(); + let actual: Value = loop { + let line = tokio::time::timeout(Duration::from_secs(10), lines.next_line()) + .await + .unwrap_or_else(|_| panic!("the shim didn't send {frame}")) + .unwrap() + .unwrap_or_else(|| panic!("the shim closed before {frame}")); + let actual: Value = serde_json::from_str(&line).unwrap(); + // Older recordings predate skill discovery; play an unsupported server. + if actual["method"] == "skills/list" && frame["method"] != "skills/list" { + let reply = json!({ "id": actual["id"], "error": { "code": -32601, "message": "unsupported" } }); + write.write_all(format!("{reply}\n").as_bytes()).await.unwrap(); + continue; + } + break actual; + }; match frame.get("method") { Some(method) => { assert_eq!(actual.get("method"), Some(method), "expected {frame}, got {actual}"); @@ -1056,3 +1065,117 @@ fn session_mcp_servers_become_thread_configuration() { ) ); } + +#[tokio::test(flavor = "multi_thread")] +async fn skills_are_listed_for_sessions_and_refreshed_on_change() { + for (method, native) in + [("session/new", "thread/start"), ("session/load", "thread/resume"), ("session/fork", "thread/fork")] + { + for initial in [ + json!({"result": {"data": [{"cwd": "/work", "errors": [], "skills": [ + {"name": "review", "enabled": true, "description": "Review", "path": "/work/SKILL.md", "scope": "repo"} + ]}]}}), + json!({"error": {"code": -32601, "message": "unsupported"}}), + json!({"result": {}}), + ] { + let listed = initial.pointer("/result/data").is_some(); + let mut initial = initial; + initial["id"] = json!(4); + let transcript = Transcript { + meta: Value::Null, + frames: vec![ + (true, json!({"id": 1, "method": "initialize"})), + (false, json!({"id": 1, "result": {}})), + (true, json!({"method": "initialized"})), + (true, json!({"id": 2, "method": "model/list"})), + (false, json!({"id": 2, "result": {"data": []}})), + (true, json!({"id": 3, "method": native})), + ( + false, + json!({"id": 3, "result": {"thread": {"id": "s", "cwd": "/work", "turns": []}, "cwd": "/work"}}), + ), + (true, json!({"id": 4, "method": "skills/list"})), + (false, initial), + (true, json!({"id": 5, "method": "turn/start"})), + (false, json!({"id": 5, "error": {"code": -32602, "message": "no model in this test"}})), + (false, json!({"method": "skills/changed", "params": {}})), + (true, json!({"id": 6, "method": "skills/list"})), + (false, json!({"id": 6, "result": {"data": [{"cwd": "/work", "skills": [], "errors": []}]}})), + (false, json!({"method": "skills/changed", "params": {}})), + (true, json!({"id": 7, "method": "skills/list"})), + (false, json!({"id": 7, "error": {"code": -32603, "message": "listing failed"}})), + (false, json!({"method": "skills/changed", "params": {}})), + (true, json!({"id": 8, "method": "skills/list"})), + (false, json!({"id": 8, "result": {"data": []}})), + ], + }; + let (ours, theirs) = tokio::io::duplex(1 << 16); + let client = Arc::new(Client::default()); + let acp = attach(client.clone(), |events| { + let (read, write) = tokio::io::split(ours); + Ok(Peer::connect_framed(read, write, events, Framing::Lines)) + }) + .unwrap(); + let (read, write) = tokio::io::split(theirs); + let player = tokio::spawn(async move { play(&transcript, read, write).await }); + request(&acp, "initialize", json!({})).await.unwrap(); + request(&acp, method, json!({"sessionId": "s", "cwd": "/work"})).await.unwrap(); + let initial_updates = client.of("s"); + assert_eq!(initial_updates.len(), usize::from(listed), "{method}"); + if listed { + assert_eq!(initial_updates[0]["availableCommands"][0]["name"], "$review"); + assert_eq!(initial_updates[0]["availableCommands"][0]["source"], "skill"); + } + assert!( + request(&acp, "session/prompt", json!({"sessionId": "s", "prompt": text("/$review check this")})) + .await + .is_err() + ); + let (frames, _read, _write) = player.await.unwrap(); + for frame in sent(&frames, "skills/list") { + assert_eq!(frame["params"], json!({"cwds": ["/work"]})); + } + assert_eq!( + sent(&frames, "turn/start")[0]["params"]["input"], + json!([{"type": "text", "text": "/$review check this"}]) + ); + tokio::time::timeout(Duration::from_secs(5), async { + while client.of("s").len() < usize::from(listed) + 2 { + tokio::task::yield_now().await; + } + }) + .await + .unwrap(); + let updates = client.of("s"); + assert_eq!(updates.len(), usize::from(listed) + 2); + assert_eq!(updates.last().unwrap()["availableCommands"], json!([])); + acp.kill(); + } + } +} + +#[test] +fn skills_become_commands_with_their_invocation_names() { + let update = skill_commands(&json!({"data": [{"skills": [ + {"name": "review", "enabled": true, "description": "Long", "shortDescription": "Short"}, + {"name": "review", "enabled": true, "description": "Duplicate"}, + {"name": "hidden", "enabled": false, "description": "Disabled"}, + {"name": "modern", "enabled": true, "description": "Long", "interface": {"shortDescription": "Modern"}}, + {"name": "full", "enabled": true, "description": "Full description"}, + {"name": "", "enabled": true}, {"description": "No name"} + ]}]})) + .unwrap(); + assert_eq!( + update, + json!({"sessionUpdate": "available_commands_update", "availableCommands": [ + {"name": "$review", "description": "Short", "source": "skill", "input": null}, + {"name": "$modern", "description": "Modern", "source": "skill", "input": null}, + {"name": "$full", "description": "Full description", "source": "skill", "input": null} + ]}) + ); + let commands = crate::config::commands_of(&update, "codex"); + assert!(commands.iter().all(|c| c.source == crate::AgentCommandSource::Skill)); + assert_eq!(commands[0].name, "$review"); + assert!(skill_commands(&json!({"data": []})).unwrap()["availableCommands"].as_array().unwrap().is_empty()); + assert!(skill_commands(&json!({})).is_none()); +} diff --git a/crates/ide-agent/src/native/codex/translate.rs b/crates/ide-agent/src/native/codex/translate.rs index a17d56cc1..95fc823fe 100644 --- a/crates/ide-agent/src/native/codex/translate.rs +++ b/crates/ide-agent/src/native/codex/translate.rs @@ -256,6 +256,31 @@ fn effort_name(effort: &str) -> String { } } +/// The enabled skills from `skills/list`, named as Codex ACP advertises them. +pub(super) fn skill_commands(result: &Value) -> Option { + let entries = result.get("data")?.as_array()?; + let mut seen = std::collections::HashSet::new(); + let commands: Vec = entries + .iter() + .filter_map(|entry| entry.get("skills").and_then(Value::as_array)) + .flatten() + .filter_map(|skill| { + let name = skill.get("name")?.as_str()?.trim(); + if name.is_empty() || skill.get("enabled") == Some(&Value::Bool(false)) || !seen.insert(name) { + return None; + } + let description = skill + .pointer("/interface/shortDescription") + .and_then(Value::as_str) + .or_else(|| skill.get("shortDescription").and_then(Value::as_str)) + .or_else(|| skill.get("description").and_then(Value::as_str)) + .unwrap_or(name); + Some(json!({ "name": format!("${name}"), "description": description, "source": "skill", "input": null })) + }) + .collect(); + Some(json!({ "sessionUpdate": "available_commands_update", "availableCommands": commands })) +} + /// ACP prompt content blocks as codex `UserInput`s. Embedded files become text. pub(super) fn user_input(prompt: &Value) -> Vec { let blocks = prompt.as_array().map(Vec::as_slice).unwrap_or_default();