mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
feat(app-server): add a skills/changed v2 notification (#13414)
This adds a first-class app-server v2 `skills/changed` notification for the existing skills live-reload signal. Before this change, clients only had the legacy raw `codex/event/skills_update_available` event. With this PR, v2 clients can listen for a typed JSON-RPC notification instead of depending on the legacy `codex/event/*` stream, which we want to remove soon.
This commit is contained in:
committed by
GitHub
Unverified
parent
e951ef4374
commit
0fbd84081b
@@ -148,6 +148,7 @@ Example with notification opt-out:
|
||||
- `experimentalFeature/list` — list feature flags with stage metadata (`beta`, `underDevelopment`, `stable`, etc.), enabled/default-enabled state, and cursor pagination. For non-beta flags, `displayName`/`description`/`announcement` are `null`.
|
||||
- `collaborationMode/list` — list available collaboration mode presets (experimental, no pagination). This response omits built-in developer instructions; clients should either pass `settings.developer_instructions: null` when setting a mode to use Codex's built-in instructions, or provide their own instructions explicitly.
|
||||
- `skills/list` — list skills for one or more `cwd` values (optional `forceReload`).
|
||||
- `skills/changed` — notification emitted when watched local skill files change.
|
||||
- `skills/remote/list` — list public remote skills (**under development; do not call from production clients yet**).
|
||||
- `skills/remote/export` — download a remote skill by `hazelnutId` into `skills` under `codex_home` (**under development; do not call from production clients yet**).
|
||||
- `app/list` — list available apps.
|
||||
@@ -841,6 +842,7 @@ Use `skills/list` to fetch the available skills (optionally scoped by `cwds`, wi
|
||||
You can also add `perCwdExtraUserRoots` to scan additional absolute paths as `user` scope for specific `cwd` entries.
|
||||
Entries whose `cwd` is not present in `cwds` are ignored.
|
||||
`skills/list` might reuse a cached skills result per `cwd`; setting `forceReload` to `true` refreshes the result from disk.
|
||||
The server also emits `skills/changed` notifications when watched local skill files change. Treat this as an invalidation signal and re-run `skills/list` with your current params when needed.
|
||||
|
||||
```json
|
||||
{ "method": "skills/list", "id": 25, "params": {
|
||||
@@ -876,6 +878,13 @@ Entries whose `cwd` is not present in `cwds` are ignored.
|
||||
} }
|
||||
```
|
||||
|
||||
```json
|
||||
{
|
||||
"method": "skills/changed",
|
||||
"params": {}
|
||||
}
|
||||
```
|
||||
|
||||
To enable or disable a skill by path:
|
||||
|
||||
```json
|
||||
|
||||
@@ -61,6 +61,7 @@ use codex_app_server_protocol::ReasoningTextDeltaNotification;
|
||||
use codex_app_server_protocol::RequestId;
|
||||
use codex_app_server_protocol::ServerNotification;
|
||||
use codex_app_server_protocol::ServerRequestPayload;
|
||||
use codex_app_server_protocol::SkillsChangedNotification;
|
||||
use codex_app_server_protocol::TerminalInteractionNotification;
|
||||
use codex_app_server_protocol::ThreadItem;
|
||||
use codex_app_server_protocol::ThreadNameUpdatedNotification;
|
||||
@@ -220,6 +221,15 @@ pub(crate) async fn apply_bespoke_event_handling(
|
||||
.await;
|
||||
handle_turn_complete(conversation_id, event_turn_id, &outgoing, &thread_state).await;
|
||||
}
|
||||
EventMsg::SkillsUpdateAvailable => {
|
||||
if let ApiVersion::V2 = api_version {
|
||||
outgoing
|
||||
.send_server_notification(ServerNotification::SkillsChanged(
|
||||
SkillsChangedNotification {},
|
||||
))
|
||||
.await;
|
||||
}
|
||||
}
|
||||
EventMsg::Warning(_warning_event) => {}
|
||||
EventMsg::ModelReroute(event) => {
|
||||
if let ApiVersion::V2 = api_version {
|
||||
|
||||
@@ -1,18 +1,22 @@
|
||||
use std::time::Duration;
|
||||
|
||||
use anyhow::Context;
|
||||
use anyhow::Result;
|
||||
use app_test_support::McpProcess;
|
||||
use app_test_support::to_response;
|
||||
use codex_app_server_protocol::JSONRPCResponse;
|
||||
use codex_app_server_protocol::RequestId;
|
||||
use codex_app_server_protocol::SkillsChangedNotification;
|
||||
use codex_app_server_protocol::SkillsListExtraRootsForCwd;
|
||||
use codex_app_server_protocol::SkillsListParams;
|
||||
use codex_app_server_protocol::SkillsListResponse;
|
||||
use codex_app_server_protocol::ThreadStartParams;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tempfile::TempDir;
|
||||
use tokio::time::timeout;
|
||||
|
||||
const DEFAULT_TIMEOUT: Duration = Duration::from_secs(10);
|
||||
const WATCHER_TIMEOUT: Duration = Duration::from_secs(20);
|
||||
|
||||
fn write_skill(root: &TempDir, name: &str) -> Result<()> {
|
||||
let skill_dir = root.path().join("skills").join(name);
|
||||
@@ -214,3 +218,60 @@ async fn skills_list_uses_cached_result_until_force_reload() -> Result<()> {
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn skills_changed_notification_is_emitted_after_skill_change() -> Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
write_skill(&codex_home, "demo")?;
|
||||
|
||||
let mut mcp = McpProcess::new(codex_home.path()).await?;
|
||||
timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??;
|
||||
let thread_start_request_id = mcp
|
||||
.send_thread_start_request(ThreadStartParams {
|
||||
model: None,
|
||||
model_provider: None,
|
||||
service_tier: None,
|
||||
cwd: None,
|
||||
approval_policy: None,
|
||||
sandbox: None,
|
||||
config: None,
|
||||
service_name: None,
|
||||
base_instructions: None,
|
||||
developer_instructions: None,
|
||||
personality: None,
|
||||
ephemeral: None,
|
||||
dynamic_tools: None,
|
||||
mock_experimental_field: None,
|
||||
experimental_raw_events: false,
|
||||
persist_extended_history: false,
|
||||
})
|
||||
.await?;
|
||||
let _: JSONRPCResponse = timeout(
|
||||
DEFAULT_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(thread_start_request_id)),
|
||||
)
|
||||
.await??;
|
||||
|
||||
let skill_path = codex_home
|
||||
.path()
|
||||
.join("skills")
|
||||
.join("demo")
|
||||
.join("SKILL.md");
|
||||
std::fs::write(
|
||||
&skill_path,
|
||||
"---\nname: demo\ndescription: updated\n---\n\n# Updated\n",
|
||||
)?;
|
||||
|
||||
let notification = timeout(
|
||||
WATCHER_TIMEOUT,
|
||||
mcp.read_stream_until_notification_message("skills/changed"),
|
||||
)
|
||||
.await??;
|
||||
let params = notification
|
||||
.params
|
||||
.context("skills/changed params must be present")?;
|
||||
let notification: SkillsChangedNotification = serde_json::from_value(params)?;
|
||||
|
||||
assert_eq!(notification, SkillsChangedNotification {});
|
||||
Ok(())
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user