Merge branch 'main' into rhan/oob-thread-metadata

This commit is contained in:
rhan-oai
2026-03-25 13:39:37 -07:00
committed by GitHub
80 changed files with 3784 additions and 1133 deletions
+2
View File
@@ -971,6 +971,7 @@
"jni_0.21.1": "{\"dependencies\":[{\"name\":\"cesu8\",\"req\":\"^1.1.0\"},{\"name\":\"cfg-if\",\"req\":\"^1.0.0\"},{\"name\":\"combine\",\"req\":\"^4.1.0\"},{\"name\":\"java-locator\",\"optional\":true,\"req\":\"^0.1\"},{\"name\":\"jni-sys\",\"req\":\"^0.3.0\"},{\"name\":\"libloading\",\"optional\":true,\"req\":\"^0.7\"},{\"name\":\"log\",\"req\":\"^0.4.4\"},{\"name\":\"thiserror\",\"req\":\"^1.0.20\"},{\"kind\":\"dev\",\"name\":\"assert_matches\",\"req\":\"^1.5.0\"},{\"kind\":\"dev\",\"name\":\"lazy_static\",\"req\":\"^1\"},{\"kind\":\"dev\",\"name\":\"rusty-fork\",\"req\":\"^0.3.0\"},{\"kind\":\"build\",\"name\":\"walkdir\",\"req\":\"^2\"},{\"features\":[\"Win32_Globalization\"],\"name\":\"windows-sys\",\"req\":\"^0.45.0\",\"target\":\"cfg(windows)\"},{\"kind\":\"dev\",\"name\":\"bytemuck\",\"req\":\"^1.13.0\",\"target\":\"cfg(windows)\"}],\"features\":{\"default\":[],\"invocation\":[\"java-locator\",\"libloading\"]}}",
"jobserver_0.1.34": "{\"dependencies\":[{\"features\":[\"std\"],\"name\":\"getrandom\",\"req\":\"^0.3.2\",\"target\":\"cfg(windows)\"},{\"name\":\"libc\",\"req\":\"^0.2.171\",\"target\":\"cfg(unix)\"},{\"features\":[\"fs\"],\"kind\":\"dev\",\"name\":\"nix\",\"req\":\"^0.28.0\",\"target\":\"cfg(unix)\"},{\"kind\":\"dev\",\"name\":\"tempfile\",\"req\":\"^3.10.1\"}],\"features\":{}}",
"js-sys_0.3.85": "{\"dependencies\":[{\"default_features\":false,\"name\":\"once_cell\",\"req\":\"^1.12\"},{\"default_features\":false,\"name\":\"wasm-bindgen\",\"req\":\"=0.2.108\"}],\"features\":{\"default\":[\"std\"],\"std\":[\"wasm-bindgen/std\"]}}",
"jsonwebtoken_9.3.1": "{\"dependencies\":[{\"name\":\"base64\",\"req\":\"^0.22\"},{\"default_features\":false,\"kind\":\"dev\",\"name\":\"criterion\",\"req\":\"^0.4\",\"target\":\"cfg(all(target_arch = \\\"wasm32\\\", not(any(target_os = \\\"emscripten\\\", target_os = \\\"wasi\\\"))))\"},{\"kind\":\"dev\",\"name\":\"criterion\",\"req\":\"^0.4\",\"target\":\"cfg(not(all(target_arch = \\\"wasm32\\\", not(any(target_os = \\\"emscripten\\\", target_os = \\\"wasi\\\")))))\"},{\"name\":\"js-sys\",\"req\":\"^0.3\",\"target\":\"cfg(target_arch = \\\"wasm32\\\")\"},{\"name\":\"pem\",\"optional\":true,\"req\":\"^3\"},{\"features\":[\"std\"],\"name\":\"ring\",\"req\":\"^0.17.4\",\"target\":\"cfg(not(target_arch = \\\"wasm32\\\"))\"},{\"features\":[\"std\",\"wasm32_unknown_unknown_js\"],\"name\":\"ring\",\"req\":\"^0.17.4\",\"target\":\"cfg(target_arch = \\\"wasm32\\\")\"},{\"features\":[\"derive\"],\"name\":\"serde\",\"req\":\"^1.0\"},{\"name\":\"serde_json\",\"req\":\"^1.0\"},{\"name\":\"simple_asn1\",\"optional\":true,\"req\":\"^0.6\"},{\"features\":[\"wasm-bindgen\"],\"kind\":\"dev\",\"name\":\"time\",\"req\":\"^0.3\",\"target\":\"cfg(all(target_arch = \\\"wasm32\\\", not(any(target_os = \\\"emscripten\\\", target_os = \\\"wasi\\\"))))\"},{\"kind\":\"dev\",\"name\":\"time\",\"req\":\"^0.3\",\"target\":\"cfg(not(all(target_arch = \\\"wasm32\\\", not(any(target_os = \\\"emscripten\\\", target_os = \\\"wasi\\\")))))\"},{\"kind\":\"dev\",\"name\":\"wasm-bindgen-test\",\"req\":\"^0.3.1\"}],\"features\":{\"default\":[\"use_pem\"],\"use_pem\":[\"pem\",\"simple_asn1\"]}}",
"keyring_3.6.3": "{\"dependencies\":[{\"kind\":\"dev\",\"name\":\"base64\",\"req\":\"^0.22\"},{\"name\":\"byteorder\",\"optional\":true,\"req\":\"^1.2\",\"target\":\"cfg(target_os = \\\"windows\\\")\"},{\"features\":[\"derive\",\"wrap_help\"],\"kind\":\"dev\",\"name\":\"clap\",\"req\":\"^4\"},{\"name\":\"dbus-secret-service\",\"optional\":true,\"req\":\"^4.0.0-rc.1\",\"target\":\"cfg(target_os = \\\"openbsd\\\")\"},{\"name\":\"dbus-secret-service\",\"optional\":true,\"req\":\"^4.0.0-rc.2\",\"target\":\"cfg(target_os = \\\"linux\\\")\"},{\"name\":\"dbus-secret-service\",\"optional\":true,\"req\":\"^4.0.1\",\"target\":\"cfg(target_os = \\\"freebsd\\\")\"},{\"kind\":\"dev\",\"name\":\"doc-comment\",\"req\":\"^0.3\"},{\"kind\":\"dev\",\"name\":\"env_logger\",\"req\":\"^0.11.5\"},{\"kind\":\"dev\",\"name\":\"fastrand\",\"req\":\"^2\"},{\"features\":[\"std\"],\"name\":\"linux-keyutils\",\"optional\":true,\"req\":\"^0.2\",\"target\":\"cfg(target_os = \\\"linux\\\")\"},{\"name\":\"log\",\"req\":\"^0.4.22\"},{\"name\":\"openssl\",\"optional\":true,\"req\":\"^0.10.66\"},{\"kind\":\"dev\",\"name\":\"rpassword\",\"req\":\"^7\"},{\"kind\":\"dev\",\"name\":\"rprompt\",\"req\":\"^2\"},{\"name\":\"secret-service\",\"optional\":true,\"req\":\"^4\",\"target\":\"cfg(target_os = \\\"freebsd\\\")\"},{\"name\":\"secret-service\",\"optional\":true,\"req\":\"^4\",\"target\":\"cfg(target_os = \\\"linux\\\")\"},{\"name\":\"secret-service\",\"optional\":true,\"req\":\"^4\",\"target\":\"cfg(target_os = \\\"openbsd\\\")\"},{\"name\":\"security-framework\",\"optional\":true,\"req\":\"^2\",\"target\":\"cfg(target_os = \\\"ios\\\")\"},{\"name\":\"security-framework\",\"optional\":true,\"req\":\"^3\",\"target\":\"cfg(target_os = \\\"macos\\\")\"},{\"kind\":\"dev\",\"name\":\"whoami\",\"req\":\"^1.5\"},{\"features\":[\"Win32_Foundation\",\"Win32_Security_Credentials\"],\"name\":\"windows-sys\",\"optional\":true,\"req\":\"^0.60\",\"target\":\"cfg(target_os = \\\"windows\\\")\"},{\"name\":\"zbus\",\"optional\":true,\"req\":\"^4\",\"target\":\"cfg(target_os = \\\"freebsd\\\")\"},{\"name\":\"zbus\",\"optional\":true,\"req\":\"^4\",\"target\":\"cfg(target_os = \\\"linux\\\")\"},{\"name\":\"zbus\",\"optional\":true,\"req\":\"^4\",\"target\":\"cfg(target_os = \\\"openbsd\\\")\"},{\"name\":\"zeroize\",\"req\":\"^1.8.1\",\"target\":\"cfg(target_os = \\\"windows\\\")\"}],\"features\":{\"apple-native\":[\"dep:security-framework\"],\"async-io\":[\"zbus?/async-io\"],\"async-secret-service\":[\"dep:secret-service\",\"dep:zbus\"],\"crypto-openssl\":[\"dbus-secret-service?/crypto-openssl\",\"secret-service?/crypto-openssl\"],\"crypto-rust\":[\"dbus-secret-service?/crypto-rust\",\"secret-service?/crypto-rust\"],\"linux-native\":[\"dep:linux-keyutils\"],\"linux-native-async-persistent\":[\"linux-native\",\"async-secret-service\"],\"linux-native-sync-persistent\":[\"linux-native\",\"sync-secret-service\"],\"sync-secret-service\":[\"dep:dbus-secret-service\"],\"tokio\":[\"zbus?/tokio\"],\"vendored\":[\"dbus-secret-service?/vendored\",\"openssl?/vendored\"],\"windows-native\":[\"dep:windows-sys\",\"dep:byteorder\"]}}",
"kqueue-sys_1.0.4": "{\"dependencies\":[{\"name\":\"bitflags\",\"req\":\"^1.2.1\"},{\"name\":\"libc\",\"req\":\"^0.2.74\"}],\"features\":{}}",
"kqueue_1.1.1": "{\"dependencies\":[{\"features\":[\"html_reports\"],\"kind\":\"dev\",\"name\":\"criterion\",\"req\":\"^0.5\"},{\"kind\":\"dev\",\"name\":\"dhat\",\"req\":\"^0.3.2\"},{\"name\":\"kqueue-sys\",\"req\":\"^1.0.4\"},{\"name\":\"libc\",\"req\":\"^0.2.17\"},{\"kind\":\"dev\",\"name\":\"tempfile\",\"req\":\"^3.1.0\"}],\"features\":{}}",
@@ -1281,6 +1282,7 @@
"simd-adler32_0.3.8": "{\"dependencies\":[{\"kind\":\"dev\",\"name\":\"adler\",\"req\":\"^1.0.2\"},{\"kind\":\"dev\",\"name\":\"adler32\",\"req\":\"^1.2.0\"},{\"kind\":\"dev\",\"name\":\"criterion\",\"req\":\"^0.3\"},{\"kind\":\"dev\",\"name\":\"rand\",\"req\":\"^0.8\"}],\"features\":{\"const-generics\":[],\"default\":[\"std\",\"const-generics\"],\"nightly\":[],\"std\":[]}}",
"simdutf8_0.1.5": "{\"dependencies\":[],\"features\":{\"aarch64_neon\":[],\"aarch64_neon_prefetch\":[],\"default\":[\"std\"],\"hints\":[],\"public_imp\":[],\"std\":[]}}",
"similar_2.7.0": "{\"dependencies\":[{\"default_features\":false,\"name\":\"bstr\",\"optional\":true,\"req\":\"^1.5.0\"},{\"kind\":\"dev\",\"name\":\"console\",\"req\":\"^0.15.0\"},{\"kind\":\"dev\",\"name\":\"insta\",\"req\":\"^1.10.0\"},{\"features\":[\"derive\"],\"name\":\"serde\",\"optional\":true,\"req\":\"^1.0.130\"},{\"kind\":\"dev\",\"name\":\"serde_json\",\"req\":\"^1.0.68\"},{\"name\":\"unicode-segmentation\",\"optional\":true,\"req\":\"^1.7.1\"},{\"name\":\"web-time\",\"optional\":true,\"req\":\"^1.1\"}],\"features\":{\"bytes\":[\"bstr\",\"text\"],\"default\":[\"text\"],\"inline\":[\"text\"],\"text\":[],\"unicode\":[\"text\",\"unicode-segmentation\",\"bstr?/unicode\",\"bstr?/std\"],\"wasm32_web_time\":[\"web-time\"]}}",
"simple_asn1_0.6.4": "{\"dependencies\":[{\"default_features\":false,\"name\":\"num-bigint\",\"req\":\"^0.4\"},{\"default_features\":false,\"name\":\"num-traits\",\"req\":\"^0.2\"},{\"kind\":\"dev\",\"name\":\"quickcheck\",\"req\":\"^1.0.3\"},{\"kind\":\"dev\",\"name\":\"rand\",\"req\":\"^0.8.4\"},{\"default_features\":false,\"name\":\"thiserror\",\"req\":\"^2\"},{\"default_features\":false,\"features\":[\"formatting\",\"macros\",\"parsing\"],\"name\":\"time\",\"req\":\"^0.3.47\"},{\"default_features\":false,\"features\":[\"formatting\",\"macros\",\"parsing\",\"quickcheck\"],\"kind\":\"dev\",\"name\":\"time\",\"req\":\"^0.3\"}],\"features\":{}}",
"siphasher_1.0.2": "{\"dependencies\":[{\"features\":[\"derive\"],\"name\":\"serde\",\"optional\":true,\"req\":\"^1.0\"},{\"name\":\"serde_json\",\"optional\":true,\"req\":\"^1.0\"}],\"features\":{\"default\":[\"std\"],\"serde_no_std\":[\"serde/alloc\"],\"serde_std\":[\"std\",\"serde/std\"],\"std\":[]}}",
"slab_0.4.12": "{\"dependencies\":[{\"default_features\":false,\"features\":[\"alloc\"],\"name\":\"serde\",\"optional\":true,\"req\":\"^1.0.95\"},{\"features\":[\"derive\"],\"kind\":\"dev\",\"name\":\"serde\",\"req\":\"^1\"},{\"kind\":\"dev\",\"name\":\"serde_test\",\"req\":\"^1\"}],\"features\":{\"default\":[\"std\"],\"std\":[]}}",
"smallvec_1.15.1": "{\"dependencies\":[{\"name\":\"arbitrary\",\"optional\":true,\"req\":\"^1\"},{\"default_features\":false,\"name\":\"bincode\",\"optional\":true,\"req\":\"^2\"},{\"kind\":\"dev\",\"name\":\"bincode1\",\"package\":\"bincode\",\"req\":\"^1.0.1\"},{\"kind\":\"dev\",\"name\":\"debugger_test\",\"req\":\"^0.1.0\"},{\"kind\":\"dev\",\"name\":\"debugger_test_parser\",\"req\":\"^0.1.0\"},{\"default_features\":false,\"name\":\"malloc_size_of\",\"optional\":true,\"req\":\"^0.1\"},{\"default_features\":false,\"name\":\"serde\",\"optional\":true,\"req\":\"^1\"},{\"default_features\":false,\"name\":\"unty\",\"optional\":true,\"req\":\"^0.0.4\"}],\"features\":{\"const_generics\":[],\"const_new\":[\"const_generics\"],\"debugger_visualizer\":[],\"drain_filter\":[],\"drain_keep_rest\":[\"drain_filter\"],\"impl_bincode\":[\"bincode\",\"unty\"],\"may_dangle\":[],\"specialization\":[],\"union\":[],\"write\":[]}}",
+62 -2
View File
@@ -1473,8 +1473,11 @@ dependencies = [
"codex-utils-cli",
"codex-utils-json-to-toml",
"codex-utils-pty",
"constant_time_eq",
"core_test_support",
"futures",
"hmac",
"jsonwebtoken",
"opentelemetry",
"opentelemetry_sdk",
"owo-colors",
@@ -1484,6 +1487,7 @@ dependencies = [
"serde",
"serde_json",
"serial_test",
"sha2",
"shlex",
"tempfile",
"time",
@@ -1846,6 +1850,7 @@ dependencies = [
"futures",
"multimap",
"pretty_assertions",
"schemars 0.8.22",
"serde",
"serde_json",
"serde_path_to_error",
@@ -1895,6 +1900,7 @@ dependencies = [
"codex-code-mode",
"codex-config",
"codex-connectors",
"codex-core-skills",
"codex-exec-server",
"codex-execpolicy",
"codex-features",
@@ -1912,7 +1918,6 @@ dependencies = [
"codex-secrets",
"codex-shell-command",
"codex-shell-escalation",
"codex-skills",
"codex-state",
"codex-terminal-detection",
"codex-test-macros",
@@ -1962,7 +1967,6 @@ dependencies = [
"seccompiler",
"serde",
"serde_json",
"serde_yaml",
"serial_test",
"sha1",
"shlex",
@@ -1991,6 +1995,35 @@ dependencies = [
"zstd",
]
[[package]]
name = "codex-core-skills"
version = "0.0.0"
dependencies = [
"anyhow",
"codex-analytics",
"codex-app-server-protocol",
"codex-config",
"codex-instructions",
"codex-login",
"codex-otel",
"codex-protocol",
"codex-skills",
"codex-utils-absolute-path",
"codex-utils-plugins",
"dirs",
"dunce",
"pretty_assertions",
"serde",
"serde_json",
"serde_yaml",
"shlex",
"tempfile",
"tokio",
"toml 0.9.11+spec-1.1.0",
"tracing",
"zip",
]
[[package]]
name = "codex-debug-client"
version = "0.0.0"
@@ -5757,6 +5790,21 @@ dependencies = [
"wasm-bindgen",
]
[[package]]
name = "jsonwebtoken"
version = "9.3.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "5a87cc7a48537badeae96744432de36f4be2b4a34a05a5ef32e9dd8a1c169dde"
dependencies = [
"base64 0.22.1",
"js-sys",
"pem",
"ring",
"serde",
"serde_json",
"simple_asn1",
]
[[package]]
name = "keyring"
version = "3.6.3"
@@ -9247,6 +9295,18 @@ version = "2.7.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "bbbb5d9659141646ae647b42fe094daf6c6192d1620870b449d9557f748b2daa"
[[package]]
name = "simple_asn1"
version = "0.6.4"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "0d585997b0ac10be3c5ee635f1bab02d512760d14b7c468801ac8a01d9ae5f1d"
dependencies = [
"num-bigint",
"num-traits",
"thiserror 2.0.18",
"time",
]
[[package]]
name = "siphasher"
version = "1.0.2"
+5
View File
@@ -25,6 +25,7 @@ members = [
"shell-escalation",
"skills",
"core",
"core-skills",
"hooks",
"instructions",
"secrets",
@@ -119,6 +120,7 @@ codex-cloud-requirements = { path = "cloud-requirements" }
codex-connectors = { path = "connectors" }
codex-config = { path = "config" }
codex-core = { path = "core" }
codex-core-skills = { path = "core-skills" }
codex-exec = { path = "exec" }
codex-exec-server = { path = "exec-server" }
codex-execpolicy = { path = "execpolicy" }
@@ -202,6 +204,7 @@ chrono = "0.4.43"
clap = "4"
clap_complete = "4"
color-eyre = "0.6.3"
constant_time_eq = "0.3.1"
crossbeam-channel = "0.5.15"
crossterm = "0.28.1"
csv = "1.3.1"
@@ -220,6 +223,7 @@ flate2 = "1.1.4"
futures = { version = "0.3", default-features = false }
gethostname = "1.1.0"
globset = "0.4"
hmac = "0.12.1"
http = "1.3.1"
icu_decimal = "2.1"
icu_locale_core = "2.1"
@@ -232,6 +236,7 @@ indexmap = "2.12.0"
insta = "1.46.3"
inventory = "0.3.19"
itertools = "0.14.0"
jsonwebtoken = "9.3.1"
keyring = { version = "3.6", default-features = false }
landlock = "0.4.4"
lazy_static = "1"
+415 -85
View File
@@ -85,17 +85,128 @@ impl From<InProcessServerEvent> for AppServerEvent {
}
fn event_requires_delivery(event: &InProcessServerEvent) -> bool {
// These terminal events drive surface shutdown/completion state. Dropping
// them under backpressure can leave exec/TUI waiting forever even though
// the underlying turn has already ended.
// These transcript and terminal events must remain lossless. Dropping
// streamed assistant text or the authoritative completed item can leave
// the TUI with permanently corrupted markdown, while dropping completion
// notifications can leave surfaces waiting forever.
match event {
InProcessServerEvent::ServerNotification(notification) => {
server_notification_requires_delivery(notification)
}
_ => false,
}
}
/// Returns `true` for notifications that must survive backpressure.
///
/// Transcript events (`AgentMessageDelta`, `PlanDelta`, reasoning deltas) and
/// the authoritative `ItemCompleted` / `TurnCompleted` form the lossless tier
/// of the event stream. Dropping any of these corrupts the visible assistant
/// output or leaves surfaces waiting for a completion signal that already
/// fired. Everything else (`CommandExecutionOutputDelta`, progress, etc.) is
/// best-effort and may be dropped with only cosmetic impact.
///
/// Both the in-process and remote transports delegate to this function so the
/// classification stays in sync.
pub(crate) fn server_notification_requires_delivery(notification: &ServerNotification) -> bool {
matches!(
event,
InProcessServerEvent::ServerNotification(
codex_app_server_protocol::ServerNotification::TurnCompleted(_),
)
notification,
ServerNotification::TurnCompleted(_)
| ServerNotification::ItemCompleted(_)
| ServerNotification::AgentMessageDelta(_)
| ServerNotification::PlanDelta(_)
| ServerNotification::ReasoningSummaryTextDelta(_)
| ServerNotification::ReasoningTextDelta(_)
)
}
/// Outcome of attempting to forward a single event to the consumer channel.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
enum ForwardEventResult {
/// The event was delivered (or intentionally dropped); the stream is healthy.
Continue,
/// The consumer channel is closed; the caller should stop producing events.
DisableStream,
}
/// Forwards a single in-process event to the consumer, respecting the
/// lossless/best-effort split.
///
/// Lossless events (transcript deltas, item/turn completions) block until the
/// consumer drains capacity. Best-effort events use `try_send` and increment
/// `skipped_events` on failure. When a lag marker needs to be flushed before a
/// lossless event, the flush itself blocks so the marker is never lost.
///
/// If a dropped event is a `ServerRequest`, `reject_server_request` is called
/// so the server does not wait for a response that will never come.
async fn forward_in_process_event<F>(
event_tx: &mpsc::Sender<InProcessServerEvent>,
skipped_events: &mut usize,
event: InProcessServerEvent,
mut reject_server_request: F,
) -> ForwardEventResult
where
F: FnMut(ServerRequest),
{
if *skipped_events > 0 {
if event_requires_delivery(&event) {
// Surface lag before the lossless event, but do not let the lag marker itself cause
// us to drop the transcript/completion notification the caller is blocked on.
if event_tx
.send(InProcessServerEvent::Lagged {
skipped: *skipped_events,
})
.await
.is_err()
{
return ForwardEventResult::DisableStream;
}
*skipped_events = 0;
} else {
match event_tx.try_send(InProcessServerEvent::Lagged {
skipped: *skipped_events,
}) {
Ok(()) => {
*skipped_events = 0;
}
Err(mpsc::error::TrySendError::Full(_)) => {
*skipped_events = skipped_events.saturating_add(1);
warn!("dropping in-process app-server event because consumer queue is full");
if let InProcessServerEvent::ServerRequest(request) = event {
reject_server_request(request);
}
return ForwardEventResult::Continue;
}
Err(mpsc::error::TrySendError::Closed(_)) => {
return ForwardEventResult::DisableStream;
}
}
}
}
if event_requires_delivery(&event) {
// Block until the consumer catches up for transcript/completion notifications; this
// preserves the visible assistant output even when the queue is otherwise saturated.
if event_tx.send(event).await.is_err() {
return ForwardEventResult::DisableStream;
}
return ForwardEventResult::Continue;
}
match event_tx.try_send(event) {
Ok(()) => ForwardEventResult::Continue,
Err(mpsc::error::TrySendError::Full(event)) => {
*skipped_events = skipped_events.saturating_add(1);
warn!("dropping in-process app-server event because consumer queue is full");
if let InProcessServerEvent::ServerRequest(request) = event {
reject_server_request(request);
}
ForwardEventResult::Continue
}
Err(mpsc::error::TrySendError::Closed(_)) => ForwardEventResult::DisableStream,
}
}
/// Layered error for [`InProcessAppServerClient::request_typed`].
///
/// This keeps transport failures, server-side JSON-RPC failures, and response
@@ -366,83 +477,26 @@ impl InProcessAppServerClient {
continue;
}
if skipped_events > 0 {
if event_requires_delivery(&event) {
// Surface lag before the terminal event, but
// do not let the lag marker itself cause us to
// drop the completion/abort notification that
// the caller is blocked on.
if event_tx
.send(InProcessServerEvent::Lagged {
skipped: skipped_events,
})
.await
.is_err()
{
event_stream_enabled = false;
continue;
}
skipped_events = 0;
} else {
match event_tx.try_send(InProcessServerEvent::Lagged {
skipped: skipped_events,
}) {
Ok(()) => {
skipped_events = 0;
}
Err(mpsc::error::TrySendError::Full(_)) => {
skipped_events = skipped_events.saturating_add(1);
warn!(
"dropping in-process app-server event because consumer queue is full"
);
if let InProcessServerEvent::ServerRequest(request) = event {
let _ = request_sender.fail_server_request(
request.id().clone(),
JSONRPCErrorError {
code: -32001,
message: "in-process app-server event queue is full".to_string(),
data: None,
},
);
}
continue;
}
Err(mpsc::error::TrySendError::Closed(_)) => {
event_stream_enabled = false;
continue;
}
}
}
}
if event_requires_delivery(&event) {
// Block until the consumer catches up for
// terminal notifications; this preserves the
// completion signal even when the queue is
// otherwise saturated.
if event_tx.send(event).await.is_err() {
event_stream_enabled = false;
}
continue;
}
match event_tx.try_send(event) {
Ok(()) => {}
Err(mpsc::error::TrySendError::Full(event)) => {
skipped_events = skipped_events.saturating_add(1);
warn!("dropping in-process app-server event because consumer queue is full");
if let InProcessServerEvent::ServerRequest(request) = event {
let _ = request_sender.fail_server_request(
request.id().clone(),
JSONRPCErrorError {
code: -32001,
message: "in-process app-server event queue is full".to_string(),
data: None,
},
);
}
}
Err(mpsc::error::TrySendError::Closed(_)) => {
match forward_in_process_event(
&event_tx,
&mut skipped_events,
event,
|request| {
let _ = request_sender.fail_server_request(
request.id().clone(),
JSONRPCErrorError {
code: -32001,
message: "in-process app-server event queue is full"
.to_string(),
data: None,
},
);
},
)
.await
{
ForwardEventResult::Continue => {}
ForwardEventResult::DisableStream => {
event_stream_enabled = false;
}
}
@@ -933,6 +987,53 @@ mod tests {
.expect("message should send");
}
fn command_execution_output_delta_notification(delta: &str) -> ServerNotification {
ServerNotification::CommandExecutionOutputDelta(
codex_app_server_protocol::CommandExecutionOutputDeltaNotification {
thread_id: "thread".to_string(),
turn_id: "turn".to_string(),
item_id: "item".to_string(),
delta: delta.to_string(),
},
)
}
fn agent_message_delta_notification(delta: &str) -> ServerNotification {
ServerNotification::AgentMessageDelta(
codex_app_server_protocol::AgentMessageDeltaNotification {
thread_id: "thread".to_string(),
turn_id: "turn".to_string(),
item_id: "item".to_string(),
delta: delta.to_string(),
},
)
}
fn item_completed_notification(text: &str) -> ServerNotification {
ServerNotification::ItemCompleted(codex_app_server_protocol::ItemCompletedNotification {
thread_id: "thread".to_string(),
turn_id: "turn".to_string(),
item: codex_app_server_protocol::ThreadItem::AgentMessage {
id: "item".to_string(),
text: text.to_string(),
phase: None,
memory_citation: None,
},
})
}
fn turn_completed_notification() -> ServerNotification {
ServerNotification::TurnCompleted(codex_app_server_protocol::TurnCompletedNotification {
thread_id: "thread".to_string(),
turn: codex_app_server_protocol::Turn {
id: "turn".to_string(),
items: Vec::new(),
status: codex_app_server_protocol::TurnStatus::Completed,
error: None,
},
})
}
fn test_remote_connect_args(websocket_url: String) -> RemoteAppServerConnectArgs {
RemoteAppServerConnectArgs {
websocket_url,
@@ -1043,6 +1144,94 @@ mod tests {
client.shutdown().await.expect("shutdown should complete");
}
#[tokio::test]
async fn forward_in_process_event_preserves_transcript_notifications_under_backpressure() {
let (event_tx, mut event_rx) = mpsc::channel(1);
event_tx
.send(InProcessServerEvent::ServerNotification(
command_execution_output_delta_notification("stdout-1"),
))
.await
.expect("initial event should enqueue");
let mut skipped_events = 0usize;
let result = forward_in_process_event(
&event_tx,
&mut skipped_events,
InProcessServerEvent::ServerNotification(command_execution_output_delta_notification(
"stdout-2",
)),
|_| {},
)
.await;
assert_eq!(result, ForwardEventResult::Continue);
assert_eq!(skipped_events, 1);
let receive_task = tokio::spawn(async move {
let mut events = Vec::new();
for _ in 0..5 {
events.push(
timeout(Duration::from_secs(2), event_rx.recv())
.await
.expect("event should arrive before timeout")
.expect("event stream should stay open"),
);
}
events
});
for notification in [
agent_message_delta_notification("hello"),
item_completed_notification("hello"),
turn_completed_notification(),
] {
let result = forward_in_process_event(
&event_tx,
&mut skipped_events,
InProcessServerEvent::ServerNotification(notification),
|_| {},
)
.await;
assert_eq!(result, ForwardEventResult::Continue);
}
assert_eq!(skipped_events, 0);
let events = receive_task
.await
.expect("receiver task should join successfully");
assert!(matches!(
&events[0],
InProcessServerEvent::ServerNotification(
ServerNotification::CommandExecutionOutputDelta(notification)
) if notification.delta == "stdout-1"
));
assert!(matches!(
&events[1],
InProcessServerEvent::Lagged { skipped: 1 }
));
assert!(matches!(
&events[2],
InProcessServerEvent::ServerNotification(ServerNotification::AgentMessageDelta(
notification
)) if notification.delta == "hello"
));
assert!(matches!(
&events[3],
InProcessServerEvent::ServerNotification(ServerNotification::ItemCompleted(
notification
)) if matches!(
&notification.item,
codex_app_server_protocol::ThreadItem::AgentMessage { text, .. } if text == "hello"
)
));
assert!(matches!(
&events[4],
InProcessServerEvent::ServerNotification(ServerNotification::TurnCompleted(
notification
)) if notification.turn.status == codex_app_server_protocol::TurnStatus::Completed
));
}
#[tokio::test]
async fn remote_typed_request_roundtrip_works() {
let websocket_url = start_test_remote_server(|mut websocket| async move {
@@ -1207,6 +1396,107 @@ mod tests {
client.shutdown().await.expect("shutdown should complete");
}
#[tokio::test]
async fn remote_backpressure_preserves_transcript_notifications() {
let (done_tx, done_rx) = tokio::sync::oneshot::channel();
let websocket_url = start_test_remote_server(|mut websocket| async move {
expect_remote_initialize(&mut websocket).await;
for notification in [
command_execution_output_delta_notification("stdout-1"),
command_execution_output_delta_notification("stdout-2"),
agent_message_delta_notification("hello"),
item_completed_notification("hello"),
turn_completed_notification(),
] {
write_websocket_message(
&mut websocket,
JSONRPCMessage::Notification(
serde_json::from_value(
serde_json::to_value(notification)
.expect("notification should serialize"),
)
.expect("notification should convert to JSON-RPC"),
),
)
.await;
}
let _ = done_rx.await;
})
.await;
let mut client = RemoteAppServerClient::connect(RemoteAppServerConnectArgs {
websocket_url,
client_name: "codex-app-server-client-test".to_string(),
client_version: "0.0.0-test".to_string(),
experimental_api: true,
opt_out_notification_methods: Vec::new(),
channel_capacity: 1,
})
.await
.expect("remote client should connect");
let first_event = timeout(Duration::from_secs(2), client.next_event())
.await
.expect("first event should arrive before timeout")
.expect("event stream should stay open");
assert!(matches!(
first_event,
AppServerEvent::ServerNotification(ServerNotification::CommandExecutionOutputDelta(
notification
)) if notification.delta == "stdout-1"
));
let mut remaining_events = Vec::new();
for _ in 0..4 {
remaining_events.push(
timeout(Duration::from_secs(2), client.next_event())
.await
.expect("event should arrive before timeout")
.expect("event stream should stay open"),
);
}
let mut transcript_event_names = Vec::new();
for event in &remaining_events {
match event {
AppServerEvent::Lagged { skipped: 1 } => {}
AppServerEvent::ServerNotification(
ServerNotification::CommandExecutionOutputDelta(notification),
) if notification.delta == "stdout-2" => {}
AppServerEvent::ServerNotification(ServerNotification::AgentMessageDelta(
notification,
)) if notification.delta == "hello" => {
transcript_event_names.push("agent_message_delta");
}
AppServerEvent::ServerNotification(ServerNotification::ItemCompleted(
notification,
)) if matches!(
&notification.item,
codex_app_server_protocol::ThreadItem::AgentMessage { text, .. } if text == "hello"
) =>
{
transcript_event_names.push("item_completed");
}
AppServerEvent::ServerNotification(ServerNotification::TurnCompleted(
notification,
)) if notification.turn.status
== codex_app_server_protocol::TurnStatus::Completed =>
{
transcript_event_names.push("turn_completed");
}
_ => panic!("unexpected remaining event: {event:?}"),
}
}
assert_eq!(
transcript_event_names,
vec!["agent_message_delta", "item_completed", "turn_completed"]
);
done_tx
.send(())
.expect("server completion signal should send");
client.shutdown().await.expect("shutdown should complete");
}
#[tokio::test]
async fn remote_server_request_resolution_roundtrip_works() {
let websocket_url = start_test_remote_server(|mut websocket| async move {
@@ -1446,7 +1736,7 @@ mod tests {
}
#[test]
fn event_requires_delivery_marks_terminal_events() {
fn event_requires_delivery_marks_transcript_and_terminal_events() {
assert!(event_requires_delivery(
&InProcessServerEvent::ServerNotification(
codex_app_server_protocol::ServerNotification::TurnCompleted(
@@ -1462,9 +1752,49 @@ mod tests {
)
)
));
assert!(event_requires_delivery(
&InProcessServerEvent::ServerNotification(
codex_app_server_protocol::ServerNotification::AgentMessageDelta(
codex_app_server_protocol::AgentMessageDeltaNotification {
thread_id: "thread".to_string(),
turn_id: "turn".to_string(),
item_id: "item".to_string(),
delta: "hello".to_string(),
}
)
)
));
assert!(event_requires_delivery(
&InProcessServerEvent::ServerNotification(
codex_app_server_protocol::ServerNotification::ItemCompleted(
codex_app_server_protocol::ItemCompletedNotification {
thread_id: "thread".to_string(),
turn_id: "turn".to_string(),
item: codex_app_server_protocol::ThreadItem::AgentMessage {
id: "item".to_string(),
text: "hello".to_string(),
phase: None,
memory_citation: None,
},
}
)
)
));
assert!(!event_requires_delivery(&InProcessServerEvent::Lagged {
skipped: 1
}));
assert!(!event_requires_delivery(
&InProcessServerEvent::ServerNotification(
codex_app_server_protocol::ServerNotification::CommandExecutionOutputDelta(
codex_app_server_protocol::CommandExecutionOutputDeltaNotification {
thread_id: "thread".to_string(),
turn_id: "turn".to_string(),
item_id: "item".to_string(),
delta: "stdout".to_string(),
}
)
)
));
}
#[tokio::test]
+42 -4
View File
@@ -21,6 +21,7 @@ use crate::RequestResult;
use crate::SHUTDOWN_TIMEOUT;
use crate::TypedRequestError;
use crate::request_method_name;
use crate::server_notification_requires_delivery;
use codex_app_server_protocol::ClientInfo;
use codex_app_server_protocol::ClientNotification;
use codex_app_server_protocol::ClientRequest;
@@ -854,11 +855,11 @@ async fn reject_if_server_request_dropped(
fn event_requires_delivery(event: &AppServerEvent) -> bool {
match event {
AppServerEvent::ServerNotification(ServerNotification::TurnCompleted(_)) => true,
AppServerEvent::ServerNotification(notification) => {
server_notification_requires_delivery(notification)
}
AppServerEvent::Disconnected { .. } => true,
AppServerEvent::Lagged { .. }
| AppServerEvent::ServerNotification(_)
| AppServerEvent::ServerRequest(_) => false,
AppServerEvent::Lagged { .. } | AppServerEvent::ServerRequest(_) => false,
}
}
@@ -905,3 +906,40 @@ async fn write_jsonrpc_message(
))
})
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn event_requires_delivery_marks_transcript_and_disconnect_events() {
assert!(event_requires_delivery(
&AppServerEvent::ServerNotification(ServerNotification::AgentMessageDelta(
codex_app_server_protocol::AgentMessageDeltaNotification {
thread_id: "thread".to_string(),
turn_id: "turn".to_string(),
item_id: "item".to_string(),
delta: "hello".to_string(),
},
),)
));
assert!(event_requires_delivery(
&AppServerEvent::ServerNotification(ServerNotification::ItemCompleted(
codex_app_server_protocol::ItemCompletedNotification {
thread_id: "thread".to_string(),
turn_id: "turn".to_string(),
item: codex_app_server_protocol::ThreadItem::Plan {
id: "item".to_string(),
text: "step".to_string(),
},
}
),)
));
assert!(event_requires_delivery(&AppServerEvent::Disconnected {
message: "closed".to_string(),
}));
assert!(!event_requires_delivery(&AppServerEvent::Lagged {
skipped: 1
}));
}
}
+4
View File
@@ -53,10 +53,14 @@ codex-utils-absolute-path = { workspace = true }
codex-utils-json-to-toml = { workspace = true }
chrono = { workspace = true }
clap = { workspace = true, features = ["derive"] }
constant_time_eq = { workspace = true }
futures = { workspace = true }
hmac = { workspace = true }
jsonwebtoken = { workspace = true }
owo-colors = { workspace = true, features = ["supports-colors"] }
serde = { workspace = true, features = ["derive"] }
serde_json = { workspace = true }
sha2 = { workspace = true }
tempfile = { workspace = true }
time = { workspace = true }
toml = { workspace = true }
+9
View File
@@ -34,6 +34,15 @@ When running with `--listen ws://IP:PORT`, the same listener also serves basic H
Websocket transport is currently experimental and unsupported. Do not rely on it for production workloads.
Security note:
- Loopback websocket listeners (`ws://127.0.0.1:PORT`) remain appropriate for localhost and SSH port-forwarding workflows.
- Non-loopback websocket listeners currently allow unauthenticated connections by default during rollout. If you expose one remotely, configure websocket auth explicitly now.
- Supported auth modes are app-server flags:
- `--ws-auth capability-token --ws-token-file /absolute/path`
- `--ws-auth signed-bearer-token --ws-shared-secret-file /absolute/path` for HMAC-signed JWT/JWS bearer tokens, with optional `--ws-issuer`, `--ws-audience`, `--ws-max-clock-skew-seconds`
- Clients present the credential as `Authorization: Bearer <token>` during the websocket handshake. Auth is enforced before JSON-RPC `initialize`.
Tracing/log output:
- `RUST_LOG` controls log filtering/verbosity.
@@ -203,6 +203,8 @@ use codex_core::config::types::McpServerTransportConfig;
use codex_core::config_loader::CloudRequirementsLoadError;
use codex_core::config_loader::CloudRequirementsLoadErrorCode;
use codex_core::config_loader::CloudRequirementsLoader;
use codex_core::config_loader::LoaderOverrides;
use codex_core::config_loader::load_config_layers_state;
use codex_core::default_client::set_default_client_residency_requirement;
use codex_core::error::CodexErr;
use codex_core::error::Result as CodexResult;
@@ -282,6 +284,7 @@ use codex_state::StateRuntime;
use codex_state::ThreadMetadata;
use codex_state::ThreadMetadataBuilder;
use codex_state::log_db::LogDbLayer;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_json_to_toml::json_to_toml;
use codex_utils_pty::DEFAULT_OUTPUT_BYTES_CAP;
use std::collections::BTreeMap;
@@ -5509,13 +5512,63 @@ impl CodexMessageProcessor {
}
};
let skills_manager = self.thread_manager.skills_manager();
let plugins_manager = self.thread_manager.plugins_manager();
let cli_overrides = self.current_cli_overrides();
let mut data = Vec::new();
for cwd in cwds {
let extra_roots = extra_roots_by_cwd
.get(&cwd)
.map_or(&[][..], std::vec::Vec::as_slice);
let cwd_abs = match AbsolutePathBuf::try_from(cwd.as_path()) {
Ok(path) => path,
Err(err) => {
let error_path = cwd.clone();
data.push(codex_app_server_protocol::SkillsListEntry {
cwd,
skills: Vec::new(),
errors: errors_to_info(&[codex_core::skills::SkillError {
path: error_path,
message: err.to_string(),
}]),
});
continue;
}
};
let config_layer_stack = match load_config_layers_state(
&self.config.codex_home,
Some(cwd_abs),
&cli_overrides,
LoaderOverrides::default(),
CloudRequirementsLoader::default(),
)
.await
{
Ok(config_layer_stack) => config_layer_stack,
Err(err) => {
let error_path = cwd.clone();
data.push(codex_app_server_protocol::SkillsListEntry {
cwd,
skills: Vec::new(),
errors: errors_to_info(&[codex_core::skills::SkillError {
path: error_path,
message: err.to_string(),
}]),
});
continue;
}
};
let effective_skill_roots = plugins_manager.effective_skill_roots_for_layer_stack(
&config_layer_stack,
config.features.enabled(Feature::Plugins),
);
let skills_input = codex_core::skills::SkillsLoadInput::new(
cwd.clone(),
effective_skill_roots,
config_layer_stack,
config.bundled_skills_enabled(),
);
let outcome = skills_manager
.skills_for_cwd_with_extra_user_roots(&cwd, &config, force_reload, extra_roots)
.skills_for_cwd_with_extra_user_roots(&skills_input, force_reload, extra_roots)
.await;
let errors = errors_to_info(&outcome.errors);
let skills = skills_to_info(&outcome.skills, &outcome.disabled_paths);
+7
View File
@@ -27,6 +27,7 @@ use crate::transport::CHANNEL_CAPACITY;
use crate::transport::ConnectionState;
use crate::transport::OutboundConnectionState;
use crate::transport::TransportEvent;
use crate::transport::auth::policy_from_settings;
use crate::transport::route_outgoing_envelope;
use crate::transport::start_stdio_connection;
use crate::transport::start_websocket_acceptor;
@@ -81,6 +82,9 @@ mod transport;
pub use crate::error_code::INPUT_TOO_LARGE_ERROR_CODE;
pub use crate::error_code::INVALID_PARAMS_ERROR_CODE;
pub use crate::transport::AppServerTransport;
pub use crate::transport::auth::AppServerWebsocketAuthArgs;
pub use crate::transport::auth::AppServerWebsocketAuthSettings;
pub use crate::transport::auth::WebsocketAuthCliMode;
const LOG_FORMAT_ENV_VAR: &str = "LOG_FORMAT";
@@ -337,6 +341,7 @@ pub async fn run_main(
default_analytics_enabled,
AppServerTransport::Stdio,
SessionSource::VSCode,
AppServerWebsocketAuthSettings::default(),
)
.await
}
@@ -348,6 +353,7 @@ pub async fn run_main_with_transport(
default_analytics_enabled: bool,
transport: AppServerTransport,
session_source: SessionSource,
auth: AppServerWebsocketAuthSettings,
) -> IoResult<()> {
let (transport_event_tx, mut transport_event_rx) =
mpsc::channel::<TransportEvent>(CHANNEL_CAPACITY);
@@ -375,6 +381,7 @@ pub async fn run_main_with_transport(
bind_address,
transport_event_tx.clone(),
shutdown_token.clone(),
policy_from_settings(&auth)?,
)
.await?;
TransportRuntime::WebSocket {
+6
View File
@@ -1,5 +1,6 @@
use clap::Parser;
use codex_app_server::AppServerTransport;
use codex_app_server::AppServerWebsocketAuthArgs;
use codex_app_server::run_main_with_transport;
use codex_arg0::Arg0DispatchPaths;
use codex_arg0::arg0_dispatch_or_else;
@@ -31,6 +32,9 @@ struct AppServerArgs {
value_parser = SessionSource::from_startup_arg
)]
session_source: SessionSource,
#[command(flatten)]
auth: AppServerWebsocketAuthArgs,
}
fn main() -> anyhow::Result<()> {
@@ -43,6 +47,7 @@ fn main() -> anyhow::Result<()> {
};
let transport = args.listen;
let session_source = args.session_source;
let auth = args.auth.try_into_settings()?;
run_main_with_transport(
arg0_paths,
@@ -51,6 +56,7 @@ fn main() -> anyhow::Result<()> {
/*default_analytics_enabled*/ false,
transport,
session_source,
auth,
)
.await?;
Ok(())
+30 -4
View File
@@ -1,3 +1,8 @@
pub(crate) mod auth;
use self::auth::WebsocketAuthPolicy;
use self::auth::authorize_upgrade;
use self::auth::should_warn_about_unauthenticated_non_loopback_listener;
use crate::error_code::OVERLOADED_ERROR_CODE;
use crate::message_processor::ConnectionSessionState;
use crate::outgoing_message::ConnectionId;
@@ -12,6 +17,7 @@ use axum::extract::State;
use axum::extract::ws::Message as WebSocketMessage;
use axum::extract::ws::WebSocket;
use axum::extract::ws::WebSocketUpgrade;
use axum::http::HeaderMap;
use axum::http::Request;
use axum::http::StatusCode;
use axum::http::header::ORIGIN;
@@ -83,7 +89,7 @@ fn print_websocket_startup_banner(addr: SocketAddr) {
);
} else {
eprintln!(
" {note_label} this is a raw WS server; consider running behind TLS/auth for real remote use"
" {note_label} websocket auth is opt-in in this build; configure `--ws-auth ...` before real remote use"
);
}
}
@@ -92,6 +98,7 @@ fn print_websocket_startup_banner(addr: SocketAddr) {
struct WebSocketListenerState {
transport_event_tx: mpsc::Sender<TransportEvent>,
connection_counter: Arc<AtomicU64>,
auth_policy: Arc<WebsocketAuthPolicy>,
}
async fn health_check_handler() -> StatusCode {
@@ -118,12 +125,23 @@ async fn websocket_upgrade_handler(
websocket: WebSocketUpgrade,
ConnectInfo(peer_addr): ConnectInfo<SocketAddr>,
State(state): State<WebSocketListenerState>,
headers: HeaderMap,
) -> impl IntoResponse {
if let Err(err) = authorize_upgrade(&headers, state.auth_policy.as_ref()) {
warn!(
%peer_addr,
message = err.message(),
"rejecting websocket client during upgrade"
);
return (err.status_code(), err.message()).into_response();
}
let connection_id = ConnectionId(state.connection_counter.fetch_add(1, Ordering::Relaxed));
info!(%peer_addr, "websocket client connected");
websocket.on_upgrade(move |stream| async move {
run_websocket_connection(connection_id, stream, state.transport_event_tx).await;
})
websocket
.on_upgrade(move |stream| async move {
run_websocket_connection(connection_id, stream, state.transport_event_tx).await;
})
.into_response()
}
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
@@ -333,7 +351,14 @@ pub(crate) async fn start_websocket_acceptor(
bind_address: SocketAddr,
transport_event_tx: mpsc::Sender<TransportEvent>,
shutdown_token: CancellationToken,
auth_policy: WebsocketAuthPolicy,
) -> IoResult<JoinHandle<()>> {
if should_warn_about_unauthenticated_non_loopback_listener(bind_address, &auth_policy) {
warn!(
%bind_address,
"starting non-loopback websocket listener without auth; websocket auth is opt-in for now and will become the default in a future release"
);
}
let listener = TcpListener::bind(bind_address).await?;
let local_addr = listener.local_addr()?;
print_websocket_startup_banner(local_addr);
@@ -347,6 +372,7 @@ pub(crate) async fn start_websocket_acceptor(
.with_state(WebSocketListenerState {
transport_event_tx,
connection_counter: Arc::new(AtomicU64::new(1)),
auth_policy: Arc::new(auth_policy),
});
let server = axum::serve(
listener,
+583
View File
@@ -0,0 +1,583 @@
use anyhow::Context;
use axum::http::HeaderMap;
use axum::http::StatusCode;
use axum::http::header::AUTHORIZATION;
use clap::Args;
use clap::ValueEnum;
use codex_utils_absolute_path::AbsolutePathBuf;
use constant_time_eq::constant_time_eq_32;
use jsonwebtoken::Algorithm;
use jsonwebtoken::DecodingKey;
use jsonwebtoken::Validation;
use jsonwebtoken::decode;
use serde::Deserialize;
use sha2::Digest;
use sha2::Sha256;
use std::io;
use std::io::ErrorKind;
use std::net::SocketAddr;
use std::path::Path;
use std::path::PathBuf;
use time::OffsetDateTime;
const DEFAULT_MAX_CLOCK_SKEW_SECONDS: u64 = 30;
const MIN_SIGNED_BEARER_SECRET_BYTES: usize = 32;
const INVALID_AUTHORIZATION_HEADER_MESSAGE: &str = "invalid authorization header";
#[derive(Debug, Clone, Default, PartialEq, Eq, Args)]
pub struct AppServerWebsocketAuthArgs {
/// Websocket auth mode for non-loopback listeners.
#[arg(long = "ws-auth", value_name = "MODE", value_enum)]
pub ws_auth: Option<WebsocketAuthCliMode>,
/// Absolute path to the capability-token file.
#[arg(long = "ws-token-file", value_name = "PATH")]
pub ws_token_file: Option<PathBuf>,
/// Absolute path to the shared secret file for signed JWT bearer tokens.
#[arg(long = "ws-shared-secret-file", value_name = "PATH")]
pub ws_shared_secret_file: Option<PathBuf>,
/// Expected issuer for signed JWT bearer tokens.
#[arg(long = "ws-issuer", value_name = "ISSUER")]
pub ws_issuer: Option<String>,
/// Expected audience for signed JWT bearer tokens.
#[arg(long = "ws-audience", value_name = "AUDIENCE")]
pub ws_audience: Option<String>,
/// Maximum clock skew when validating signed JWT bearer tokens.
#[arg(long = "ws-max-clock-skew-seconds", value_name = "SECONDS")]
pub ws_max_clock_skew_seconds: Option<u64>,
}
#[derive(Debug, Clone, Copy, PartialEq, Eq, ValueEnum)]
pub enum WebsocketAuthCliMode {
CapabilityToken,
SignedBearerToken,
}
#[derive(Debug, Clone, Default, PartialEq, Eq)]
pub struct AppServerWebsocketAuthSettings {
pub config: Option<AppServerWebsocketAuthConfig>,
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum AppServerWebsocketAuthConfig {
CapabilityToken {
token_file: AbsolutePathBuf,
},
SignedBearerToken {
shared_secret_file: AbsolutePathBuf,
issuer: Option<String>,
audience: Option<String>,
max_clock_skew_seconds: u64,
},
}
#[derive(Clone, Debug, Default)]
pub(crate) struct WebsocketAuthPolicy {
pub(crate) mode: Option<WebsocketAuthMode>,
}
#[derive(Clone, Debug)]
pub(crate) enum WebsocketAuthMode {
CapabilityToken {
token_sha256: [u8; 32],
},
SignedBearerToken {
shared_secret: Vec<u8>,
issuer: Option<String>,
audience: Option<String>,
max_clock_skew_seconds: i64,
},
}
#[derive(Debug)]
pub(crate) struct WebsocketAuthError {
status_code: StatusCode,
message: &'static str,
}
#[derive(Deserialize)]
struct JwtClaims {
exp: i64,
nbf: Option<i64>,
iss: Option<String>,
aud: Option<JwtAudienceClaim>,
}
#[derive(Deserialize)]
#[serde(untagged)]
enum JwtAudienceClaim {
Single(String),
Multiple(Vec<String>),
}
impl WebsocketAuthError {
pub(crate) fn status_code(&self) -> StatusCode {
self.status_code
}
pub(crate) fn message(&self) -> &'static str {
self.message
}
}
impl AppServerWebsocketAuthArgs {
pub fn try_into_settings(self) -> anyhow::Result<AppServerWebsocketAuthSettings> {
let normalize = |value: Option<String>| {
value.and_then(|value| {
let trimmed = value.trim();
(!trimmed.is_empty()).then(|| trimmed.to_string())
})
};
let config = match self.ws_auth {
Some(WebsocketAuthCliMode::CapabilityToken) => {
if self.ws_shared_secret_file.is_some()
|| self.ws_issuer.is_some()
|| self.ws_audience.is_some()
|| self.ws_max_clock_skew_seconds.is_some()
{
anyhow::bail!(
"`--ws-shared-secret-file`, `--ws-issuer`, `--ws-audience`, and `--ws-max-clock-skew-seconds` require `--ws-auth signed-bearer-token`"
);
}
let token_file = self.ws_token_file.context(
"`--ws-token-file` is required when `--ws-auth capability-token` is set",
)?;
Some(AppServerWebsocketAuthConfig::CapabilityToken {
token_file: absolute_path_arg("--ws-token-file", token_file)?,
})
}
Some(WebsocketAuthCliMode::SignedBearerToken) => {
if self.ws_token_file.is_some() {
anyhow::bail!(
"`--ws-token-file` requires `--ws-auth capability-token`, not `signed-bearer-token`"
);
}
let shared_secret_file = self.ws_shared_secret_file.context(
"`--ws-shared-secret-file` is required when `--ws-auth signed-bearer-token` is set",
)?;
Some(AppServerWebsocketAuthConfig::SignedBearerToken {
shared_secret_file: absolute_path_arg(
"--ws-shared-secret-file",
shared_secret_file,
)?,
issuer: normalize(self.ws_issuer),
audience: normalize(self.ws_audience),
max_clock_skew_seconds: self
.ws_max_clock_skew_seconds
.unwrap_or(DEFAULT_MAX_CLOCK_SKEW_SECONDS),
})
}
None => {
if self.ws_token_file.is_some()
|| self.ws_shared_secret_file.is_some()
|| self.ws_issuer.is_some()
|| self.ws_audience.is_some()
|| self.ws_max_clock_skew_seconds.is_some()
{
anyhow::bail!(
"websocket auth flags require `--ws-auth capability-token` or `--ws-auth signed-bearer-token`"
);
}
None
}
};
Ok(AppServerWebsocketAuthSettings { config })
}
}
pub(crate) fn policy_from_settings(
settings: &AppServerWebsocketAuthSettings,
) -> io::Result<WebsocketAuthPolicy> {
let mode = match settings.config.as_ref() {
Some(AppServerWebsocketAuthConfig::CapabilityToken { token_file }) => {
let token = read_trimmed_secret(token_file.as_ref())?;
Some(WebsocketAuthMode::CapabilityToken {
token_sha256: sha256_digest(token.as_bytes()),
})
}
Some(AppServerWebsocketAuthConfig::SignedBearerToken {
shared_secret_file,
issuer,
audience,
max_clock_skew_seconds,
}) => {
let shared_secret = read_trimmed_secret(shared_secret_file.as_ref())?.into_bytes();
validate_signed_bearer_secret(shared_secret_file.as_ref(), &shared_secret)?;
let max_clock_skew_seconds = i64::try_from(*max_clock_skew_seconds).map_err(|_| {
io::Error::new(
ErrorKind::InvalidInput,
"websocket auth clock skew must fit in a signed 64-bit integer",
)
})?;
Some(WebsocketAuthMode::SignedBearerToken {
shared_secret,
issuer: issuer.clone(),
audience: audience.clone(),
max_clock_skew_seconds,
})
}
None => None,
};
Ok(WebsocketAuthPolicy { mode })
}
pub(crate) fn should_warn_about_unauthenticated_non_loopback_listener(
bind_address: SocketAddr,
policy: &WebsocketAuthPolicy,
) -> bool {
!bind_address.ip().is_loopback() && policy.mode.is_none()
}
pub(crate) fn authorize_upgrade(
headers: &HeaderMap,
policy: &WebsocketAuthPolicy,
) -> Result<(), WebsocketAuthError> {
let Some(mode) = policy.mode.as_ref() else {
return Ok(());
};
let token = bearer_token_from_headers(headers)?;
match mode {
WebsocketAuthMode::CapabilityToken { token_sha256 } => {
let actual_sha256 = sha256_digest(token.as_bytes());
if constant_time_eq_32(token_sha256, &actual_sha256) {
Ok(())
} else {
Err(unauthorized("invalid websocket bearer token"))
}
}
WebsocketAuthMode::SignedBearerToken {
shared_secret,
issuer,
audience,
max_clock_skew_seconds,
} => verify_signed_bearer_token(
token,
shared_secret,
issuer.as_deref(),
audience.as_deref(),
*max_clock_skew_seconds,
),
}
}
fn verify_signed_bearer_token(
token: &str,
shared_secret: &[u8],
issuer: Option<&str>,
audience: Option<&str>,
max_clock_skew_seconds: i64,
) -> Result<(), WebsocketAuthError> {
let claims = decode_jwt_claims(token, shared_secret)?;
validate_jwt_claims(&claims, issuer, audience, max_clock_skew_seconds)
}
fn decode_jwt_claims(token: &str, shared_secret: &[u8]) -> Result<JwtClaims, WebsocketAuthError> {
let mut validation = Validation::new(Algorithm::HS256);
validation.required_spec_claims.clear();
validation.validate_exp = false;
validation.validate_nbf = false;
validation.validate_aud = false;
decode::<JwtClaims>(token, &DecodingKey::from_secret(shared_secret), &validation)
.map(|token_data| token_data.claims)
.map_err(|_| unauthorized("invalid websocket jwt"))
}
fn validate_jwt_claims(
claims: &JwtClaims,
issuer: Option<&str>,
audience: Option<&str>,
max_clock_skew_seconds: i64,
) -> Result<(), WebsocketAuthError> {
let now = OffsetDateTime::now_utc().unix_timestamp();
if now > claims.exp.saturating_add(max_clock_skew_seconds) {
return Err(unauthorized("expired websocket jwt"));
}
if let Some(nbf) = claims.nbf
&& now < nbf.saturating_sub(max_clock_skew_seconds)
{
return Err(unauthorized("websocket jwt is not valid yet"));
}
if let Some(expected_issuer) = issuer
&& claims.iss.as_deref() != Some(expected_issuer)
{
return Err(unauthorized("websocket jwt issuer mismatch"));
}
if let Some(expected_audience) = audience
&& !audience_matches(claims.aud.as_ref(), expected_audience)
{
return Err(unauthorized("websocket jwt audience mismatch"));
}
Ok(())
}
fn audience_matches(audience: Option<&JwtAudienceClaim>, expected_audience: &str) -> bool {
match audience {
Some(JwtAudienceClaim::Single(actual)) => actual == expected_audience,
Some(JwtAudienceClaim::Multiple(actual)) => {
actual.iter().any(|audience| audience == expected_audience)
}
None => false,
}
}
fn bearer_token_from_headers(headers: &HeaderMap) -> Result<&str, WebsocketAuthError> {
let raw_header = headers
.get(AUTHORIZATION)
.ok_or_else(|| unauthorized("missing websocket bearer token"))?;
let header = raw_header
.to_str()
.map_err(|_| unauthorized(INVALID_AUTHORIZATION_HEADER_MESSAGE))?;
let Some((scheme, token)) = header.split_once(' ') else {
return Err(unauthorized(INVALID_AUTHORIZATION_HEADER_MESSAGE));
};
if !scheme.eq_ignore_ascii_case("Bearer") {
return Err(unauthorized(INVALID_AUTHORIZATION_HEADER_MESSAGE));
}
let token = token.trim();
if token.is_empty() {
return Err(unauthorized(INVALID_AUTHORIZATION_HEADER_MESSAGE));
}
Ok(token)
}
fn validate_signed_bearer_secret(path: &Path, shared_secret: &[u8]) -> io::Result<()> {
if shared_secret.len() < MIN_SIGNED_BEARER_SECRET_BYTES {
return Err(io::Error::new(
ErrorKind::InvalidInput,
format!(
"signed websocket bearer secret {} must be at least {MIN_SIGNED_BEARER_SECRET_BYTES} bytes",
path.display()
),
));
}
Ok(())
}
fn read_trimmed_secret(path: &std::path::Path) -> io::Result<String> {
let raw = std::fs::read_to_string(path).map_err(|err| {
io::Error::new(
err.kind(),
format!(
"failed to read websocket auth secret {}: {err}",
path.display()
),
)
})?;
let trimmed = raw.trim();
if trimmed.is_empty() {
return Err(io::Error::new(
ErrorKind::InvalidInput,
format!("websocket auth secret {} must not be empty", path.display()),
));
}
Ok(trimmed.to_string())
}
fn absolute_path_arg(flag_name: &str, path: PathBuf) -> anyhow::Result<AbsolutePathBuf> {
AbsolutePathBuf::try_from(path).with_context(|| format!("{flag_name} must be an absolute path"))
}
fn sha256_digest(input: &[u8]) -> [u8; 32] {
let mut digest = [0u8; 32];
digest.copy_from_slice(&Sha256::digest(input));
digest
}
fn unauthorized(message: &'static str) -> WebsocketAuthError {
WebsocketAuthError {
status_code: StatusCode::UNAUTHORIZED,
message,
}
}
#[cfg(test)]
mod tests {
use super::*;
use base64::Engine;
use base64::engine::general_purpose::URL_SAFE_NO_PAD;
use hmac::Hmac;
use hmac::Mac;
use serde_json::json;
type HmacSha256 = Hmac<Sha256>;
fn signed_token(shared_secret: &[u8], claims: serde_json::Value) -> String {
let header = URL_SAFE_NO_PAD.encode(br#"{"alg":"HS256","typ":"JWT"}"#);
let claims_segment = URL_SAFE_NO_PAD.encode(serde_json::to_vec(&claims).unwrap());
let payload = format!("{header}.{claims_segment}");
let mut mac = HmacSha256::new_from_slice(shared_secret).unwrap();
mac.update(payload.as_bytes());
let signature = URL_SAFE_NO_PAD.encode(mac.finalize().into_bytes());
format!("{payload}.{signature}")
}
#[test]
fn warns_about_unauthenticated_non_loopback_listener() {
let policy = WebsocketAuthPolicy::default();
assert!(should_warn_about_unauthenticated_non_loopback_listener(
"0.0.0.0:8765".parse().unwrap(),
&policy,
));
assert!(!should_warn_about_unauthenticated_non_loopback_listener(
"127.0.0.1:8765".parse().unwrap(),
&policy,
));
assert!(!should_warn_about_unauthenticated_non_loopback_listener(
"0.0.0.0:8765".parse().unwrap(),
&WebsocketAuthPolicy {
mode: Some(WebsocketAuthMode::CapabilityToken {
token_sha256: [0u8; 32],
}),
},
));
}
#[test]
fn capability_token_args_require_token_file() {
let err = AppServerWebsocketAuthArgs {
ws_auth: Some(WebsocketAuthCliMode::CapabilityToken),
..Default::default()
}
.try_into_settings()
.expect_err("capability-token mode should require a token file");
assert!(
err.to_string().contains("--ws-token-file"),
"unexpected error: {err}"
);
}
#[test]
fn signed_bearer_args_require_mode_when_mode_specific_flags_are_set() {
let err = AppServerWebsocketAuthArgs {
ws_shared_secret_file: Some(PathBuf::from("/tmp/secret")),
..Default::default()
}
.try_into_settings()
.expect_err("mode-specific flags should require --ws-auth");
assert!(
err.to_string().contains("websocket auth flags require"),
"unexpected error: {err}"
);
}
#[test]
fn signed_bearer_args_default_clock_skew_and_trim_optional_claims() {
let settings = AppServerWebsocketAuthArgs {
ws_auth: Some(WebsocketAuthCliMode::SignedBearerToken),
ws_shared_secret_file: Some(PathBuf::from("/tmp/secret")),
ws_issuer: Some(" issuer ".to_string()),
ws_audience: Some(" ".to_string()),
..Default::default()
}
.try_into_settings()
.expect("signed bearer args should parse");
assert_eq!(
settings,
AppServerWebsocketAuthSettings {
config: Some(AppServerWebsocketAuthConfig::SignedBearerToken {
shared_secret_file: AbsolutePathBuf::from_absolute_path("/tmp/secret")
.expect("absolute path"),
issuer: Some("issuer".to_string()),
audience: None,
max_clock_skew_seconds: DEFAULT_MAX_CLOCK_SKEW_SECONDS,
}),
}
);
}
#[test]
fn signed_bearer_token_verification_rejects_tampering() {
let shared_secret = b"0123456789abcdef0123456789abcdef";
let token = signed_token(
shared_secret,
json!({
"exp": OffsetDateTime::now_utc().unix_timestamp() + 60,
}),
);
let tampered = token.replace(".eyJleHAi", ".eyJleHBi");
let err = verify_signed_bearer_token(&tampered, shared_secret, None, None, 30)
.expect_err("tampered jwt should fail");
assert_eq!(err.status_code(), StatusCode::UNAUTHORIZED);
}
#[test]
fn signed_bearer_token_verification_accepts_valid_token() {
let shared_secret = b"0123456789abcdef0123456789abcdef";
let token = signed_token(
shared_secret,
json!({
"exp": OffsetDateTime::now_utc().unix_timestamp() + 60,
"iss": "issuer",
"aud": "audience",
}),
);
verify_signed_bearer_token(&token, shared_secret, Some("issuer"), Some("audience"), 30)
.expect("valid signed token should verify");
}
#[test]
fn signed_bearer_token_verification_accepts_multiple_audiences() {
let shared_secret = b"0123456789abcdef0123456789abcdef";
let token = signed_token(
shared_secret,
json!({
"exp": OffsetDateTime::now_utc().unix_timestamp() + 60,
"aud": ["other-audience", "audience"],
}),
);
verify_signed_bearer_token(&token, shared_secret, None, Some("audience"), 30)
.expect("jwt audience arrays should verify");
}
#[test]
fn signed_bearer_token_verification_rejects_alg_none_tokens() {
let claims_segment = URL_SAFE_NO_PAD.encode(
serde_json::to_vec(&json!({
"exp": OffsetDateTime::now_utc().unix_timestamp() + 60,
}))
.unwrap(),
);
let header_segment = URL_SAFE_NO_PAD.encode(br#"{"alg":"none","typ":"JWT"}"#);
let token = format!("{header_segment}.{claims_segment}.");
let err =
verify_signed_bearer_token(&token, b"0123456789abcdef0123456789abcdef", None, None, 30)
.expect_err("alg=none jwt should be rejected");
assert_eq!(err.status_code(), StatusCode::UNAUTHORIZED);
}
#[test]
fn signed_bearer_token_verification_rejects_missing_exp() {
let shared_secret = b"0123456789abcdef0123456789abcdef";
let token = signed_token(
shared_secret,
json!({
"iss": "issuer",
}),
);
let err = verify_signed_bearer_token(&token, shared_secret, None, None, 30)
.expect_err("jwt without exp should be rejected");
assert_eq!(err.status_code(), StatusCode::UNAUTHORIZED);
}
#[test]
fn validate_signed_bearer_secret_rejects_short_secret() {
let err = validate_signed_bearer_secret(Path::new("/tmp/secret"), b"too-short")
.expect_err("short shared secret should be rejected");
assert_eq!(err.kind(), ErrorKind::InvalidInput);
assert!(
err.to_string().contains("must be at least 32 bytes"),
"unexpected error: {err}"
);
}
}
@@ -2,6 +2,8 @@ use anyhow::Context;
use anyhow::Result;
use anyhow::bail;
use app_test_support::create_mock_responses_server_sequence_unchecked;
use base64::Engine;
use base64::engine::general_purpose::URL_SAFE_NO_PAD;
use codex_app_server_protocol::ClientInfo;
use codex_app_server_protocol::InitializeParams;
use codex_app_server_protocol::JSONRPCError;
@@ -12,12 +14,16 @@ use codex_app_server_protocol::JSONRPCResponse;
use codex_app_server_protocol::RequestId;
use futures::SinkExt;
use futures::StreamExt;
use hmac::Hmac;
use hmac::Mac;
use reqwest::StatusCode;
use serde_json::json;
use sha2::Sha256;
use std::net::SocketAddr;
use std::path::Path;
use std::process::Stdio;
use tempfile::TempDir;
use time::OffsetDateTime;
use tokio::io::AsyncBufReadExt;
use tokio::io::BufReader;
use tokio::process::Child;
@@ -29,15 +35,17 @@ use tokio::time::timeout;
use tokio_tungstenite::MaybeTlsStream;
use tokio_tungstenite::WebSocketStream;
use tokio_tungstenite::connect_async;
use tokio_tungstenite::tungstenite::Error as WebSocketError;
use tokio_tungstenite::tungstenite::Error as WsError;
use tokio_tungstenite::tungstenite::Message as WebSocketMessage;
use tokio_tungstenite::tungstenite::client::IntoClientRequest;
use tokio_tungstenite::tungstenite::http::HeaderValue;
use tokio_tungstenite::tungstenite::http::header::AUTHORIZATION;
use tokio_tungstenite::tungstenite::http::header::ORIGIN;
pub(super) const DEFAULT_READ_TIMEOUT: Duration = Duration::from_secs(10);
pub(super) type WsClient = WebSocketStream<MaybeTlsStream<tokio::net::TcpStream>>;
type HmacSha256 = Hmac<Sha256>;
#[tokio::test]
async fn websocket_transport_routes_per_connection_handshake_and_responses() -> Result<()> {
@@ -112,46 +120,26 @@ async fn websocket_transport_serves_health_endpoints_on_same_listener() -> Resul
}
#[tokio::test]
async fn websocket_transport_rejects_requests_with_origin_header() -> Result<()> {
async fn websocket_transport_rejects_browser_origin_without_auth() -> Result<()> {
let server = create_mock_responses_server_sequence_unchecked(Vec::new()).await;
let codex_home = TempDir::new()?;
create_config_toml(codex_home.path(), &server.uri(), "never")?;
let (mut process, bind_addr) = spawn_websocket_server(codex_home.path()).await?;
let client = reqwest::Client::new();
let deadline = Instant::now() + Duration::from_secs(10);
let healthz = loop {
match client
.get(format!("http://{bind_addr}/healthz"))
.header(ORIGIN.as_str(), "https://example.com")
.send()
.await
.with_context(|| format!("failed to GET http://{bind_addr}/healthz with Origin header"))
{
Ok(response) => break response,
Err(err) => {
if Instant::now() >= deadline {
bail!("failed to GET http://{bind_addr}/healthz with Origin header: {err}");
}
sleep(Duration::from_millis(50)).await;
}
}
};
assert_eq!(healthz.status(), StatusCode::FORBIDDEN);
let mut ws = connect_websocket(bind_addr).await?;
send_initialize_request(&mut ws, 1, "ws_loopback_client").await?;
let init = read_response_for_id(&mut ws, 1).await?;
assert_eq!(init.id, RequestId::Integer(1));
drop(ws);
let url = format!("ws://{bind_addr}");
let mut request = url.into_client_request()?;
request
.headers_mut()
.insert(ORIGIN, HeaderValue::from_static("https://example.com"));
match connect_async(request).await {
Err(WebSocketError::Http(response)) => {
assert_eq!(response.status(), StatusCode::FORBIDDEN);
}
Ok(_) => bail!("expected websocket handshake with Origin header to be rejected"),
Err(err) => bail!("expected HTTP rejection for Origin header, got {err}"),
}
assert_websocket_connect_rejected_with_headers(
bind_addr,
None,
Some("https://evil.example"),
StatusCode::FORBIDDEN,
)
.await?;
process
.kill()
@@ -160,12 +148,205 @@ async fn websocket_transport_rejects_requests_with_origin_header() -> Result<()>
Ok(())
}
#[tokio::test]
async fn websocket_transport_rejects_missing_and_invalid_capability_tokens() -> Result<()> {
let server = create_mock_responses_server_sequence_unchecked(Vec::new()).await;
let codex_home = TempDir::new()?;
let token_file = codex_home.path().join("app-server-token");
std::fs::write(&token_file, "super-secret-token\n")?;
create_config_toml(codex_home.path(), &server.uri(), "never")?;
let auth_args = vec![
"--ws-auth".to_string(),
"capability-token".to_string(),
"--ws-token-file".to_string(),
token_file.display().to_string(),
];
let (mut process, bind_addr) =
spawn_websocket_server_with_args(codex_home.path(), "ws://127.0.0.1:0", &auth_args).await?;
assert_websocket_connect_rejected(bind_addr, None).await?;
assert_websocket_connect_rejected(bind_addr, Some("wrong-token")).await?;
let mut ws = connect_websocket_with_bearer(bind_addr, Some("super-secret-token")).await?;
send_initialize_request(&mut ws, 1, "ws_auth_client").await?;
let init = read_response_for_id(&mut ws, 1).await?;
assert_eq!(init.id, RequestId::Integer(1));
process
.kill()
.await
.context("failed to stop websocket app-server process")?;
Ok(())
}
#[tokio::test]
async fn websocket_transport_verifies_signed_short_lived_bearer_tokens() -> Result<()> {
let server = create_mock_responses_server_sequence_unchecked(Vec::new()).await;
let codex_home = TempDir::new()?;
let shared_secret_file = codex_home.path().join("app-server-signing-secret");
let shared_secret = "0123456789abcdef0123456789abcdef";
std::fs::write(&shared_secret_file, format!("{shared_secret}\n"))?;
create_config_toml(codex_home.path(), &server.uri(), "never")?;
let auth_args = vec![
"--ws-auth".to_string(),
"signed-bearer-token".to_string(),
"--ws-shared-secret-file".to_string(),
shared_secret_file.display().to_string(),
"--ws-issuer".to_string(),
"codex-enroller".to_string(),
"--ws-audience".to_string(),
"codex-app-server".to_string(),
"--ws-max-clock-skew-seconds".to_string(),
"1".to_string(),
];
let (mut process, bind_addr) =
spawn_websocket_server_with_args(codex_home.path(), "ws://127.0.0.1:0", &auth_args).await?;
let expired_token = signed_bearer_token(
shared_secret.as_bytes(),
json!({
"exp": OffsetDateTime::now_utc().unix_timestamp() - 30,
"iss": "codex-enroller",
"aud": "codex-app-server",
}),
)?;
assert_websocket_connect_rejected(bind_addr, Some(expired_token.as_str())).await?;
let malformed_token = "not-a-jwt";
assert_websocket_connect_rejected(bind_addr, Some(malformed_token)).await?;
let not_yet_valid_token = signed_bearer_token(
shared_secret.as_bytes(),
json!({
"exp": OffsetDateTime::now_utc().unix_timestamp() + 60,
"nbf": OffsetDateTime::now_utc().unix_timestamp() + 30,
"iss": "codex-enroller",
"aud": "codex-app-server",
}),
)?;
assert_websocket_connect_rejected(bind_addr, Some(not_yet_valid_token.as_str())).await?;
let wrong_issuer_token = signed_bearer_token(
shared_secret.as_bytes(),
json!({
"exp": OffsetDateTime::now_utc().unix_timestamp() + 60,
"iss": "someone-else",
"aud": "codex-app-server",
}),
)?;
assert_websocket_connect_rejected(bind_addr, Some(wrong_issuer_token.as_str())).await?;
let wrong_audience_token = signed_bearer_token(
shared_secret.as_bytes(),
json!({
"exp": OffsetDateTime::now_utc().unix_timestamp() + 60,
"iss": "codex-enroller",
"aud": "wrong-audience",
}),
)?;
assert_websocket_connect_rejected(bind_addr, Some(wrong_audience_token.as_str())).await?;
let wrong_signature_token = signed_bearer_token(
b"fedcba9876543210fedcba9876543210",
json!({
"exp": OffsetDateTime::now_utc().unix_timestamp() + 60,
"iss": "codex-enroller",
"aud": "codex-app-server",
}),
)?;
assert_websocket_connect_rejected(bind_addr, Some(wrong_signature_token.as_str())).await?;
let valid_token = signed_bearer_token(
shared_secret.as_bytes(),
json!({
"exp": OffsetDateTime::now_utc().unix_timestamp() + 60,
"iss": "codex-enroller",
"aud": "codex-app-server",
}),
)?;
let mut ws = connect_websocket_with_bearer(bind_addr, Some(valid_token.as_str())).await?;
send_initialize_request(&mut ws, 1, "ws_signed_auth_client").await?;
let init = read_response_for_id(&mut ws, 1).await?;
assert_eq!(init.id, RequestId::Integer(1));
process
.kill()
.await
.context("failed to stop websocket app-server process")?;
Ok(())
}
#[tokio::test]
async fn websocket_transport_rejects_short_signed_bearer_secret_configuration() -> Result<()> {
let server = create_mock_responses_server_sequence_unchecked(Vec::new()).await;
let codex_home = TempDir::new()?;
let shared_secret_file = codex_home.path().join("app-server-signing-secret");
std::fs::write(&shared_secret_file, "too-short\n")?;
create_config_toml(codex_home.path(), &server.uri(), "never")?;
let output = run_websocket_server_to_completion_with_args(
codex_home.path(),
"ws://127.0.0.1:0",
&[
"--ws-auth".to_string(),
"signed-bearer-token".to_string(),
"--ws-shared-secret-file".to_string(),
shared_secret_file.display().to_string(),
],
)
.await?;
assert!(
!output.status.success(),
"short shared secret should fail websocket server startup"
);
let stderr = String::from_utf8(output.stderr).context("stderr should be valid utf-8")?;
assert!(
stderr.contains("must be at least 32 bytes"),
"unexpected stderr: {stderr}"
);
Ok(())
}
#[tokio::test]
async fn websocket_transport_allows_unauthenticated_non_loopback_startup_by_default() -> Result<()>
{
let server = create_mock_responses_server_sequence_unchecked(Vec::new()).await;
let codex_home = TempDir::new()?;
create_config_toml(codex_home.path(), &server.uri(), "never")?;
let (mut process, bind_addr) =
spawn_websocket_server_with_args(codex_home.path(), "ws://0.0.0.0:0", &[]).await?;
let mut ws = connect_websocket(bind_addr).await?;
send_initialize_request(&mut ws, 1, "ws_non_loopback_default_client").await?;
let init = read_response_for_id(&mut ws, 1).await?;
assert_eq!(init.id, RequestId::Integer(1));
process
.kill()
.await
.context("failed to stop websocket app-server process")?;
Ok(())
}
pub(super) async fn spawn_websocket_server(codex_home: &Path) -> Result<(Child, SocketAddr)> {
spawn_websocket_server_with_args(codex_home, "ws://127.0.0.1:0", &[]).await
}
pub(super) async fn spawn_websocket_server_with_args(
codex_home: &Path,
listen_url: &str,
extra_args: &[String],
) -> Result<(Child, SocketAddr)> {
let program = codex_utils_cargo_bin::cargo_bin("codex-app-server")
.context("should find app-server binary")?;
let mut cmd = Command::new(program);
cmd.arg("--listen")
.arg("ws://127.0.0.1:0")
.arg(listen_url)
.args(extra_args)
.stdin(Stdio::null())
.stdout(Stdio::null())
.stderr(Stdio::piped())
@@ -230,10 +411,18 @@ pub(super) async fn spawn_websocket_server(codex_home: &Path) -> Result<(Child,
}
pub(super) async fn connect_websocket(bind_addr: SocketAddr) -> Result<WsClient> {
let url = format!("ws://{bind_addr}");
connect_websocket_with_bearer(bind_addr, None).await
}
pub(super) async fn connect_websocket_with_bearer(
bind_addr: SocketAddr,
bearer_token: Option<&str>,
) -> Result<WsClient> {
let url = format!("ws://{}", connectable_bind_addr(bind_addr));
let request = websocket_request(url.as_str(), bearer_token, None)?;
let deadline = Instant::now() + Duration::from_secs(10);
loop {
match connect_async(&url).await {
match connect_async(request.clone()).await {
Ok((stream, _response)) => return Ok(stream),
Err(err) => {
if Instant::now() >= deadline {
@@ -245,23 +434,83 @@ pub(super) async fn connect_websocket(bind_addr: SocketAddr) -> Result<WsClient>
}
}
async fn assert_websocket_connect_rejected(
bind_addr: SocketAddr,
bearer_token: Option<&str>,
) -> Result<()> {
assert_websocket_connect_rejected_with_headers(
bind_addr,
bearer_token,
None,
StatusCode::UNAUTHORIZED,
)
.await
}
async fn assert_websocket_connect_rejected_with_headers(
bind_addr: SocketAddr,
bearer_token: Option<&str>,
origin: Option<&str>,
expected_status: StatusCode,
) -> Result<()> {
let url = format!("ws://{}", connectable_bind_addr(bind_addr));
let request = websocket_request(url.as_str(), bearer_token, origin)?;
match connect_async(request).await {
Ok((_stream, response)) => {
bail!(
"expected websocket handshake rejection, got {}",
response.status()
)
}
Err(WsError::Http(response)) => {
assert_eq!(response.status(), expected_status);
Ok(())
}
Err(err) => bail!("expected http rejection during websocket handshake: {err}"),
}
}
async fn run_websocket_server_to_completion_with_args(
codex_home: &Path,
listen_url: &str,
extra_args: &[String],
) -> Result<std::process::Output> {
let program = codex_utils_cargo_bin::cargo_bin("codex-app-server")
.context("should find app-server binary")?;
let mut cmd = Command::new(program);
cmd.arg("--listen")
.arg(listen_url)
.args(extra_args)
.stdin(Stdio::null())
.stdout(Stdio::null())
.stderr(Stdio::piped())
.env("CODEX_HOME", codex_home)
.env("RUST_LOG", "debug");
timeout(Duration::from_secs(10), cmd.output())
.await
.context("timed out waiting for websocket app-server to exit")?
.context("failed to run websocket app-server")
}
async fn http_get(
client: &reqwest::Client,
bind_addr: SocketAddr,
path: &str,
) -> Result<reqwest::Response> {
let connectable_bind_addr = connectable_bind_addr(bind_addr);
let deadline = Instant::now() + Duration::from_secs(10);
loop {
match client
.get(format!("http://{bind_addr}{path}"))
.get(format!("http://{connectable_bind_addr}{path}"))
.send()
.await
.with_context(|| format!("failed to GET http://{bind_addr}{path}"))
.with_context(|| format!("failed to GET http://{connectable_bind_addr}{path}"))
{
Ok(response) => return Ok(response),
Err(err) => {
if Instant::now() >= deadline {
bail!("failed to GET http://{bind_addr}{path}: {err}");
bail!("failed to GET http://{connectable_bind_addr}{path}: {err}");
}
sleep(Duration::from_millis(50)).await;
}
@@ -269,6 +518,30 @@ async fn http_get(
}
}
fn websocket_request(
url: &str,
bearer_token: Option<&str>,
origin: Option<&str>,
) -> Result<tokio_tungstenite::tungstenite::http::Request<()>> {
let mut request = url
.into_client_request()
.context("failed to create websocket request")?;
if let Some(bearer_token) = bearer_token {
request.headers_mut().insert(
AUTHORIZATION,
HeaderValue::from_str(&format!("Bearer {bearer_token}"))
.context("invalid bearer token header")?,
);
}
if let Some(origin) = origin {
request.headers_mut().insert(
ORIGIN,
HeaderValue::from_str(origin).context("invalid origin header")?,
);
}
Ok(request)
}
pub(super) async fn send_initialize_request(
stream: &mut WsClient,
id: i64,
@@ -459,3 +732,25 @@ stream_max_retries = 0
),
)
}
fn connectable_bind_addr(bind_addr: SocketAddr) -> SocketAddr {
match bind_addr {
SocketAddr::V4(addr) if addr.ip().is_unspecified() => {
SocketAddr::from(([127, 0, 0, 1], addr.port()))
}
SocketAddr::V6(addr) if addr.ip().is_unspecified() => {
SocketAddr::from(([0, 0, 0, 0, 0, 0, 0, 1], addr.port()))
}
_ => bind_addr,
}
}
fn signed_bearer_token(shared_secret: &[u8], claims: serde_json::Value) -> Result<String> {
let header_segment = URL_SAFE_NO_PAD.encode(br#"{"alg":"HS256","typ":"JWT"}"#);
let claims_segment = URL_SAFE_NO_PAD.encode(serde_json::to_vec(&claims)?);
let payload = format!("{header_segment}.{claims_segment}");
let mut mac = HmacSha256::new_from_slice(shared_secret).context("failed to create hmac")?;
mac.update(payload.as_bytes());
let signature = URL_SAFE_NO_PAD.encode(mac.finalize().into_bytes());
Ok(format!("{payload}.{signature}"))
}
+120 -42
View File
@@ -353,6 +353,9 @@ struct AppServerCommand {
/// See https://developers.openai.com/codex/config-advanced/#metrics for more details.
#[arg(long = "analytics-default-enabled")]
analytics_default_enabled: bool,
#[command(flatten)]
auth: codex_app_server::AppServerWebsocketAuthArgs,
}
#[derive(Debug, clap::Subcommand)]
@@ -643,49 +646,59 @@ async fn cli_main(arg0_paths: Arg0DispatchPaths) -> anyhow::Result<()> {
prepend_config_flags(&mut mcp_cli.config_overrides, root_config_overrides.clone());
mcp_cli.run().await?;
}
Some(Subcommand::AppServer(app_server_cli)) => match app_server_cli.subcommand {
None => {
reject_remote_mode_for_subcommand(root_remote.as_deref(), "app-server")?;
let transport = app_server_cli.listen;
codex_app_server::run_main_with_transport(
arg0_paths.clone(),
root_config_overrides,
codex_core::config_loader::LoaderOverrides::default(),
app_server_cli.analytics_default_enabled,
transport,
codex_protocol::protocol::SessionSource::VSCode,
)
.await?;
Some(Subcommand::AppServer(app_server_cli)) => {
let AppServerCommand {
subcommand,
listen,
analytics_default_enabled,
auth,
} = app_server_cli;
match subcommand {
None => {
reject_remote_mode_for_subcommand(root_remote.as_deref(), "app-server")?;
let transport = listen;
let auth = auth.try_into_settings()?;
codex_app_server::run_main_with_transport(
arg0_paths.clone(),
root_config_overrides,
codex_core::config_loader::LoaderOverrides::default(),
analytics_default_enabled,
transport,
codex_protocol::protocol::SessionSource::VSCode,
auth,
)
.await?;
}
Some(AppServerSubcommand::GenerateTs(gen_cli)) => {
reject_remote_mode_for_subcommand(
root_remote.as_deref(),
"app-server generate-ts",
)?;
let options = codex_app_server_protocol::GenerateTsOptions {
experimental_api: gen_cli.experimental,
..Default::default()
};
codex_app_server_protocol::generate_ts_with_options(
&gen_cli.out_dir,
gen_cli.prettier.as_deref(),
options,
)?;
}
Some(AppServerSubcommand::GenerateJsonSchema(gen_cli)) => {
reject_remote_mode_for_subcommand(
root_remote.as_deref(),
"app-server generate-json-schema",
)?;
codex_app_server_protocol::generate_json_with_experimental(
&gen_cli.out_dir,
gen_cli.experimental,
)?;
}
Some(AppServerSubcommand::GenerateInternalJsonSchema(gen_cli)) => {
codex_app_server_protocol::generate_internal_json_schema(&gen_cli.out_dir)?;
}
}
Some(AppServerSubcommand::GenerateTs(gen_cli)) => {
reject_remote_mode_for_subcommand(
root_remote.as_deref(),
"app-server generate-ts",
)?;
let options = codex_app_server_protocol::GenerateTsOptions {
experimental_api: gen_cli.experimental,
..Default::default()
};
codex_app_server_protocol::generate_ts_with_options(
&gen_cli.out_dir,
gen_cli.prettier.as_deref(),
options,
)?;
}
Some(AppServerSubcommand::GenerateJsonSchema(gen_cli)) => {
reject_remote_mode_for_subcommand(
root_remote.as_deref(),
"app-server generate-json-schema",
)?;
codex_app_server_protocol::generate_json_with_experimental(
&gen_cli.out_dir,
gen_cli.experimental,
)?;
}
Some(AppServerSubcommand::GenerateInternalJsonSchema(gen_cli)) => {
codex_app_server_protocol::generate_internal_json_schema(&gen_cli.out_dir)?;
}
},
}
#[cfg(target_os = "macos")]
Some(Subcommand::App(app_cli)) => {
reject_remote_mode_for_subcommand(root_remote.as_deref(), "app")?;
@@ -1701,6 +1714,71 @@ mod tests {
assert!(parse_result.is_err());
}
#[test]
fn app_server_capability_token_flags_parse() {
let app_server = app_server_from_args(
[
"codex",
"app-server",
"--ws-auth",
"capability-token",
"--ws-token-file",
"/tmp/codex-token",
]
.as_ref(),
);
assert_eq!(
app_server.auth.ws_auth,
Some(codex_app_server::WebsocketAuthCliMode::CapabilityToken)
);
assert_eq!(
app_server.auth.ws_token_file,
Some(PathBuf::from("/tmp/codex-token"))
);
}
#[test]
fn app_server_signed_bearer_flags_parse() {
let app_server = app_server_from_args(
[
"codex",
"app-server",
"--ws-auth",
"signed-bearer-token",
"--ws-shared-secret-file",
"/tmp/codex-secret",
"--ws-issuer",
"issuer",
"--ws-audience",
"audience",
"--ws-max-clock-skew-seconds",
"9",
]
.as_ref(),
);
assert_eq!(
app_server.auth.ws_auth,
Some(codex_app_server::WebsocketAuthCliMode::SignedBearerToken)
);
assert_eq!(
app_server.auth.ws_shared_secret_file,
Some(PathBuf::from("/tmp/codex-secret"))
);
assert_eq!(app_server.auth.ws_issuer.as_deref(), Some("issuer"));
assert_eq!(app_server.auth.ws_audience.as_deref(), Some("audience"));
assert_eq!(app_server.auth.ws_max_clock_skew_seconds, Some(9));
}
#[test]
fn app_server_rejects_removed_insecure_non_loopback_flag() {
let parse_result = MultitoolCli::try_parse_from([
"codex",
"app-server",
"--allow-unauthenticated-non-loopback-ws",
]);
assert!(parse_result.is_err());
}
#[test]
fn features_enable_parses_feature_name() {
let cli = MultitoolCli::try_parse_from(["codex", "features", "enable", "unified_exec"])
+1
View File
@@ -14,6 +14,7 @@ codex-protocol = { workspace = true }
codex-utils-absolute-path = { workspace = true }
futures = { workspace = true, features = ["alloc", "std"] }
multimap = { workspace = true }
schemars = { workspace = true }
serde = { workspace = true, features = ["derive"] }
serde_json = { workspace = true }
serde_path_to_error = { workspace = true }
+7
View File
@@ -5,7 +5,9 @@ mod diagnostics;
mod fingerprint;
mod merge;
mod overrides;
mod project_root_markers;
mod requirements_exec_policy;
mod skills_config;
mod state;
pub const CONFIG_TOML_FILE: &str = "config.toml";
@@ -46,12 +48,17 @@ pub use diagnostics::io_error_from_config_error;
pub use fingerprint::version_for_toml;
pub use merge::merge_toml_values;
pub use overrides::build_cli_overrides_layer;
pub use project_root_markers::default_project_root_markers;
pub use project_root_markers::project_root_markers_from_config;
pub use requirements_exec_policy::RequirementsExecPolicy;
pub use requirements_exec_policy::RequirementsExecPolicyDecisionToml;
pub use requirements_exec_policy::RequirementsExecPolicyParseError;
pub use requirements_exec_policy::RequirementsExecPolicyPatternTokenToml;
pub use requirements_exec_policy::RequirementsExecPolicyPrefixRuleToml;
pub use requirements_exec_policy::RequirementsExecPolicyToml;
pub use skills_config::BundledSkillsConfig;
pub use skills_config::SkillConfig;
pub use skills_config::SkillsConfig;
pub use state::ConfigLayerEntry;
pub use state::ConfigLayerStack;
pub use state::ConfigLayerStackOrdering;
@@ -0,0 +1,50 @@
use std::io;
use toml::Value as TomlValue;
const DEFAULT_PROJECT_ROOT_MARKERS: &[&str] = &[".git"];
/// Reads `project_root_markers` from a merged `config.toml` [toml::Value].
///
/// Invariants:
/// - If `project_root_markers` is not specified, returns `Ok(None)`.
/// - If `project_root_markers` is specified, returns `Ok(Some(markers))` where
/// `markers` is a `Vec<String>` (including `Ok(Some(Vec::new()))` for an
/// empty array, which indicates that root detection should be disabled).
/// - Returns an error if `project_root_markers` is specified but is not an
/// array of strings.
pub fn project_root_markers_from_config(config: &TomlValue) -> io::Result<Option<Vec<String>>> {
let Some(table) = config.as_table() else {
return Ok(None);
};
let Some(markers_value) = table.get("project_root_markers") else {
return Ok(None);
};
let TomlValue::Array(entries) = markers_value else {
return Err(io::Error::new(
io::ErrorKind::InvalidData,
"project_root_markers must be an array of strings",
));
};
if entries.is_empty() {
return Ok(Some(Vec::new()));
}
let mut markers = Vec::new();
for entry in entries {
let Some(marker) = entry.as_str() else {
return Err(io::Error::new(
io::ErrorKind::InvalidData,
"project_root_markers must be an array of strings",
));
};
markers.push(marker.to_string());
}
Ok(Some(markers))
}
pub fn default_project_root_markers() -> Vec<String> {
DEFAULT_PROJECT_ROOT_MARKERS
.iter()
.map(ToString::to_string)
.collect()
}
+53
View File
@@ -0,0 +1,53 @@
//! Skill-related configuration types shared across crates.
use codex_utils_absolute_path::AbsolutePathBuf;
use schemars::JsonSchema;
use serde::Deserialize;
use serde::Serialize;
const fn default_enabled() -> bool {
true
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema)]
#[schemars(deny_unknown_fields)]
pub struct SkillConfig {
/// Path-based selector.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub path: Option<AbsolutePathBuf>,
/// Name-based selector.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub name: Option<String>,
pub enabled: bool,
}
#[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq, Eq, JsonSchema)]
#[schemars(deny_unknown_fields)]
pub struct SkillsConfig {
#[serde(default, skip_serializing_if = "Option::is_none")]
pub bundled: Option<BundledSkillsConfig>,
#[serde(default, skip_serializing_if = "Vec::is_empty")]
pub config: Vec<SkillConfig>,
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema)]
#[schemars(deny_unknown_fields)]
pub struct BundledSkillsConfig {
#[serde(default = "default_enabled")]
pub enabled: bool,
}
impl Default for BundledSkillsConfig {
fn default() -> Self {
Self { enabled: true }
}
}
impl TryFrom<toml::Value> for SkillsConfig {
type Error = toml::de::Error;
fn try_from(value: toml::Value) -> Result<Self, Self::Error> {
SkillsConfig::deserialize(value)
}
}
+15
View File
@@ -0,0 +1,15 @@
load("//:defs.bzl", "codex_rust_crate")
codex_rust_crate(
name = "core-skills",
crate_name = "codex_core_skills",
compile_data = glob(
include = ["**"],
exclude = [
"**/* *",
"BUILD.bazel",
"Cargo.toml",
],
allow_empty = True,
),
)
+40
View File
@@ -0,0 +1,40 @@
[package]
edition.workspace = true
license.workspace = true
name = "codex-core-skills"
version.workspace = true
[lib]
doctest = false
name = "codex_core_skills"
path = "src/lib.rs"
[lints]
workspace = true
[dependencies]
anyhow = { workspace = true }
codex-analytics = { workspace = true }
codex-app-server-protocol = { workspace = true }
codex-config = { workspace = true }
codex-instructions = { workspace = true }
codex-login = { workspace = true }
codex-otel = { workspace = true }
codex-protocol = { workspace = true }
codex-skills = { workspace = true }
codex-utils-absolute-path = { workspace = true }
codex-utils-plugins = { workspace = true }
dirs = { workspace = true }
dunce = { workspace = true }
serde = { workspace = true, features = ["derive"] }
serde_json = { workspace = true }
serde_yaml = { workspace = true }
shlex = { workspace = true }
tokio = { workspace = true, features = ["fs", "macros", "rt"] }
toml = { workspace = true }
tracing = { workspace = true }
zip = { workspace = true }
[dev-dependencies]
pretty_assertions = { workspace = true }
tempfile = { workspace = true }
@@ -3,34 +3,32 @@ use std::path::Path;
use std::path::PathBuf;
use codex_app_server_protocol::ConfigLayerSource;
use codex_config::ConfigLayerStack;
use codex_config::ConfigLayerStackOrdering;
use codex_config::SkillConfig;
use codex_config::SkillsConfig;
use tracing::warn;
use crate::config::types::SkillConfig;
use crate::config::types::SkillsConfig;
use crate::config_loader::ConfigLayerStack;
use crate::config_loader::ConfigLayerStackOrdering;
use crate::skills::SkillMetadata;
use crate::SkillMetadata;
#[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
pub(crate) enum SkillConfigRuleSelector {
pub enum SkillConfigRuleSelector {
Name(String),
Path(PathBuf),
}
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub(crate) struct SkillConfigRule {
pub struct SkillConfigRule {
pub selector: SkillConfigRuleSelector,
pub enabled: bool,
}
#[derive(Debug, Clone, Default, PartialEq, Eq, Hash)]
pub(crate) struct SkillConfigRules {
pub struct SkillConfigRules {
pub entries: Vec<SkillConfigRule>,
}
pub(crate) fn skill_config_rules_from_stack(
config_layer_stack: &ConfigLayerStack,
) -> SkillConfigRules {
pub fn skill_config_rules_from_stack(config_layer_stack: &ConfigLayerStack) -> SkillConfigRules {
let mut entries = Vec::new();
for layer in config_layer_stack.get_layers(
ConfigLayerStackOrdering::LowestPrecedenceFirst,
@@ -71,7 +69,7 @@ pub(crate) fn skill_config_rules_from_stack(
SkillConfigRules { entries }
}
pub(crate) fn resolve_disabled_skill_paths(
pub fn resolve_disabled_skill_paths(
skills: &[SkillMetadata],
rules: &SkillConfigRules,
) -> HashSet<PathBuf> {
@@ -0,0 +1,30 @@
use crate::SkillMetadata;
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct SkillDependencyInfo {
pub skill_name: String,
pub name: String,
pub description: Option<String>,
}
pub fn collect_env_var_dependencies(
mentioned_skills: &[SkillMetadata],
) -> Vec<SkillDependencyInfo> {
let mut dependencies = Vec::new();
for skill in mentioned_skills {
let Some(skill_dependencies) = &skill.dependencies else {
continue;
};
for tool in &skill_dependencies.tools {
if tool.r#type != "env_var" || tool.value.is_empty() {
continue;
}
dependencies.push(SkillDependencyInfo {
skill_name: skill.name.clone(),
name: tool.value.clone(),
description: tool.description.clone(),
});
}
}
dependencies
}
@@ -2,26 +2,26 @@ use std::collections::HashMap;
use std::collections::HashSet;
use std::path::PathBuf;
use crate::instructions::SkillInstructions;
use crate::mention_syntax::TOOL_MENTION_SIGIL;
use crate::mentions::build_skill_name_counts;
use crate::skills::SkillMetadata;
use crate::SkillMetadata;
use crate::build_skill_name_counts;
use codex_analytics::AnalyticsEventsClient;
use codex_analytics::InvocationType;
use codex_analytics::SkillInvocation;
use codex_analytics::TrackEventsContext;
use codex_instructions::SkillInstructions;
use codex_otel::SessionTelemetry;
use codex_protocol::models::ResponseItem;
use codex_protocol::user_input::UserInput;
use codex_utils_plugins::mention_syntax::TOOL_MENTION_SIGIL;
use tokio::fs;
#[derive(Debug, Default)]
pub(crate) struct SkillInjections {
pub(crate) items: Vec<ResponseItem>,
pub(crate) warnings: Vec<String>,
pub struct SkillInjections {
pub items: Vec<ResponseItem>,
pub warnings: Vec<String>,
}
pub(crate) async fn build_skill_injections(
pub async fn build_skill_injections(
mentioned_skills: &[SkillMetadata],
otel: Option<&SessionTelemetry>,
analytics_client: &AnalyticsEventsClient,
@@ -97,7 +97,7 @@ fn emit_skill_injected_metric(
/// Complexity: `O(T + (N_s + N_t) * S)` time, `O(S + M)` space, where:
/// `S` = number of skills, `T` = total text length, `N_s` = number of structured skill inputs,
/// `N_t` = number of text inputs, `M` = max mentions parsed from a single text input.
pub(crate) fn collect_explicit_skill_mentions(
pub fn collect_explicit_skill_mentions(
inputs: &[UserInput],
skills: &[SkillMetadata],
disabled_paths: &HashSet<PathBuf>,
@@ -159,7 +159,7 @@ struct SkillSelectionContext<'a> {
connector_slug_counts: &'a HashMap<String, usize>,
}
pub(crate) struct ToolMentions<'a> {
pub struct ToolMentions<'a> {
names: HashSet<&'a str>,
paths: HashSet<&'a str>,
plain_names: HashSet<&'a str>,
@@ -170,17 +170,17 @@ impl<'a> ToolMentions<'a> {
self.names.is_empty() && self.paths.is_empty()
}
pub(crate) fn plain_names(&self) -> impl Iterator<Item = &'a str> + '_ {
pub fn plain_names(&self) -> impl Iterator<Item = &'a str> + '_ {
self.plain_names.iter().copied()
}
pub(crate) fn paths(&self) -> impl Iterator<Item = &'a str> + '_ {
pub fn paths(&self) -> impl Iterator<Item = &'a str> + '_ {
self.paths.iter().copied()
}
}
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub(crate) enum ToolMentionKind {
pub enum ToolMentionKind {
App,
Mcp,
Plugin,
@@ -194,7 +194,7 @@ const PLUGIN_PATH_PREFIX: &str = "plugin://";
const SKILL_PATH_PREFIX: &str = "skill://";
const SKILL_FILENAME: &str = "SKILL.md";
pub(crate) fn tool_kind_for_path(path: &str) -> ToolMentionKind {
pub fn tool_kind_for_path(path: &str) -> ToolMentionKind {
if path.starts_with(APP_PATH_PREFIX) {
ToolMentionKind::App
} else if path.starts_with(MCP_PATH_PREFIX) {
@@ -213,12 +213,12 @@ fn is_skill_filename(path: &str) -> bool {
file_name.eq_ignore_ascii_case(SKILL_FILENAME)
}
pub(crate) fn app_id_from_path(path: &str) -> Option<&str> {
pub fn app_id_from_path(path: &str) -> Option<&str> {
path.strip_prefix(APP_PATH_PREFIX)
.filter(|value| !value.is_empty())
}
pub(crate) fn plugin_config_name_from_path(path: &str) -> Option<&str> {
pub fn plugin_config_name_from_path(path: &str) -> Option<&str> {
path.strip_prefix(PLUGIN_PATH_PREFIX)
.filter(|value| !value.is_empty())
}
@@ -232,11 +232,11 @@ pub(crate) fn normalize_skill_path(path: &str) -> &str {
/// Supports explicit resource links in the form `[$tool-name](resource path)`. When a
/// resource path is present, it is captured for exact path matching while also tracking
/// the name for fallback matching.
pub(crate) fn extract_tool_mentions(text: &str) -> ToolMentions<'_> {
pub fn extract_tool_mentions(text: &str) -> ToolMentions<'_> {
extract_tool_mentions_with_sigil(text, TOOL_MENTION_SIGIL)
}
pub(crate) fn extract_tool_mentions_with_sigil(text: &str, sigil: char) -> ToolMentions<'_> {
pub fn extract_tool_mentions_with_sigil(text: &str, sigil: char) -> ToolMentions<'_> {
let text_bytes = text.as_bytes();
let mut mentioned_names: HashSet<&str> = HashSet::new();
let mut mentioned_paths: HashSet<&str> = HashSet::new();
@@ -2,13 +2,8 @@ use std::collections::HashMap;
use std::path::Path;
use std::path::PathBuf;
use crate::codex::Session;
use crate::codex::TurnContext;
use crate::skills::SkillLoadOutcome;
use crate::skills::SkillMetadata;
use codex_analytics::InvocationType;
use codex_analytics::SkillInvocation;
use codex_analytics::build_track_events_context;
use crate::SkillLoadOutcome;
use crate::SkillMetadata;
pub(crate) fn build_implicit_skill_path_indexes(
skills: Vec<SkillMetadata>,
@@ -31,14 +26,12 @@ pub(crate) fn build_implicit_skill_path_indexes(
(by_scripts_dir, by_skill_doc_path)
}
fn detect_implicit_skill_invocation_for_command(
pub fn detect_implicit_skill_invocation_for_command(
outcome: &SkillLoadOutcome,
turn_context: &TurnContext,
command: &str,
workdir: Option<&str>,
workdir: &Path,
) -> Option<SkillMetadata> {
let workdir = turn_context.resolve_path(workdir.map(str::to_owned));
let workdir = normalize_path(workdir.as_path());
let workdir = normalize_path(workdir);
let tokens = tokenize_command(command);
if let Some(candidate) = detect_skill_script_run(outcome, tokens.as_slice(), workdir.as_path())
@@ -46,82 +39,12 @@ fn detect_implicit_skill_invocation_for_command(
return Some(candidate);
}
if let Some(candidate) = detect_skill_doc_read(outcome, tokens.as_slice(), workdir.as_path()) {
return Some(candidate);
}
None
}
pub(crate) async fn maybe_emit_implicit_skill_invocation(
sess: &Session,
turn_context: &TurnContext,
command: &str,
workdir: Option<&str>,
) {
let Some(candidate) = detect_implicit_skill_invocation_for_command(
&turn_context.turn_skills.outcome,
turn_context,
command,
workdir,
) else {
return;
};
let invocation = SkillInvocation {
skill_name: candidate.name,
skill_scope: candidate.scope,
skill_path: candidate.path_to_skills_md,
invocation_type: InvocationType::Implicit,
};
let skill_scope = match invocation.skill_scope {
codex_protocol::protocol::SkillScope::User => "user",
codex_protocol::protocol::SkillScope::Repo => "repo",
codex_protocol::protocol::SkillScope::System => "system",
codex_protocol::protocol::SkillScope::Admin => "admin",
};
let skill_path = invocation.skill_path.to_string_lossy();
let skill_name = invocation.skill_name.clone();
let seen_key = format!("{skill_scope}:{skill_path}:{skill_name}");
let inserted = {
let mut seen_skills = turn_context
.turn_skills
.implicit_invocation_seen_skills
.lock()
.await;
seen_skills.insert(seen_key)
};
if !inserted {
return;
}
turn_context.session_telemetry.counter(
"codex.skill.injected",
/*inc*/ 1,
&[
("status", "ok"),
("skill", skill_name.as_str()),
("invoke_type", "implicit"),
],
);
sess.services
.analytics_events_client
.track_skill_invocations(
build_track_events_context(
turn_context.model_info.slug.clone(),
sess.conversation_id.to_string(),
turn_context.sub_id.clone(),
),
vec![invocation],
);
detect_skill_doc_read(outcome, tokens.as_slice(), workdir.as_path())
}
fn tokenize_command(command: &str) -> Vec<String> {
shlex::split(command).unwrap_or_else(|| {
command
.split_whitespace()
.map(std::string::ToString::to_string)
.collect()
})
shlex::split(command)
.unwrap_or_else(|| command.split_whitespace().map(str::to_string).collect())
}
fn script_run_token(tokens: &[String]) -> Option<&str> {
@@ -137,12 +60,9 @@ fn script_run_token(tokens: &[String]) -> Option<&str> {
return None;
}
let mut script_token: Option<&str> = None;
let mut script_token = None;
for token in tokens.iter().skip(1) {
if token == "--" {
continue;
}
if token.starts_with('-') {
if token == "--" || token.starts_with('-') {
continue;
}
script_token = Some(token.as_str());
@@ -1,22 +1,22 @@
pub(crate) mod config_rules;
pub mod config_rules;
mod env_var_dependencies;
pub mod injection;
pub(crate) mod invocation_utils;
pub mod loader;
pub mod manager;
mod mention_counts;
pub mod model;
pub mod remote;
pub mod render;
pub mod system;
pub(crate) use env_var_dependencies::collect_env_var_dependencies;
pub(crate) use env_var_dependencies::resolve_skill_dependencies_for_turn;
pub(crate) use injection::SkillInjections;
pub(crate) use injection::build_skill_injections;
pub(crate) use injection::collect_explicit_skill_mentions;
pub use env_var_dependencies::SkillDependencyInfo;
pub use env_var_dependencies::collect_env_var_dependencies;
pub(crate) use invocation_utils::build_implicit_skill_path_indexes;
pub(crate) use invocation_utils::maybe_emit_implicit_skill_invocation;
pub use invocation_utils::detect_implicit_skill_invocation_for_command;
pub use manager::SkillsLoadInput;
pub use manager::SkillsManager;
pub use mention_counts::build_skill_name_counts;
pub use model::SkillError;
pub use model::SkillLoadOutcome;
pub use model::SkillMetadata;
@@ -1,19 +1,18 @@
use crate::config_loader::ConfigLayerStack;
use crate::config_loader::ConfigLayerStackOrdering;
use crate::config_loader::default_project_root_markers;
use crate::config_loader::merge_toml_values;
use crate::config_loader::project_root_markers_from_config;
use crate::plugins::plugin_namespace_for_skill_path;
use crate::skills::model::SkillDependencies;
use crate::skills::model::SkillError;
use crate::skills::model::SkillInterface;
use crate::skills::model::SkillLoadOutcome;
use crate::skills::model::SkillManagedNetworkOverride;
use crate::skills::model::SkillMetadata;
use crate::skills::model::SkillPolicy;
use crate::skills::model::SkillToolDependency;
use crate::skills::system::system_cache_root_dir;
use crate::model::SkillDependencies;
use crate::model::SkillError;
use crate::model::SkillInterface;
use crate::model::SkillLoadOutcome;
use crate::model::SkillManagedNetworkOverride;
use crate::model::SkillMetadata;
use crate::model::SkillPolicy;
use crate::model::SkillToolDependency;
use crate::system::system_cache_root_dir;
use codex_app_server_protocol::ConfigLayerSource;
use codex_config::ConfigLayerStack;
use codex_config::ConfigLayerStackOrdering;
use codex_config::default_project_root_markers;
use codex_config::merge_toml_values;
use codex_config::project_root_markers_from_config;
use codex_protocol::models::FileSystemPermissions;
use codex_protocol::models::MacOsSeatbeltProfileExtensions;
use codex_protocol::models::NetworkPermissions;
@@ -21,6 +20,7 @@ use codex_protocol::models::PermissionProfile;
use codex_protocol::protocol::Product;
use codex_protocol::protocol::SkillScope;
use codex_utils_absolute_path::AbsolutePathBufGuard;
use codex_utils_plugins::plugin_namespace_for_skill_path;
use dirs::home_dir;
use dunce::canonicalize as canonicalize_path;
use serde::Deserialize;
@@ -35,9 +35,6 @@ use std::path::PathBuf;
use toml::Value as TomlValue;
use tracing::error;
#[cfg(test)]
use crate::config::Config;
#[derive(Debug, Deserialize)]
struct SkillFrontmatter {
#[serde(default)]
@@ -176,12 +173,12 @@ impl fmt::Display for SkillParseError {
impl Error for SkillParseError {}
pub(crate) struct SkillRoot {
pub(crate) path: PathBuf,
pub(crate) scope: SkillScope,
pub struct SkillRoot {
pub path: PathBuf,
pub scope: SkillScope,
}
pub(crate) fn load_skills_from_roots<I>(roots: I) -> SkillLoadOutcome
pub fn load_skills_from_roots<I>(roots: I) -> SkillLoadOutcome
where
I: IntoIterator<Item = SkillRoot>,
{
@@ -1,14 +1,9 @@
use super::*;
use crate::config::ConfigBuilder;
use crate::config::ConfigOverrides;
use crate::config::ConfigToml;
use crate::config::ProjectConfig;
use crate::config_loader::ConfigLayerEntry;
use crate::config_loader::ConfigLayerStack;
use crate::config_loader::ConfigRequirements;
use crate::config_loader::ConfigRequirementsToml;
use codex_config::CONFIG_TOML_FILE;
use codex_protocol::config_types::TrustLevel;
use codex_config::ConfigLayerEntry;
use codex_config::ConfigLayerStack;
use codex_config::ConfigRequirements;
use codex_config::ConfigRequirementsToml;
use codex_protocol::models::FileSystemPermissions;
use codex_protocol::models::MacOsAutomationPermission;
use codex_protocol::models::MacOsContactsPermission;
@@ -19,53 +14,109 @@ use codex_protocol::protocol::Product;
use codex_protocol::protocol::SkillScope;
use codex_utils_absolute_path::AbsolutePathBuf;
use pretty_assertions::assert_eq;
use std::collections::HashMap;
use std::path::Path;
use tempfile::TempDir;
use toml::Value as TomlValue;
const REPO_ROOT_CONFIG_DIR_NAME: &str = ".codex";
async fn make_config(codex_home: &TempDir) -> Config {
struct TestConfig {
cwd: PathBuf,
config_layer_stack: ConfigLayerStack,
}
async fn make_config(codex_home: &TempDir) -> TestConfig {
make_config_for_cwd(codex_home, codex_home.path().to_path_buf()).await
}
async fn make_config_for_cwd(codex_home: &TempDir, cwd: PathBuf) -> Config {
let trust_root = cwd
.ancestors()
.find(|ancestor| ancestor.join(".git").exists())
.map(Path::to_path_buf)
.unwrap_or_else(|| cwd.clone());
fs::write(
codex_home.path().join(CONFIG_TOML_FILE),
toml::to_string(&ConfigToml {
projects: Some(HashMap::from([(
trust_root.to_string_lossy().to_string(),
ProjectConfig {
trust_level: Some(TrustLevel::Trusted),
},
)])),
..Default::default()
})
.expect("serialize config"),
)
.unwrap();
let harness_overrides = ConfigOverrides {
cwd: Some(cwd),
..Default::default()
};
ConfigBuilder::default()
.codex_home(codex_home.path().to_path_buf())
.harness_overrides(harness_overrides)
.build()
.await
.expect("defaults for test should always succeed")
fn config_file(path: PathBuf) -> AbsolutePathBuf {
AbsolutePathBuf::from_absolute_path(path).expect("config file path should be absolute")
}
fn load_skills_for_test(config: &Config) -> SkillLoadOutcome {
fn project_layers_for_cwd(cwd: &Path) -> Vec<ConfigLayerEntry> {
let cwd_dir = if cwd.is_dir() {
cwd.to_path_buf()
} else {
cwd.parent()
.expect("file cwd should have a parent directory")
.to_path_buf()
};
let project_root = cwd_dir
.ancestors()
.find(|ancestor| ancestor.join(".git").exists())
.unwrap_or(cwd_dir.as_path())
.to_path_buf();
let mut layers = cwd_dir
.ancestors()
.scan(false, |done, dir| {
if *done {
None
} else {
if dir == project_root {
*done = true;
}
Some(dir.to_path_buf())
}
})
.collect::<Vec<_>>();
layers.reverse();
layers
.into_iter()
.filter_map(|dir| {
let dot_codex = dir.join(REPO_ROOT_CONFIG_DIR_NAME);
dot_codex.is_dir().then(|| {
ConfigLayerEntry::new(
ConfigLayerSource::Project {
dot_codex_folder: AbsolutePathBuf::from_absolute_path(dot_codex)
.expect("project .codex path should be absolute"),
},
TomlValue::Table(toml::map::Map::new()),
)
})
})
.collect()
}
async fn make_config_for_cwd(codex_home: &TempDir, cwd: PathBuf) -> TestConfig {
let user_config_path = codex_home.path().join(CONFIG_TOML_FILE);
let system_config_path = codex_home.path().join("etc/codex/config.toml");
fs::create_dir_all(
system_config_path
.parent()
.expect("system config path should have a parent"),
)
.expect("create fake system config dir");
let mut layers = vec![
ConfigLayerEntry::new(
ConfigLayerSource::System {
file: config_file(system_config_path),
},
TomlValue::Table(toml::map::Map::new()),
),
ConfigLayerEntry::new(
ConfigLayerSource::User {
file: config_file(user_config_path),
},
TomlValue::Table(toml::map::Map::new()),
),
];
layers.extend(project_layers_for_cwd(&cwd));
TestConfig {
cwd,
config_layer_stack: ConfigLayerStack::new(
layers,
ConfigRequirements::default(),
ConfigRequirementsToml::default(),
)
.expect("valid config layer stack"),
}
}
fn load_skills_for_test(config: &TestConfig) -> SkillLoadOutcome {
// Keep unit tests hermetic by never scanning the real `$HOME/.agents/skills`.
super::load_skills_from_roots(super::skill_roots_with_home_dir(
&config.config_layer_stack,
@@ -5,61 +5,67 @@ use std::path::PathBuf;
use std::sync::Arc;
use std::sync::RwLock;
use codex_config::ConfigLayerStack;
use codex_protocol::protocol::Product;
use codex_protocol::protocol::SkillScope;
use codex_utils_absolute_path::AbsolutePathBuf;
use toml::Value as TomlValue;
use tracing::info;
use tracing::warn;
use crate::config::Config;
use crate::config::types::SkillsConfig;
use crate::config_loader::CloudRequirementsLoader;
use crate::config_loader::LoaderOverrides;
use crate::config_loader::load_config_layers_state;
use crate::plugins::PluginsManager;
use crate::skills::SkillLoadOutcome;
use crate::skills::build_implicit_skill_path_indexes;
use crate::skills::config_rules::SkillConfigRules;
use crate::skills::config_rules::resolve_disabled_skill_paths;
use crate::skills::config_rules::skill_config_rules_from_stack;
use crate::skills::loader::SkillRoot;
use crate::skills::loader::load_skills_from_roots;
use crate::skills::loader::skill_roots;
use crate::skills::system::install_system_skills;
use crate::skills::system::uninstall_system_skills;
use crate::SkillLoadOutcome;
use crate::build_implicit_skill_path_indexes;
use crate::config_rules::SkillConfigRules;
use crate::config_rules::resolve_disabled_skill_paths;
use crate::config_rules::skill_config_rules_from_stack;
use crate::loader::SkillRoot;
use crate::loader::load_skills_from_roots;
use crate::loader::skill_roots;
use crate::system::install_system_skills;
use crate::system::uninstall_system_skills;
use codex_config::SkillsConfig;
#[derive(Debug, Clone)]
pub struct SkillsLoadInput {
pub cwd: PathBuf,
pub effective_skill_roots: Vec<PathBuf>,
pub config_layer_stack: ConfigLayerStack,
pub bundled_skills_enabled: bool,
}
impl SkillsLoadInput {
pub fn new(
cwd: PathBuf,
effective_skill_roots: Vec<PathBuf>,
config_layer_stack: ConfigLayerStack,
bundled_skills_enabled: bool,
) -> Self {
Self {
cwd,
effective_skill_roots,
config_layer_stack,
bundled_skills_enabled,
}
}
}
pub struct SkillsManager {
codex_home: PathBuf,
plugins_manager: Arc<PluginsManager>,
restriction_product: Option<Product>,
cache_by_cwd: RwLock<HashMap<PathBuf, SkillLoadOutcome>>,
cache_by_config: RwLock<HashMap<ConfigSkillsCacheKey, SkillLoadOutcome>>,
}
impl SkillsManager {
pub fn new(
codex_home: PathBuf,
plugins_manager: Arc<PluginsManager>,
bundled_skills_enabled: bool,
) -> Self {
Self::new_with_restriction_product(
codex_home,
plugins_manager,
bundled_skills_enabled,
Some(Product::Codex),
)
pub fn new(codex_home: PathBuf, bundled_skills_enabled: bool) -> Self {
Self::new_with_restriction_product(codex_home, bundled_skills_enabled, Some(Product::Codex))
}
pub fn new_with_restriction_product(
codex_home: PathBuf,
plugins_manager: Arc<PluginsManager>,
bundled_skills_enabled: bool,
restriction_product: Option<Product>,
) -> Self {
let manager = Self {
codex_home,
plugins_manager,
restriction_product,
cache_by_cwd: RwLock::new(HashMap::new()),
cache_by_config: RwLock::new(HashMap::new()),
@@ -80,9 +86,9 @@ impl SkillsManager {
/// This path uses a cache keyed by the effective skill-relevant config state rather than just
/// cwd so role-local and session-local skill overrides cannot bleed across sessions that happen
/// to share a directory.
pub fn skills_for_config(&self, config: &Config) -> SkillLoadOutcome {
let roots = self.skill_roots_for_config(config);
let skill_config_rules = skill_config_rules_from_stack(&config.config_layer_stack);
pub fn skills_for_config(&self, input: &SkillsLoadInput) -> SkillLoadOutcome {
let roots = self.skill_roots_for_config(input);
let skill_config_rules = skill_config_rules_from_stack(&input.config_layer_stack);
let cache_key = config_skills_cache_key(&roots, &skill_config_rules);
if let Some(outcome) = self.cached_outcome_for_config(&cache_key) {
return outcome;
@@ -97,14 +103,13 @@ impl SkillsManager {
outcome
}
pub(crate) fn skill_roots_for_config(&self, config: &Config) -> Vec<SkillRoot> {
let loaded_plugins = self.plugins_manager.plugins_for_config(config);
pub fn skill_roots_for_config(&self, input: &SkillsLoadInput) -> Vec<SkillRoot> {
let mut roots = skill_roots(
&config.config_layer_stack,
&config.cwd,
loaded_plugins.effective_skill_roots(),
&input.config_layer_stack,
input.cwd.as_path(),
input.effective_skill_roots.clone(),
);
if !config.bundled_skills_enabled() {
if !input.bundled_skills_enabled {
roots.retain(|root| root.scope != SkillScope::System);
}
roots
@@ -112,74 +117,34 @@ impl SkillsManager {
pub async fn skills_for_cwd(
&self,
cwd: &Path,
config: &Config,
input: &SkillsLoadInput,
force_reload: bool,
) -> SkillLoadOutcome {
if !force_reload && let Some(outcome) = self.cached_outcome_for_cwd(cwd) {
if !force_reload && let Some(outcome) = self.cached_outcome_for_cwd(input.cwd.as_path()) {
return outcome;
}
self.skills_for_cwd_with_extra_user_roots(cwd, config, force_reload, &[])
self.skills_for_cwd_with_extra_user_roots(input, force_reload, &[])
.await
}
pub async fn skills_for_cwd_with_extra_user_roots(
&self,
cwd: &Path,
config: &Config,
input: &SkillsLoadInput,
force_reload: bool,
extra_user_roots: &[PathBuf],
) -> SkillLoadOutcome {
if !force_reload && let Some(outcome) = self.cached_outcome_for_cwd(cwd) {
if !force_reload && let Some(outcome) = self.cached_outcome_for_cwd(input.cwd.as_path()) {
return outcome;
}
let normalized_extra_user_roots = normalize_extra_user_roots(extra_user_roots);
let cwd_abs = match AbsolutePathBuf::try_from(cwd) {
Ok(cwd_abs) => cwd_abs,
Err(err) => {
return SkillLoadOutcome {
errors: vec![crate::skills::model::SkillError {
path: cwd.to_path_buf(),
message: err.to_string(),
}],
..Default::default()
};
}
};
let cli_overrides: Vec<(String, TomlValue)> = Vec::new();
let config_layer_stack = match load_config_layers_state(
&self.codex_home,
Some(cwd_abs),
&cli_overrides,
LoaderOverrides::default(),
CloudRequirementsLoader::default(),
)
.await
{
Ok(config_layer_stack) => config_layer_stack,
Err(err) => {
return SkillLoadOutcome {
errors: vec![crate::skills::model::SkillError {
path: cwd.to_path_buf(),
message: err.to_string(),
}],
..Default::default()
};
}
};
let loaded_plugins = self
.plugins_manager
.plugins_for_config_with_force_reload(config, force_reload);
let mut roots = skill_roots(
&config_layer_stack,
cwd,
loaded_plugins.effective_skill_roots(),
&input.config_layer_stack,
input.cwd.as_path(),
input.effective_skill_roots.clone(),
);
if !bundled_skills_enabled_from_stack(&config_layer_stack) {
if !bundled_skills_enabled_from_stack(&input.config_layer_stack) {
roots.retain(|root| root.scope != SkillScope::System);
}
roots.extend(
@@ -191,13 +156,13 @@ impl SkillsManager {
scope: SkillScope::User,
}),
);
let skill_config_rules = skill_config_rules_from_stack(&config_layer_stack);
let skill_config_rules = skill_config_rules_from_stack(&input.config_layer_stack);
let outcome = self.build_skill_outcome(roots, &skill_config_rules);
let mut cache = self
.cache_by_cwd
.write()
.unwrap_or_else(std::sync::PoisonError::into_inner);
cache.insert(cwd.to_path_buf(), outcome.clone());
cache.insert(input.cwd.clone(), outcome.clone());
outcome
}
@@ -206,7 +171,7 @@ impl SkillsManager {
roots: Vec<SkillRoot>,
skill_config_rules: &SkillConfigRules,
) -> SkillLoadOutcome {
let outcome = crate::skills::filter_skill_load_outcome_for_product(
let outcome = crate::filter_skill_load_outcome_for_product(
load_skills_from_roots(roots),
self.restriction_product,
);
@@ -261,8 +226,8 @@ struct ConfigSkillsCacheKey {
skill_config_rules: SkillConfigRules,
}
pub(crate) fn bundled_skills_enabled_from_stack(
config_layer_stack: &crate::config_loader::ConfigLayerStack,
pub fn bundled_skills_enabled_from_stack(
config_layer_stack: &codex_config::ConfigLayerStack,
) -> bool {
let effective_config = config_layer_stack.effective_config();
let Some(skills_value) = effective_config
@@ -1,15 +1,15 @@
use super::*;
use crate::config::ConfigBuilder;
use crate::config::ConfigOverrides;
use crate::config_loader::ConfigLayerEntry;
use crate::config_loader::ConfigLayerStack;
use crate::config_loader::ConfigRequirementsToml;
use crate::plugins::PluginsManager;
use crate::skills::SkillMetadata;
use crate::skills::config_rules::resolve_disabled_skill_paths;
use crate::skills::config_rules::skill_config_rules_from_stack;
use crate::SkillMetadata;
use crate::config_rules::resolve_disabled_skill_paths;
use crate::config_rules::skill_config_rules_from_stack;
use codex_app_server_protocol::ConfigLayerSource;
use codex_config::CONFIG_TOML_FILE;
use codex_config::ConfigLayerEntry;
use codex_config::ConfigLayerStack;
use codex_config::ConfigRequirementsToml;
use codex_utils_absolute_path::AbsolutePathBuf;
use pretty_assertions::assert_eq;
use std::collections::HashSet;
use std::fs;
use std::path::PathBuf;
use tempfile::TempDir;
@@ -64,6 +64,77 @@ fn test_skill(name: &str, path: PathBuf) -> SkillMetadata {
}
}
fn user_config_layer(codex_home: &TempDir, config_toml: &str) -> ConfigLayerEntry {
let config_path = AbsolutePathBuf::try_from(codex_home.path().join(CONFIG_TOML_FILE))
.expect("user config path should be absolute");
ConfigLayerEntry::new(
ConfigLayerSource::User { file: config_path },
toml::from_str(config_toml).expect("user layer toml"),
)
}
fn config_stack(codex_home: &TempDir, user_config_toml: &str) -> ConfigLayerStack {
ConfigLayerStack::new(
vec![user_config_layer(codex_home, user_config_toml)],
Default::default(),
ConfigRequirementsToml::default(),
)
.expect("valid config layer stack")
}
fn config_stack_with_session_flags(
codex_home: &TempDir,
user_config_toml: &str,
session_flags_toml: &str,
) -> ConfigLayerStack {
ConfigLayerStack::new(
vec![
user_config_layer(codex_home, user_config_toml),
ConfigLayerEntry::new(
ConfigLayerSource::SessionFlags,
toml::from_str(session_flags_toml).expect("session layer toml"),
),
],
Default::default(),
ConfigRequirementsToml::default(),
)
.expect("valid config layer stack")
}
fn path_toggle_config(path: &std::path::Path, enabled: bool) -> String {
format!(
r#"[[skills.config]]
path = "{}"
enabled = {enabled}
"#,
path.display()
)
}
fn name_toggle_config(name: &str, enabled: bool) -> String {
format!(
r#"[[skills.config]]
name = "{name}"
enabled = {enabled}
"#
)
}
fn skills_for_config_with_stack(
skills_manager: &SkillsManager,
cwd: &TempDir,
config_layer_stack: &ConfigLayerStack,
effective_skill_roots: &[PathBuf],
) -> SkillLoadOutcome {
let skills_input = SkillsLoadInput::new(
cwd.path().to_path_buf(),
effective_skill_roots.to_vec(),
config_layer_stack.clone(),
bundled_skills_enabled_from_stack(config_layer_stack),
);
skills_manager.skills_for_config(&skills_input)
}
#[test]
fn new_with_disabled_bundled_skills_removes_stale_cached_system_skills() {
let codex_home = tempfile::tempdir().expect("tempdir");
@@ -72,9 +143,7 @@ fn new_with_disabled_bundled_skills_removes_stale_cached_system_skills() {
fs::write(stale_system_skill_dir.join("SKILL.md"), "# stale\n")
.expect("write stale system skill");
let plugins_manager = Arc::new(PluginsManager::new(codex_home.path().to_path_buf()));
let _skills_manager =
SkillsManager::new(codex_home.path().to_path_buf(), plugins_manager, false);
let _skills_manager = SkillsManager::new(codex_home.path().to_path_buf(), false);
assert!(
!codex_home.path().join("skills/.system").exists(),
@@ -86,22 +155,11 @@ fn new_with_disabled_bundled_skills_removes_stale_cached_system_skills() {
async fn skills_for_config_reuses_cache_for_same_effective_config() {
let codex_home = tempfile::tempdir().expect("tempdir");
let cwd = tempfile::tempdir().expect("tempdir");
let cfg = ConfigBuilder::default()
.codex_home(codex_home.path().to_path_buf())
.harness_overrides(ConfigOverrides {
cwd: Some(cwd.path().to_path_buf()),
..Default::default()
})
.build()
.await
.expect("defaults for test should always succeed");
let plugins_manager = Arc::new(PluginsManager::new(codex_home.path().to_path_buf()));
let skills_manager = SkillsManager::new(codex_home.path().to_path_buf(), plugins_manager, true);
let config_layer_stack = config_stack(&codex_home, "");
let skills_manager = SkillsManager::new(codex_home.path().to_path_buf(), true);
write_user_skill(&codex_home, "a", "skill-a", "from a");
let outcome1 = skills_manager.skills_for_config(&cfg);
let outcome1 = skills_for_config_with_stack(&skills_manager, &cwd, &config_layer_stack, &[]);
assert!(
outcome1.skills.iter().any(|s| s.name == "skill-a"),
"expected skill-a to be discovered"
@@ -110,7 +168,7 @@ async fn skills_for_config_reuses_cache_for_same_effective_config() {
// Write a new skill after the first call; the second call should reuse the config-aware cache
// entry because the effective skill config is unchanged.
write_user_skill(&codex_home, "b", "skill-b", "from b");
let outcome2 = skills_manager.skills_for_config(&cfg);
let outcome2 = skills_for_config_with_stack(&skills_manager, &cwd, &config_layer_stack, &[]);
assert_eq!(outcome2.errors, outcome1.errors);
assert_eq!(outcome2.skills, outcome1.skills);
}
@@ -127,39 +185,23 @@ async fn skills_for_config_disables_plugin_skills_by_name() {
"sample-search",
"search sample data",
);
fs::write(
codex_home.path().join(crate::config::CONFIG_TOML_FILE),
r#"[features]
plugins = true
[[skills.config]]
name = "sample:sample-search"
enabled = false
[plugins."sample@test"]
enabled = true
"#,
)
.expect("write config");
let config = ConfigBuilder::default()
.codex_home(codex_home.path().to_path_buf())
.harness_overrides(ConfigOverrides {
cwd: Some(cwd.path().to_path_buf()),
..Default::default()
})
.build()
.await
.expect("load config");
let plugins_manager = Arc::new(PluginsManager::new(codex_home.path().to_path_buf()));
let skills_manager = SkillsManager::new(
codex_home.path().to_path_buf(),
plugins_manager,
config.bundled_skills_enabled(),
let config_layer_stack = config_stack(
&codex_home,
&name_toggle_config("sample:sample-search", false),
);
let plugin_skill_root = skill_path
.parent()
.and_then(std::path::Path::parent)
.expect("plugin skill should live under a skills root")
.to_path_buf();
let skills_manager = SkillsManager::new(codex_home.path().to_path_buf(), true);
let outcome = skills_manager.skills_for_config(&config);
let outcome = skills_for_config_with_stack(
&skills_manager,
&cwd,
&config_layer_stack,
&[plugin_skill_root],
);
let skill = outcome
.skills
.iter()
@@ -182,27 +224,21 @@ async fn skills_for_cwd_reuses_cached_entry_even_when_entry_has_extra_roots() {
let codex_home = tempfile::tempdir().expect("tempdir");
let cwd = tempfile::tempdir().expect("tempdir");
let extra_root = tempfile::tempdir().expect("tempdir");
let config = ConfigBuilder::default()
.codex_home(codex_home.path().to_path_buf())
.harness_overrides(ConfigOverrides {
cwd: Some(cwd.path().to_path_buf()),
..Default::default()
})
.build()
.await
.expect("defaults for test should always succeed");
let plugins_manager = Arc::new(PluginsManager::new(codex_home.path().to_path_buf()));
let skills_manager = SkillsManager::new(codex_home.path().to_path_buf(), plugins_manager, true);
let _ = skills_manager.skills_for_config(&config);
let config_layer_stack = config_stack(&codex_home, "");
let skills_manager = SkillsManager::new(codex_home.path().to_path_buf(), true);
let _ = skills_for_config_with_stack(&skills_manager, &cwd, &config_layer_stack, &[]);
write_user_skill(&extra_root, "x", "extra-skill", "from extra root");
let extra_root_path = extra_root.path().to_path_buf();
let base_input = SkillsLoadInput::new(
cwd.path().to_path_buf(),
Vec::new(),
config_layer_stack.clone(),
bundled_skills_enabled_from_stack(&config_layer_stack),
);
let outcome_with_extra = skills_manager
.skills_for_cwd_with_extra_user_roots(
cwd.path(),
&config,
&base_input,
true,
std::slice::from_ref(&extra_root_path),
)
@@ -222,9 +258,13 @@ async fn skills_for_cwd_reuses_cached_entry_even_when_entry_has_extra_roots() {
// The cwd-only API returns the current cached entry for this cwd, even when that entry
// was produced with extra roots.
let outcome_without_extra = skills_manager
.skills_for_cwd(cwd.path(), &config, false)
.await;
let base_input = SkillsLoadInput::new(
cwd.path().to_path_buf(),
Vec::new(),
config_layer_stack.clone(),
bundled_skills_enabled_from_stack(&config_layer_stack),
);
let outcome_without_extra = skills_manager.skills_for_cwd(&base_input, false).await;
assert_eq!(outcome_without_extra.skills, outcome_with_extra.skills);
assert_eq!(outcome_without_extra.errors, outcome_with_extra.errors);
}
@@ -240,29 +280,8 @@ async fn skills_for_config_excludes_bundled_skills_when_disabled_in_config() {
"---\nname: bundled-skill\ndescription: from bundled root\n---\n\n# Body\n",
)
.expect("write bundled skill");
fs::write(
codex_home.path().join(crate::config::CONFIG_TOML_FILE),
"[skills.bundled]\nenabled = false\n",
)
.expect("write config");
let config = ConfigBuilder::default()
.codex_home(codex_home.path().to_path_buf())
.harness_overrides(ConfigOverrides {
cwd: Some(cwd.path().to_path_buf()),
..Default::default()
})
.build()
.await
.expect("load config");
let plugins_manager = Arc::new(PluginsManager::new(codex_home.path().to_path_buf()));
let skills_manager = SkillsManager::new(
codex_home.path().to_path_buf(),
plugins_manager,
config.bundled_skills_enabled(),
);
let config_layer_stack = config_stack(&codex_home, "[skills.bundled]\nenabled = false\n");
let skills_manager = SkillsManager::new(codex_home.path().to_path_buf(), false);
// Recreate the cached bundled skill after startup cleanup so this assertion exercises
// root selection rather than relying on directory removal succeeding.
@@ -273,7 +292,7 @@ async fn skills_for_config_excludes_bundled_skills_when_disabled_in_config() {
)
.expect("rewrite bundled skill");
let outcome = skills_manager.skills_for_config(&config);
let outcome = skills_for_config_with_stack(&skills_manager, &cwd, &config_layer_stack, &[]);
assert!(
outcome
.skills
@@ -294,29 +313,23 @@ async fn skills_for_cwd_with_extra_roots_only_refreshes_on_force_reload() {
let cwd = tempfile::tempdir().expect("tempdir");
let extra_root_a = tempfile::tempdir().expect("tempdir");
let extra_root_b = tempfile::tempdir().expect("tempdir");
let config = ConfigBuilder::default()
.codex_home(codex_home.path().to_path_buf())
.harness_overrides(ConfigOverrides {
cwd: Some(cwd.path().to_path_buf()),
..Default::default()
})
.build()
.await
.expect("defaults for test should always succeed");
let plugins_manager = Arc::new(PluginsManager::new(codex_home.path().to_path_buf()));
let skills_manager = SkillsManager::new(codex_home.path().to_path_buf(), plugins_manager, true);
let _ = skills_manager.skills_for_config(&config);
let config_layer_stack = config_stack(&codex_home, "");
let skills_manager = SkillsManager::new(codex_home.path().to_path_buf(), true);
let _ = skills_for_config_with_stack(&skills_manager, &cwd, &config_layer_stack, &[]);
write_user_skill(&extra_root_a, "x", "extra-skill-a", "from extra root a");
write_user_skill(&extra_root_b, "x", "extra-skill-b", "from extra root b");
let extra_root_a_path = extra_root_a.path().to_path_buf();
let base_input = SkillsLoadInput::new(
cwd.path().to_path_buf(),
Vec::new(),
config_layer_stack.clone(),
bundled_skills_enabled_from_stack(&config_layer_stack),
);
let outcome_a = skills_manager
.skills_for_cwd_with_extra_user_roots(
cwd.path(),
&config,
&base_input,
true,
std::slice::from_ref(&extra_root_a_path),
)
@@ -337,8 +350,7 @@ async fn skills_for_cwd_with_extra_roots_only_refreshes_on_force_reload() {
let extra_root_b_path = extra_root_b.path().to_path_buf();
let outcome_b = skills_manager
.skills_for_cwd_with_extra_user_roots(
cwd.path(),
&config,
&base_input,
false,
std::slice::from_ref(&extra_root_b_path),
)
@@ -358,8 +370,7 @@ async fn skills_for_cwd_with_extra_roots_only_refreshes_on_force_reload() {
let outcome_reloaded = skills_manager
.skills_for_cwd_with_extra_user_roots(
cwd.path(),
&config,
&base_input,
true,
std::slice::from_ref(&extra_root_b_path),
)
@@ -399,25 +410,11 @@ fn disabled_paths_for_skills_allows_session_flags_to_override_user_layer() {
.expect("user config path should be absolute");
let user_layer = ConfigLayerEntry::new(
ConfigLayerSource::User { file: user_file },
toml::from_str(&format!(
r#"[[skills.config]]
path = "{}"
enabled = false
"#,
skill_path.display()
))
.expect("user layer toml"),
toml::from_str(&path_toggle_config(&skill_path, false)).expect("user layer toml"),
);
let session_layer = ConfigLayerEntry::new(
ConfigLayerSource::SessionFlags,
toml::from_str(&format!(
r#"[[skills.config]]
path = "{}"
enabled = true
"#,
skill_path.display()
))
.expect("session layer toml"),
toml::from_str(&path_toggle_config(&skill_path, true)).expect("session layer toml"),
);
let stack = ConfigLayerStack::new(
vec![user_layer, session_layer],
@@ -443,25 +440,11 @@ fn disabled_paths_for_skills_allows_session_flags_to_disable_user_enabled_skill(
.expect("user config path should be absolute");
let user_layer = ConfigLayerEntry::new(
ConfigLayerSource::User { file: user_file },
toml::from_str(&format!(
r#"[[skills.config]]
path = "{}"
enabled = true
"#,
skill_path.display()
))
.expect("user layer toml"),
toml::from_str(&path_toggle_config(&skill_path, true)).expect("user layer toml"),
);
let session_layer = ConfigLayerEntry::new(
ConfigLayerSource::SessionFlags,
toml::from_str(&format!(
r#"[[skills.config]]
path = "{}"
enabled = false
"#,
skill_path.display()
))
.expect("session layer toml"),
toml::from_str(&path_toggle_config(&skill_path, false)).expect("session layer toml"),
);
let stack = ConfigLayerStack::new(
vec![user_layer, session_layer],
@@ -487,13 +470,7 @@ fn disabled_paths_for_skills_disables_matching_name_selectors() {
.expect("user config path should be absolute");
let user_layer = ConfigLayerEntry::new(
ConfigLayerSource::User { file: user_file },
toml::from_str(
r#"[[skills.config]]
name = "github:yeet"
enabled = false
"#,
)
.expect("user layer toml"),
toml::from_str(&name_toggle_config("github:yeet", false)).expect("user layer toml"),
);
let stack = ConfigLayerStack::new(
vec![user_layer],
@@ -519,24 +496,11 @@ fn disabled_paths_for_skills_allows_name_selector_to_override_path_selector() {
.expect("user config path should be absolute");
let user_layer = ConfigLayerEntry::new(
ConfigLayerSource::User { file: user_file },
toml::from_str(&format!(
r#"[[skills.config]]
path = "{}"
enabled = false
"#,
skill_path.display()
))
.expect("user layer toml"),
toml::from_str(&path_toggle_config(&skill_path, false)).expect("user layer toml"),
);
let session_layer = ConfigLayerEntry::new(
ConfigLayerSource::SessionFlags,
toml::from_str(
r#"[[skills.config]]
name = "github:yeet"
enabled = true
"#,
)
.expect("session layer toml"),
toml::from_str(&name_toggle_config("github:yeet", true)).expect("session layer toml"),
);
let stack = ConfigLayerStack::new(
vec![user_layer, session_layer],
@@ -565,58 +529,20 @@ async fn skills_for_config_ignores_cwd_cache_when_session_flags_reenable_skill()
"---\nname: demo-skill\ndescription: demo description\n---\n\n# Body\n",
)
.expect("write skill");
fs::write(
codex_home.path().join(crate::config::CONFIG_TOML_FILE),
format!(
r#"[[skills.config]]
path = "{}"
enabled = false
"#,
skill_path.display()
),
)
.expect("write config");
let parent_config = ConfigBuilder::default()
.codex_home(codex_home.path().to_path_buf())
.harness_overrides(ConfigOverrides {
cwd: Some(cwd.path().to_path_buf()),
..Default::default()
})
.build()
.await
.expect("load parent config");
let role_path = codex_home.path().join("enable-role.toml");
fs::write(
&role_path,
format!(
r#"[[skills.config]]
path = "{}"
enabled = true
"#,
skill_path.display()
),
)
.expect("write role config");
let mut child_config = parent_config.clone();
child_config.agent_roles.insert(
"custom".to_string(),
crate::config::AgentRoleConfig {
description: None,
config_file: Some(role_path),
nickname_candidates: None,
},
let disabled_skill_config = path_toggle_config(&skill_path, false);
let enabled_skill_config = path_toggle_config(&skill_path, true);
let parent_stack = config_stack(&codex_home, &disabled_skill_config);
let child_stack =
config_stack_with_session_flags(&codex_home, &disabled_skill_config, &enabled_skill_config);
let skills_manager = SkillsManager::new(codex_home.path().to_path_buf(), true);
let parent_input = SkillsLoadInput::new(
cwd.path().to_path_buf(),
Vec::new(),
parent_stack.clone(),
bundled_skills_enabled_from_stack(&parent_stack),
);
crate::agent::role::apply_role_to_config(&mut child_config, Some("custom"))
.await
.expect("custom role should apply");
let plugins_manager = Arc::new(PluginsManager::new(codex_home.path().to_path_buf()));
let skills_manager = SkillsManager::new(codex_home.path().to_path_buf(), plugins_manager, true);
let parent_outcome = skills_manager
.skills_for_cwd(cwd.path(), &parent_config, true)
.await;
let parent_outcome = skills_manager.skills_for_cwd(&parent_input, true).await;
let parent_skill = parent_outcome
.skills
.iter()
@@ -624,7 +550,7 @@ enabled = true
.expect("demo skill should be discovered");
assert_eq!(parent_outcome.is_skill_enabled(parent_skill), false);
let child_outcome = skills_manager.skills_for_config(&child_config);
let child_outcome = skills_for_config_with_stack(&skills_manager, &cwd, &child_stack, &[]);
let child_skill = child_outcome
.skills
.iter()
@@ -0,0 +1,24 @@
use std::collections::HashMap;
use std::collections::HashSet;
use std::path::PathBuf;
use super::SkillMetadata;
/// Counts how often each skill name appears (exact and ASCII-lowercase), excluding disabled paths.
pub fn build_skill_name_counts(
skills: &[SkillMetadata],
disabled_paths: &HashSet<PathBuf>,
) -> (HashMap<String, usize>, HashMap<String, usize>) {
let mut exact_counts: HashMap<String, usize> = HashMap::new();
let mut lower_counts: HashMap<String, usize> = HashMap::new();
for skill in skills {
if disabled_paths.contains(&skill.path_to_skills_md) {
continue;
}
*exact_counts.entry(skill.name.clone()).or_insert(0) += 1;
*lower_counts
.entry(skill.name.to_ascii_lowercase())
.or_insert(0) += 1;
}
(exact_counts, lower_counts)
}
@@ -6,9 +6,8 @@ use std::path::Path;
use std::path::PathBuf;
use std::time::Duration;
use crate::auth::CodexAuth;
use crate::config::Config;
use crate::default_client::build_reqwest_client;
use codex_login::CodexAuth;
use codex_login::default_client::build_reqwest_client;
const REMOTE_SKILLS_API_TIMEOUT: Duration = Duration::from_secs(30);
@@ -88,13 +87,13 @@ struct RemoteSkill {
}
pub async fn list_remote_skills(
config: &Config,
chatgpt_base_url: String,
auth: Option<&CodexAuth>,
scope: RemoteSkillScope,
product_surface: RemoteSkillProductSurface,
enabled: Option<bool>,
) -> Result<Vec<RemoteSkillSummary>> {
let base_url = config.chatgpt_base_url.trim_end_matches('/');
let base_url = chatgpt_base_url.trim_end_matches('/');
let auth = ensure_chatgpt_auth(auth)?;
let url = format!("{base_url}/hazelnuts");
@@ -146,14 +145,15 @@ pub async fn list_remote_skills(
}
pub async fn export_remote_skill(
config: &Config,
chatgpt_base_url: String,
codex_home: PathBuf,
auth: Option<&CodexAuth>,
skill_id: &str,
) -> Result<RemoteSkillDownloadResult> {
let auth = ensure_chatgpt_auth(auth)?;
let client = build_reqwest_client();
let base_url = config.chatgpt_base_url.trim_end_matches('/');
let base_url = chatgpt_base_url.trim_end_matches('/');
let url = format!("{base_url}/hazelnuts/{skill_id}/export");
let mut request = client.get(&url).timeout(REMOTE_SKILLS_API_TIMEOUT);
@@ -181,7 +181,7 @@ pub async fn export_remote_skill(
anyhow::bail!("Downloaded remote skill payload is not a zip archive");
}
let output_dir = config.codex_home.join("skills").join(skill_id);
let output_dir = codex_home.join("skills").join(skill_id);
tokio::fs::create_dir_all(&output_dir)
.await
.context("Failed to create downloaded skills directory")?;
@@ -1,4 +1,4 @@
use crate::skills::model::SkillMetadata;
use crate::model::SkillMetadata;
use codex_protocol::protocol::SKILLS_INSTRUCTIONS_CLOSE_TAG;
use codex_protocol::protocol::SKILLS_INSTRUCTIONS_OPEN_TAG;
+1 -2
View File
@@ -35,11 +35,11 @@ codex-async-utils = { workspace = true }
codex-code-mode = { workspace = true }
codex-connectors = { workspace = true }
codex-config = { workspace = true }
codex-core-skills = { workspace = true }
codex-exec-server = { workspace = true }
codex-features = { workspace = true }
codex-login = { workspace = true }
codex-shell-command = { workspace = true }
codex-skills = { workspace = true }
codex-execpolicy = { workspace = true }
codex-git-utils = { workspace = true }
codex-hooks = { workspace = true }
@@ -93,7 +93,6 @@ rmcp = { workspace = true, default-features = false, features = [
schemars = { workspace = true }
serde = { workspace = true, features = ["derive"] }
serde_json = { workspace = true }
serde_yaml = { workspace = true }
sha1 = { workspace = true }
shlex = { workspace = true }
similar = { workspace = true }
+7 -3
View File
@@ -1,9 +1,10 @@
use super::*;
use crate::SkillsManager;
use crate::config::CONFIG_TOML_FILE;
use crate::config::ConfigBuilder;
use crate::config_loader::ConfigLayerStackOrdering;
use crate::plugins::PluginsManager;
use crate::skills::SkillsManager;
use crate::skills_load_input_from_config;
use codex_protocol::config_types::ReasoningSummary;
use codex_protocol::config_types::Verbosity;
use codex_protocol::openai_models::ReasoningEffort;
@@ -629,8 +630,11 @@ enabled = false
.expect("custom role should apply");
let plugins_manager = Arc::new(PluginsManager::new(home.path().to_path_buf()));
let skills_manager = SkillsManager::new(home.path().to_path_buf(), plugins_manager, true);
let outcome = skills_manager.skills_for_config(&config);
let skills_manager = SkillsManager::new(home.path().to_path_buf(), true);
let plugin_outcome = plugins_manager.plugins_for_config(&config);
let effective_skill_roots = plugin_outcome.effective_skill_roots();
let skills_input = skills_load_input_from_config(&config, effective_skill_roots);
let outcome = skills_manager.skills_for_config(&skills_input);
let skill = outcome
.skills
.iter()
+1 -1
View File
@@ -12,7 +12,7 @@ pub(crate) fn render_apps_section(connectors: &[AppInfo]) -> Option<String> {
}
let body = format!(
"## Apps (Connectors)\nApps (Connectors) can be explicitly triggered in user messages in the format `[$app-name](app://{{connector_id}})`. Apps can also be implicitly triggered as long as the context suggests usage of available apps, the available apps will be listed by the `tool_search` tool.\nAn app is equivalent to a set of MCP tools within the `{CODEX_APPS_MCP_SERVER_NAME}` MCP.\nAn installed app's MCP tools are either provided to you already, or can be lazy-loaded through the `tool_search` tool.\nDo not additionally call list_mcp_resources or list_mcp_resource_templates for apps."
"## Apps (Connectors)\nApps (Connectors) can be explicitly triggered in user messages in the format `[$app-name](app://{{connector_id}})`. Apps can also be implicitly triggered as long as the context suggests usage of available apps.\nAn app is equivalent to a set of MCP tools within the `{CODEX_APPS_MCP_SERVER_NAME}` MCP.\nAn installed app's MCP tools are either provided to you already, or can be lazy-loaded through the `tool_search` tool. If `tool_search` is available, the apps that are searchable by `tools_search` will be listed by it.\nDo not additionally call list_mcp_resources or list_mcp_resource_templates for apps."
);
Some(format!(
"{APPS_INSTRUCTIONS_OPEN_TAG}\n{body}\n{APPS_INSTRUCTIONS_CLOSE_TAG}"
+90 -20
View File
@@ -37,8 +37,9 @@ use crate::realtime_conversation::handle_audio as handle_realtime_conversation_a
use crate::realtime_conversation::handle_close as handle_realtime_conversation_close;
use crate::realtime_conversation::handle_start as handle_realtime_conversation_start;
use crate::realtime_conversation::handle_text as handle_realtime_conversation_text;
use crate::render_skills_section;
use crate::rollout::session_index;
use crate::skills::render_skills_section;
use crate::skills_load_input_from_config;
use crate::stream_events_utils::HandleOutputCtx;
use crate::stream_events_utils::handle_non_tool_response_item;
use crate::stream_events_utils::handle_output_item_done;
@@ -51,6 +52,10 @@ use async_channel::Receiver;
use async_channel::Sender;
use chrono::Local;
use chrono::Utc;
use codex_analytics::AnalyticsEventsClient;
use codex_analytics::AppInvocation;
use codex_analytics::InvocationType;
use codex_analytics::build_track_events_context;
use codex_app_server_protocol::McpServerElicitationRequest;
use codex_app_server_protocol::McpServerElicitationRequestParams;
use codex_exec_server::Environment;
@@ -232,6 +237,14 @@ pub(crate) struct PreviousTurnSettings {
pub(crate) realtime_active: Option<bool>,
}
use crate::SkillError;
use crate::SkillInjections;
use crate::SkillLoadOutcome;
use crate::SkillMetadata;
use crate::SkillsManager;
use crate::build_skill_injections;
use crate::collect_env_var_dependencies;
use crate::collect_explicit_skill_mentions;
use crate::exec_policy::ExecPolicyUpdateError;
use crate::feedback_tags;
use crate::guardian::GuardianReviewSessionManager;
@@ -241,6 +254,9 @@ use crate::hook_runtime::record_additional_contexts;
use crate::hook_runtime::record_pending_input;
use crate::hook_runtime::run_pending_session_start_hooks;
use crate::hook_runtime::run_user_prompt_submit_hooks;
use crate::injection::ToolMentionKind;
use crate::injection::app_id_from_path;
use crate::injection::tool_kind_for_path;
use crate::instructions::UserInstructions;
use crate::mcp::CODEX_APPS_MCP_SERVER_NAME;
use crate::mcp::McpManager;
@@ -298,6 +314,7 @@ use crate::protocol::TokenUsage;
use crate::protocol::TokenUsageInfo;
use crate::protocol::TurnDiffEvent;
use crate::protocol::WarningEvent;
use crate::resolve_skill_dependencies_for_turn;
use crate::rollout::RolloutRecorder;
use crate::rollout::RolloutRecorderParams;
use crate::rollout::map_session_init_error;
@@ -306,18 +323,6 @@ use crate::rollout::policy::EventPersistenceMode;
use crate::session_startup_prewarm::SessionStartupPrewarmHandle;
use crate::shell;
use crate::shell_snapshot::ShellSnapshot;
use crate::skills::SkillError;
use crate::skills::SkillInjections;
use crate::skills::SkillLoadOutcome;
use crate::skills::SkillMetadata;
use crate::skills::SkillsManager;
use crate::skills::build_skill_injections;
use crate::skills::collect_env_var_dependencies;
use crate::skills::collect_explicit_skill_mentions;
use crate::skills::injection::ToolMentionKind;
use crate::skills::injection::app_id_from_path;
use crate::skills::injection::tool_kind_for_path;
use crate::skills::resolve_skill_dependencies_for_turn;
use crate::skills_watcher::SkillsWatcher;
use crate::skills_watcher::SkillsWatcherEvent;
use crate::state::ActiveTurn;
@@ -476,7 +481,10 @@ impl Codex {
let (tx_sub, rx_sub) = async_channel::bounded(SUBMISSION_CHANNEL_CAPACITY);
let (tx_event, rx_event) = async_channel::unbounded();
let loaded_skills = skills_manager.skills_for_config(&config);
let plugin_outcome = plugins_manager.plugins_for_config(&config);
let effective_skill_roots = plugin_outcome.effective_skill_roots();
let skills_input = skills_load_input_from_config(&config, effective_skill_roots);
let loaded_skills = skills_manager.skills_for_config(&skills_input);
for err in &loaded_skills.errors {
error!(
@@ -2508,10 +2516,16 @@ impl Session {
&per_turn_config,
)
.await;
let plugin_outcome = self
.services
.plugins_manager
.plugins_for_config(&per_turn_config);
let effective_skill_roots = plugin_outcome.effective_skill_roots();
let skills_input = skills_load_input_from_config(&per_turn_config, effective_skill_roots);
let skills_outcome = Arc::new(
self.services
.skills_manager
.skills_for_config(&per_turn_config),
.skills_for_config(&skills_input),
);
let mut turn_context: TurnContext = Self::make_turn_context(
self.conversation_id,
@@ -4566,8 +4580,14 @@ mod handlers {
use crate::codex::SessionSettingsUpdate;
use crate::codex::SteerInputError;
use crate::SkillError;
use crate::codex::spawn_review_thread;
use crate::config::Config;
use crate::config_loader::CloudRequirementsLoader;
use crate::config_loader::LoaderOverrides;
use crate::config_loader::load_config_layers_state;
use codex_features::Feature;
use codex_utils_absolute_path::AbsolutePathBuf;
use crate::mcp::auth::compute_auth_statuses;
use crate::mcp::collect_mcp_snapshot_from_manager;
@@ -5003,11 +5023,64 @@ mod handlers {
};
let skills_manager = &sess.services.skills_manager;
let plugins_manager = &sess.services.plugins_manager;
let config = sess.get_config().await;
let codex_home = sess.codex_home().await;
let mut skills = Vec::new();
let empty_cli_overrides: &[(String, toml::Value)] = &[];
for cwd in cwds {
let cwd_abs = match AbsolutePathBuf::try_from(cwd.as_path()) {
Ok(path) => path,
Err(err) => {
let message = err.to_string();
let cwd_for_entry = cwd.clone();
skills.push(SkillsListEntry {
cwd: cwd_for_entry.clone(),
skills: Vec::new(),
errors: super::errors_to_info(&[SkillError {
path: cwd_for_entry,
message,
}]),
});
continue;
}
};
let config_layer_stack = match load_config_layers_state(
&codex_home,
Some(cwd_abs),
empty_cli_overrides,
LoaderOverrides::default(),
CloudRequirementsLoader::default(),
)
.await
{
Ok(config_layer_stack) => config_layer_stack,
Err(err) => {
let message = err.to_string();
let cwd_for_entry = cwd.clone();
skills.push(SkillsListEntry {
cwd: cwd_for_entry.clone(),
skills: Vec::new(),
errors: super::errors_to_info(&[SkillError {
path: cwd_for_entry,
message,
}]),
});
continue;
}
};
let effective_skill_roots = plugins_manager.effective_skill_roots_for_layer_stack(
&config_layer_stack,
config.features.enabled(Feature::Plugins),
);
let skills_input = crate::SkillsLoadInput::new(
cwd.clone(),
effective_skill_roots,
config_layer_stack,
config.bundled_skills_enabled(),
);
let outcome = skills_manager
.skills_for_cwd(&cwd, config.as_ref(), force_reload)
.skills_for_cwd(&skills_input, force_reload)
.await;
let errors = super::errors_to_info(&outcome.errors);
let skills_metadata = super::skills_to_info(&outcome.skills, &outcome.disabled_paths);
@@ -6536,10 +6609,7 @@ pub(crate) async fn built_tools(
None
};
let auth = sess.services.auth_manager.auth().await;
let discoverable_tools = if apps_enabled
&& turn_context.tools_config.search_tool
&& turn_context.tools_config.tool_suggest
{
let discoverable_tools = if apps_enabled && turn_context.tools_config.tool_suggest {
if let Some(accessible_connectors) = accessible_connectors_with_enabled_state.as_ref() {
match connectors::list_tool_suggest_discoverable_tools_with_auth(
&turn_context.config,
+21 -18
View File
@@ -2341,7 +2341,10 @@ async fn new_default_turn_uses_config_aware_skills_for_role_overrides() {
let parent_outcome = session
.services
.skills_manager
.skills_for_cwd(&parent_config.cwd, &parent_config, true)
.skills_for_cwd(
&crate::skills_load_input_from_config(&parent_config, Vec::new()),
true,
)
.await;
let parent_skill = parent_outcome
.skills
@@ -2537,11 +2540,7 @@ async fn session_new_fails_when_zsh_fork_enabled_without_zsh_path() {
let (agent_status_tx, _agent_status_rx) = watch::channel(AgentStatus::PendingInit);
let plugins_manager = Arc::new(PluginsManager::new(config.codex_home.clone()));
let mcp_manager = Arc::new(McpManager::new(Arc::clone(&plugins_manager)));
let skills_manager = Arc::new(SkillsManager::new(
config.codex_home.clone(),
Arc::clone(&plugins_manager),
true,
));
let skills_manager = Arc::new(SkillsManager::new(config.codex_home.clone(), true));
let result = Session::new(
session_configuration,
Arc::clone(&config),
@@ -2642,11 +2641,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
let state = SessionState::new(session_configuration.clone());
let plugins_manager = Arc::new(PluginsManager::new(config.codex_home.clone()));
let mcp_manager = Arc::new(McpManager::new(Arc::clone(&plugins_manager)));
let skills_manager = Arc::new(SkillsManager::new(
config.codex_home.clone(),
Arc::clone(&plugins_manager),
true,
));
let skills_manager = Arc::new(SkillsManager::new(config.codex_home.clone(), true));
let network_approval = Arc::new(NetworkApprovalService::default());
let environment = Arc::new(
codex_exec_server::Environment::create(None)
@@ -2714,7 +2709,13 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
config.js_repl_node_module_dirs.clone(),
));
let skills_outcome = Arc::new(services.skills_manager.skills_for_config(&per_turn_config));
let plugin_outcome = services
.plugins_manager
.plugins_for_config(&per_turn_config);
let effective_skill_roots = plugin_outcome.effective_skill_roots();
let skills_input =
crate::skills_load_input_from_config(&per_turn_config, effective_skill_roots);
let skills_outcome = Arc::new(services.skills_manager.skills_for_config(&skills_input));
let turn_context = Session::make_turn_context(
conversation_id,
Some(Arc::clone(&auth_manager)),
@@ -3478,11 +3479,7 @@ pub(crate) async fn make_session_and_context_with_dynamic_tools_and_rx(
let state = SessionState::new(session_configuration.clone());
let plugins_manager = Arc::new(PluginsManager::new(config.codex_home.clone()));
let mcp_manager = Arc::new(McpManager::new(Arc::clone(&plugins_manager)));
let skills_manager = Arc::new(SkillsManager::new(
config.codex_home.clone(),
Arc::clone(&plugins_manager),
true,
));
let skills_manager = Arc::new(SkillsManager::new(config.codex_home.clone(), true));
let network_approval = Arc::new(NetworkApprovalService::default());
let environment = Arc::new(
codex_exec_server::Environment::create(None)
@@ -3550,7 +3547,13 @@ pub(crate) async fn make_session_and_context_with_dynamic_tools_and_rx(
config.js_repl_node_module_dirs.clone(),
));
let skills_outcome = Arc::new(services.skills_manager.skills_for_config(&per_turn_config));
let plugin_outcome = services
.plugins_manager
.plugins_for_config(&per_turn_config);
let effective_skill_roots = plugin_outcome.effective_skill_roots();
let skills_input =
crate::skills_load_input_from_config(&per_turn_config, effective_skill_roots);
let skills_outcome = Arc::new(services.skills_manager.skills_for_config(&skills_input));
let turn_context = Arc::new(Session::make_turn_context(
conversation_id,
Some(Arc::clone(&auth_manager)),
+1 -5
View File
@@ -429,11 +429,7 @@ async fn guardian_subagent_does_not_inherit_parent_exec_policy_rules() {
CollaborationModesConfig::default(),
));
let plugins_manager = Arc::new(PluginsManager::new(config.codex_home.clone()));
let skills_manager = Arc::new(SkillsManager::new(
config.codex_home.clone(),
Arc::clone(&plugins_manager),
true,
));
let skills_manager = Arc::new(SkillsManager::new(config.codex_home.clone(), true));
let mcp_manager = Arc::new(McpManager::new(Arc::clone(&plugins_manager)));
let skills_watcher = Arc::new(SkillsWatcher::noop());
+1 -1
View File
@@ -2849,7 +2849,7 @@ impl Config {
}
pub fn bundled_skills_enabled(&self) -> bool {
crate::skills::manager::bundled_skills_enabled_from_stack(&self.config_layer_stack)
crate::manager::bundled_skills_enabled_from_stack(&self.config_layer_stack)
}
}
+3 -34
View File
@@ -801,17 +801,9 @@ impl Notice {
pub(crate) const TABLE_KEY: &'static str = "notice";
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema)]
#[schemars(deny_unknown_fields)]
pub struct SkillConfig {
/// Path-based selector.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub path: Option<AbsolutePathBuf>,
/// Name-based selector.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub name: Option<String>,
pub enabled: bool,
}
pub use codex_config::BundledSkillsConfig;
pub use codex_config::SkillConfig;
pub use codex_config::SkillsConfig;
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema)]
#[schemars(deny_unknown_fields)]
@@ -820,29 +812,6 @@ pub struct PluginConfig {
pub enabled: bool,
}
#[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq, Eq, JsonSchema)]
#[schemars(deny_unknown_fields)]
pub struct SkillsConfig {
#[serde(default, skip_serializing_if = "Option::is_none")]
pub bundled: Option<BundledSkillsConfig>,
#[serde(default, skip_serializing_if = "Vec::is_empty")]
pub config: Vec<SkillConfig>,
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema)]
#[schemars(deny_unknown_fields)]
pub struct BundledSkillsConfig {
#[serde(default = "default_enabled")]
pub enabled: bool,
}
impl Default for BundledSkillsConfig {
fn default() -> Self {
Self { enabled: true }
}
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Default, JsonSchema)]
#[schemars(deny_unknown_fields)]
pub struct SandboxWorkspaceWrite {
+7 -51
View File
@@ -52,10 +52,12 @@ pub use codex_config::TextRange;
pub use codex_config::WebSearchModeRequirement;
pub(crate) use codex_config::build_cli_overrides_layer;
pub(crate) use codex_config::config_error_from_toml;
pub use codex_config::default_project_root_markers;
pub use codex_config::format_config_error;
pub use codex_config::format_config_error_with_source;
pub(crate) use codex_config::io_error_from_config_error;
pub use codex_config::merge_toml_values;
pub use codex_config::project_root_markers_from_config;
#[cfg(test)]
pub(crate) use codex_config::version_for_toml;
@@ -67,8 +69,6 @@ pub const SYSTEM_CONFIG_TOML_FILE_UNIX: &str = "/etc/codex/config.toml";
#[cfg(windows)]
const DEFAULT_PROGRAM_DATA_DIR_WINDOWS: &str = r"C:\ProgramData";
const DEFAULT_PROJECT_ROOT_MARKERS: &[&str] = &[".git"];
pub(crate) async fn first_layer_config_error(layers: &ConfigLayerStack) -> Option<ConfigError> {
codex_config::first_layer_config_error::<ConfigToml>(layers, CONFIG_TOML_FILE).await
}
@@ -285,6 +285,11 @@ pub async fn load_config_layers_state(
));
}
if let Some(config) = managed_config_from_mdm {
// As a general rule, config from MDM should _not_ include relative
// paths, starting with `./`, but a path starting with `~/` _is_ a
// supported use case. Because resolve_relative_paths_in_config_toml()
// relies on AbsolutePathBufGuard to resolve `~/`, we must supply a
// value for base_dir, so codex_home is as good a value as any.
let managed_config =
resolve_relative_paths_in_config_toml(config.managed_config, codex_home)?;
layers.push(ConfigLayerEntry::new_with_raw_toml(
@@ -524,55 +529,6 @@ async fn load_requirements_from_legacy_scheme(
Ok(())
}
/// Reads `project_root_markers` from the [toml::Value] produced by merging
/// `config.toml` from the config layers in the stack preceding
/// [ConfigLayerSource::Project].
///
/// Invariants:
/// - If `project_root_markers` is not specified, returns `Ok(None)`.
/// - If `project_root_markers` is specified, returns `Ok(Some(markers))` where
/// `markers` is a `Vec<String>` (including `Ok(Some(Vec::new()))` for an
/// empty array, which indicates that root detection should be disabled).
/// - Returns an error if `project_root_markers` is specified but is not an
/// array of strings.
pub(crate) fn project_root_markers_from_config(
config: &TomlValue,
) -> io::Result<Option<Vec<String>>> {
let Some(table) = config.as_table() else {
return Ok(None);
};
let Some(markers_value) = table.get("project_root_markers") else {
return Ok(None);
};
let TomlValue::Array(entries) = markers_value else {
return Err(io::Error::new(
io::ErrorKind::InvalidData,
"project_root_markers must be an array of strings",
));
};
if entries.is_empty() {
return Ok(Some(Vec::new()));
}
let mut markers = Vec::new();
for entry in entries {
let Some(marker) = entry.as_str() else {
return Err(io::Error::new(
io::ErrorKind::InvalidData,
"project_root_markers must be an array of strings",
));
};
markers.push(marker.to_string());
}
Ok(Some(markers))
}
pub(crate) fn default_project_root_markers() -> Vec<String> {
DEFAULT_PROJECT_ROOT_MARKERS
.iter()
.map(ToString::to_string)
.collect()
}
struct ProjectTrustContext {
project_root: AbsolutePathBuf,
project_root_key: String,
-1
View File
@@ -1,3 +1,2 @@
pub(crate) use codex_instructions::SkillInstructions;
pub use codex_instructions::USER_INSTRUCTIONS_PREFIX;
pub(crate) use codex_instructions::UserInstructions;
+27 -2
View File
@@ -61,18 +61,44 @@ pub use text_encoding::bytes_to_string_smart;
mod mcp_tool_call;
mod memories;
pub mod mention_syntax;
mod mentions;
pub mod message_history;
mod model_provider_info;
pub mod utils;
pub use utils::path_utils;
pub mod personality_migration;
pub mod plugins;
pub(crate) mod mentions {
pub(crate) use crate::plugins::build_connector_slug_counts;
pub(crate) use crate::plugins::build_skill_name_counts;
pub(crate) use crate::plugins::collect_explicit_app_ids;
pub(crate) use crate::plugins::collect_explicit_plugin_mentions;
pub(crate) use crate::plugins::collect_tool_mentions_from_messages;
}
mod sandbox_tags;
pub mod sandboxing;
mod session_prefix;
mod session_startup_prewarm;
mod shell_detect;
pub mod skills;
pub(crate) use skills::SkillError;
pub(crate) use skills::SkillInjections;
pub(crate) use skills::SkillLoadOutcome;
pub(crate) use skills::SkillMetadata;
pub(crate) use skills::SkillsLoadInput;
pub(crate) use skills::SkillsManager;
pub(crate) use skills::build_skill_injections;
pub(crate) use skills::build_skill_name_counts;
pub(crate) use skills::collect_env_var_dependencies;
pub(crate) use skills::collect_explicit_skill_mentions;
pub(crate) use skills::config_rules;
pub(crate) use skills::injection;
pub(crate) use skills::loader;
pub(crate) use skills::manager;
pub(crate) use skills::maybe_emit_implicit_skill_invocation;
pub(crate) use skills::model;
pub(crate) use skills::render_skills_section;
pub(crate) use skills::resolve_skill_dependencies_for_turn;
pub(crate) use skills::skills_load_input_from_config;
mod skills_watcher;
mod stream_events_utils;
pub mod test_support;
@@ -126,7 +152,6 @@ pub mod seatbelt;
mod session_rollout_init_error;
pub mod shell;
pub mod shell_snapshot;
pub mod skills;
pub mod spawn;
pub mod state_db_bridge;
pub use codex_rollout::state_db;
+2 -2
View File
@@ -15,6 +15,7 @@ use super::auth::McpOAuthLoginSupport;
use super::auth::oauth_login_support;
use super::auth::resolve_oauth_scopes;
use super::auth::should_retry_without_scopes;
use crate::SkillMetadata;
use crate::codex::Session;
use crate::codex::TurnContext;
use crate::config::Config;
@@ -24,8 +25,7 @@ use crate::config::types::McpServerConfig;
use crate::config::types::McpServerTransportConfig;
use crate::default_client::is_first_party_originator;
use crate::default_client::originator;
use crate::skills::SkillMetadata;
use crate::skills::model::SkillToolDependency;
use crate::model::SkillToolDependency;
use codex_features::Feature;
const SKILL_MCP_DEPENDENCY_PROMPT_ID: &str = "skill_mcp_dependency_install";
@@ -1,5 +1,5 @@
use super::*;
use crate::skills::model::SkillDependencies;
use crate::model::SkillDependencies;
use codex_protocol::protocol::SkillScope;
use pretty_assertions::assert_eq;
use std::path::PathBuf;
+7 -8
View File
@@ -27,6 +27,7 @@ use super::store::PluginStore;
use super::store::PluginStoreError;
use super::sync_openai_plugins_repo;
use crate::AuthManager;
use crate::SkillMetadata;
use crate::auth::CodexAuth;
use crate::config::Config;
use crate::config::ConfigService;
@@ -36,12 +37,12 @@ use crate::config::edit::ConfigEditsBuilder;
use crate::config::types::McpServerConfig;
use crate::config::types::PluginConfig;
use crate::config_loader::ConfigLayerStack;
use crate::skills::SkillMetadata;
use crate::skills::config_rules::SkillConfigRules;
use crate::skills::config_rules::resolve_disabled_skill_paths;
use crate::skills::config_rules::skill_config_rules_from_stack;
use crate::skills::loader::SkillRoot;
use crate::skills::loader::load_skills_from_roots;
use crate::config_rules::SkillConfigRules;
use crate::config_rules::resolve_disabled_skill_paths;
use crate::config_rules::skill_config_rules_from_stack;
use crate::loader::SkillRoot;
use crate::loader::load_skills_from_roots;
use codex_analytics::AnalyticsEventsClient;
use codex_app_server_protocol::ConfigValueWriteParams;
use codex_app_server_protocol::MergeStrategy;
use codex_features::Feature;
@@ -73,8 +74,6 @@ use toml_edit::value;
use tracing::info;
use tracing::warn;
use crate::AnalyticsEventsClient;
const DEFAULT_SKILLS_DIR_NAME: &str = "skills";
const DEFAULT_MCP_CONFIG_FILE: &str = ".mcp.json";
const DEFAULT_APP_CONFIG_FILE: &str = ".app.json";
+1 -1
View File
@@ -1,5 +1,5 @@
use codex_utils_absolute_path::AbsolutePathBuf;
pub(crate) use codex_utils_plugins::PLUGIN_MANIFEST_PATH;
use codex_utils_plugins::PLUGIN_MANIFEST_PATH;
use serde::Deserialize;
use serde_json::Value as JsonValue;
use std::fs;
@@ -1,19 +1,18 @@
use std::collections::HashMap;
use std::collections::HashSet;
use std::path::PathBuf;
use codex_protocol::user_input::UserInput;
use crate::connectors;
use crate::injection::ToolMentionKind;
use crate::injection::app_id_from_path;
use crate::injection::extract_tool_mentions_with_sigil;
use crate::injection::plugin_config_name_from_path;
use crate::injection::tool_kind_for_path;
use crate::mention_syntax::PLUGIN_TEXT_MENTION_SIGIL;
use crate::mention_syntax::TOOL_MENTION_SIGIL;
use crate::plugins::PluginCapabilitySummary;
use crate::skills::SkillMetadata;
use crate::skills::injection::ToolMentionKind;
use crate::skills::injection::app_id_from_path;
use crate::skills::injection::extract_tool_mentions_with_sigil;
use crate::skills::injection::plugin_config_name_from_path;
use crate::skills::injection::tool_kind_for_path;
use super::PluginCapabilitySummary;
pub(crate) struct CollectedToolMentions {
pub(crate) plain_names: HashSet<String>,
@@ -102,23 +101,7 @@ pub(crate) fn collect_explicit_plugin_mentions(
.collect()
}
pub(crate) fn build_skill_name_counts(
skills: &[SkillMetadata],
disabled_paths: &HashSet<PathBuf>,
) -> (HashMap<String, usize>, HashMap<String, usize>) {
let mut exact_counts: HashMap<String, usize> = HashMap::new();
let mut lower_counts: HashMap<String, usize> = HashMap::new();
for skill in skills {
if disabled_paths.contains(&skill.path_to_skills_md) {
continue;
}
*exact_counts.entry(skill.name.clone()).or_insert(0) += 1;
*lower_counts
.entry(skill.name.to_ascii_lowercase())
.or_insert(0) += 1;
}
(exact_counts, lower_counts)
}
pub(crate) use crate::build_skill_name_counts;
pub(crate) fn build_connector_slug_counts(
connectors: &[connectors::AppInfo],
+7 -1
View File
@@ -5,6 +5,7 @@ mod injection;
mod manager;
mod manifest;
mod marketplace;
mod mentions;
mod remote;
mod render;
mod startup_sync;
@@ -23,7 +24,6 @@ pub use codex_plugin::PluginTelemetryMetadata;
pub type LoadedPlugin = codex_plugin::LoadedPlugin<McpServerConfig>;
pub type PluginLoadOutcome = codex_plugin::PluginLoadOutcome<McpServerConfig>;
pub(crate) use codex_plugin::plugin_namespace_for_skill_path;
pub(crate) use discoverable::list_tool_suggest_discoverable_plugins;
pub(crate) use injection::build_plugin_injections;
pub use manager::ConfiguredMarketplace;
@@ -61,3 +61,9 @@ pub(crate) use startup_sync::curated_plugins_repo_path;
pub(crate) use startup_sync::read_curated_plugins_sha;
pub(crate) use startup_sync::sync_openai_plugins_repo;
pub use toggles::collect_plugin_enabled_candidates;
pub(crate) use mentions::build_connector_slug_counts;
pub(crate) use mentions::build_skill_name_counts;
pub(crate) use mentions::collect_explicit_app_ids;
pub(crate) use mentions::collect_explicit_plugin_mentions;
pub(crate) use mentions::collect_tool_mentions_from_messages;
+230
View File
@@ -0,0 +1,230 @@
use std::collections::HashMap;
use std::collections::HashSet;
use std::env;
use std::path::Path;
use std::path::PathBuf;
use std::sync::Arc;
use crate::codex::Session;
use crate::codex::TurnContext;
use crate::config::Config;
use codex_analytics::InvocationType;
use codex_analytics::SkillInvocation;
use codex_analytics::build_track_events_context;
use codex_protocol::protocol::SkillScope;
use codex_protocol::request_user_input::RequestUserInputArgs;
use codex_protocol::request_user_input::RequestUserInputQuestion;
use codex_protocol::request_user_input::RequestUserInputResponse;
use tracing::warn;
pub use codex_core_skills::SkillDependencyInfo;
pub use codex_core_skills::SkillError;
pub use codex_core_skills::SkillLoadOutcome;
pub use codex_core_skills::SkillMetadata;
pub use codex_core_skills::SkillPolicy;
pub use codex_core_skills::SkillsLoadInput;
pub use codex_core_skills::SkillsManager;
pub use codex_core_skills::build_skill_name_counts;
pub use codex_core_skills::collect_env_var_dependencies;
pub use codex_core_skills::config_rules;
pub use codex_core_skills::detect_implicit_skill_invocation_for_command;
pub use codex_core_skills::filter_skill_load_outcome_for_product;
pub use codex_core_skills::injection;
pub use codex_core_skills::injection::SkillInjections;
pub use codex_core_skills::injection::build_skill_injections;
pub use codex_core_skills::injection::collect_explicit_skill_mentions;
pub use codex_core_skills::loader;
pub use codex_core_skills::manager;
pub use codex_core_skills::model;
pub use codex_core_skills::remote;
pub use codex_core_skills::render;
pub use codex_core_skills::render_skills_section;
pub use codex_core_skills::system;
pub(crate) fn skills_load_input_from_config(
config: &Config,
effective_skill_roots: Vec<PathBuf>,
) -> SkillsLoadInput {
SkillsLoadInput::new(
config.cwd.clone().to_path_buf(),
effective_skill_roots,
config.config_layer_stack.clone(),
config.bundled_skills_enabled(),
)
}
pub(crate) async fn resolve_skill_dependencies_for_turn(
sess: &Arc<Session>,
turn_context: &Arc<TurnContext>,
dependencies: &[SkillDependencyInfo],
) {
if dependencies.is_empty() {
return;
}
let existing_env = sess.dependency_env().await;
let mut loaded_values = HashMap::new();
let mut missing = Vec::new();
let mut seen_names = HashSet::new();
for dependency in dependencies {
let name = dependency.name.clone();
if !seen_names.insert(name.clone()) || existing_env.contains_key(&name) {
continue;
}
match env::var(&name) {
Ok(value) => {
loaded_values.insert(name.clone(), value);
}
Err(env::VarError::NotPresent) => {
missing.push(dependency.clone());
}
Err(err) => {
warn!("failed to read env var {name}: {err}");
missing.push(dependency.clone());
}
}
}
if !loaded_values.is_empty() {
sess.set_dependency_env(loaded_values).await;
}
if !missing.is_empty() {
request_skill_dependencies(sess, turn_context, &missing).await;
}
}
async fn request_skill_dependencies(
sess: &Arc<Session>,
turn_context: &Arc<TurnContext>,
dependencies: &[SkillDependencyInfo],
) {
let questions = dependencies
.iter()
.map(|dependency| {
let requirement = dependency.description.as_ref().map_or_else(
|| {
format!(
"The skill \"{}\" requires \"{}\" to be set.",
dependency.skill_name, dependency.name
)
},
|description| {
format!(
"The skill \"{}\" requires \"{}\" to be set ({}).",
dependency.skill_name, dependency.name, description
)
},
);
RequestUserInputQuestion {
id: dependency.name.clone(),
header: "Skill requires environment variable".to_string(),
question: format!(
"{requirement} This is an experimental internal feature. The value is stored in memory for this session only."
),
is_other: false,
is_secret: true,
options: None,
}
})
.collect::<Vec<_>>();
if questions.is_empty() {
return;
}
let response = sess
.request_user_input(
turn_context,
format!("skill-deps-{}", turn_context.sub_id),
RequestUserInputArgs { questions },
)
.await
.unwrap_or_else(|| RequestUserInputResponse {
answers: HashMap::new(),
});
if response.answers.is_empty() {
return;
}
let mut values = HashMap::new();
for (name, answer) in response.answers {
let mut user_note = None;
for entry in &answer.answers {
if let Some(note) = entry.strip_prefix("user_note: ")
&& !note.trim().is_empty()
{
user_note = Some(note.trim().to_string());
}
}
if let Some(value) = user_note {
values.insert(name, value);
}
}
if values.is_empty() {
return;
}
sess.set_dependency_env(values).await;
}
pub(crate) async fn maybe_emit_implicit_skill_invocation(
sess: &Session,
turn_context: &TurnContext,
command: &str,
workdir: &Path,
) {
let Some(candidate) = detect_implicit_skill_invocation_for_command(
turn_context.turn_skills.outcome.as_ref(),
command,
workdir,
) else {
return;
};
let invocation = SkillInvocation {
skill_name: candidate.name,
skill_scope: candidate.scope,
skill_path: candidate.path_to_skills_md,
invocation_type: InvocationType::Implicit,
};
let skill_scope = match invocation.skill_scope {
SkillScope::User => "user",
SkillScope::Repo => "repo",
SkillScope::System => "system",
SkillScope::Admin => "admin",
};
let skill_path = invocation.skill_path.to_string_lossy();
let skill_name = invocation.skill_name.clone();
let seen_key = format!("{skill_scope}:{skill_path}:{skill_name}");
let inserted = {
let mut seen_skills = turn_context
.turn_skills
.implicit_invocation_seen_skills
.lock()
.await;
seen_skills.insert(seen_key)
};
if !inserted {
return;
}
turn_context.session_telemetry.counter(
"codex.skill.injected",
/*inc*/ 1,
&[
("status", "ok"),
("skill", skill_name.as_str()),
("invoke_type", "implicit"),
],
);
sess.services
.analytics_events_client
.track_skill_invocations(
build_track_events_context(
turn_context.model_info.slug.clone(),
sess.conversation_id.to_string(),
turn_context.sub_id.clone(),
),
vec![invocation],
);
}
@@ -1,162 +0,0 @@
use std::collections::HashMap;
use std::collections::HashSet;
use std::env;
use std::sync::Arc;
use codex_protocol::request_user_input::RequestUserInputArgs;
use codex_protocol::request_user_input::RequestUserInputQuestion;
use codex_protocol::request_user_input::RequestUserInputResponse;
use tracing::warn;
use crate::codex::Session;
use crate::codex::TurnContext;
use crate::skills::SkillMetadata;
#[derive(Debug, Clone, PartialEq, Eq)]
pub(crate) struct SkillDependencyInfo {
pub(crate) skill_name: String,
pub(crate) name: String,
pub(crate) description: Option<String>,
}
/// Resolve required dependency values (session cache, then env vars),
/// and prompt the UI for any missing ones.
pub(crate) async fn resolve_skill_dependencies_for_turn(
sess: &Arc<Session>,
turn_context: &Arc<TurnContext>,
dependencies: &[SkillDependencyInfo],
) {
if dependencies.is_empty() {
return;
}
let existing_env = sess.dependency_env().await;
let mut loaded_values = HashMap::new();
let mut missing = Vec::new();
let mut seen_names = HashSet::new();
for dependency in dependencies {
let name = dependency.name.clone();
if !seen_names.insert(name.clone()) {
continue;
}
if existing_env.contains_key(&name) {
continue;
}
match env::var(&name) {
Ok(value) => {
loaded_values.insert(name.clone(), value);
continue;
}
Err(env::VarError::NotPresent) => {}
Err(err) => {
warn!("failed to read env var {name}: {err}");
}
}
missing.push(dependency.clone());
}
if !loaded_values.is_empty() {
sess.set_dependency_env(loaded_values).await;
}
if !missing.is_empty() {
request_skill_dependencies(sess, turn_context, &missing).await;
}
}
pub(crate) fn collect_env_var_dependencies(
mentioned_skills: &[SkillMetadata],
) -> Vec<SkillDependencyInfo> {
let mut dependencies = Vec::new();
for skill in mentioned_skills {
let Some(skill_dependencies) = &skill.dependencies else {
continue;
};
for tool in &skill_dependencies.tools {
if tool.r#type != "env_var" {
continue;
}
if tool.value.is_empty() {
continue;
}
dependencies.push(SkillDependencyInfo {
skill_name: skill.name.clone(),
name: tool.value.clone(),
description: tool.description.clone(),
});
}
}
dependencies
}
/// Prompt via request_user_input to gather missing env vars.
pub(crate) async fn request_skill_dependencies(
sess: &Arc<Session>,
turn_context: &Arc<TurnContext>,
dependencies: &[SkillDependencyInfo],
) {
let questions = dependencies
.iter()
.map(|dep| {
let requirement = dep.description.as_ref().map_or_else(
|| format!("The skill \"{}\" requires \"{}\" to be set.", dep.skill_name, dep.name),
|description| {
format!(
"The skill \"{}\" requires \"{}\" to be set ({}).",
dep.skill_name, dep.name, description
)
},
);
let question = format!(
"{requirement} This is an experimental internal feature. The value is stored in memory for this session only.",
);
RequestUserInputQuestion {
id: dep.name.clone(),
header: "Skill requires environment variable".to_string(),
question,
is_other: false,
is_secret: true,
options: None,
}
})
.collect::<Vec<_>>();
if questions.is_empty() {
return;
}
let args = RequestUserInputArgs { questions };
let call_id = format!("skill-deps-{}", turn_context.sub_id);
let response = sess
.request_user_input(turn_context, call_id, args)
.await
.unwrap_or_else(|| RequestUserInputResponse {
answers: HashMap::new(),
});
if response.answers.is_empty() {
return;
}
let mut values = HashMap::new();
for (name, answer) in response.answers {
let mut user_note = None;
for entry in &answer.answers {
if let Some(note) = entry.strip_prefix("user_note: ")
&& !note.trim().is_empty()
{
user_note = Some(note.trim().to_string());
}
}
if let Some(value) = user_note {
values.insert(name, value);
}
}
if values.is_empty() {
return;
}
sess.set_dependency_env(values).await;
}
+8 -2
View File
@@ -8,6 +8,7 @@ use tokio::runtime::Handle;
use tokio::sync::broadcast;
use tracing::warn;
use crate::SkillsManager;
use crate::config::Config;
use crate::file_watcher::FileWatcher;
use crate::file_watcher::FileWatcherSubscriber;
@@ -15,7 +16,8 @@ use crate::file_watcher::Receiver;
use crate::file_watcher::ThrottledWatchReceiver;
use crate::file_watcher::WatchPath;
use crate::file_watcher::WatchRegistration;
use crate::skills::SkillsManager;
use crate::plugins::PluginsManager;
use crate::skills_load_input_from_config;
#[cfg(not(test))]
const WATCHER_THROTTLE_INTERVAL: Duration = Duration::from_secs(10);
@@ -56,9 +58,13 @@ impl SkillsWatcher {
&self,
config: &Config,
skills_manager: &SkillsManager,
plugins_manager: &PluginsManager,
) -> WatchRegistration {
let plugin_outcome = plugins_manager.plugins_for_config(config);
let effective_skill_roots = plugin_outcome.effective_skill_roots();
let skills_input = skills_load_input_from_config(config, effective_skill_roots);
let roots = skills_manager
.skill_roots_for_config(config)
.skill_roots_for_config(&skills_input)
.into_iter()
.map(|root| WatchPath {
path: root.path,
+1 -1
View File
@@ -3,6 +3,7 @@ use std::sync::Arc;
use crate::AuthManager;
use crate::RolloutRecorder;
use crate::SkillsManager;
use crate::agent::AgentControl;
use crate::client::ModelClient;
use crate::config::StartedNetworkProxy;
@@ -11,7 +12,6 @@ use crate::mcp::McpManager;
use crate::mcp_connection_manager::McpConnectionManager;
use crate::models_manager::manager::ModelsManager;
use crate::plugins::PluginsManager;
use crate::skills::SkillsManager;
use crate::skills_watcher::SkillsWatcher;
use crate::state_db::StateDbHandle;
use crate::tools::code_mode::CodeModeService;
+6 -6
View File
@@ -2,6 +2,7 @@ use crate::AuthManager;
use crate::CodexAuth;
use crate::ModelProviderInfo;
use crate::OPENAI_PROVIDER_ID;
use crate::SkillsManager;
use crate::agent::AgentControl;
use crate::codex::Codex;
use crate::codex::CodexSpawnArgs;
@@ -22,7 +23,6 @@ use crate::protocol::SessionConfiguredEvent;
use crate::rollout::RolloutRecorder;
use crate::rollout::truncation;
use crate::shell_snapshot::ShellSnapshot;
use crate::skills::SkillsManager;
use crate::skills_watcher::SkillsWatcher;
use crate::skills_watcher::SkillsWatcherEvent;
use crate::tasks::interrupted_turn_history_marker;
@@ -231,7 +231,6 @@ impl ThreadManager {
let mcp_manager = Arc::new(McpManager::new(Arc::clone(&plugins_manager)));
let skills_manager = Arc::new(SkillsManager::new_with_restriction_product(
codex_home.clone(),
Arc::clone(&plugins_manager),
config.bundled_skills_enabled(),
restriction_product,
));
@@ -297,7 +296,6 @@ impl ThreadManager {
let mcp_manager = Arc::new(McpManager::new(Arc::clone(&plugins_manager)));
let skills_manager = Arc::new(SkillsManager::new_with_restriction_product(
codex_home.clone(),
Arc::clone(&plugins_manager),
/*bundled_skills_enabled*/ true,
restriction_product,
));
@@ -835,9 +833,11 @@ impl ThreadManagerState {
parent_trace: Option<W3cTraceContext>,
user_shell_override: Option<crate::shell::Shell>,
) -> CodexResult<NewThread> {
let watch_registration = self
.skills_watcher
.register_config(&config, self.skills_manager.as_ref());
let watch_registration = self.skills_watcher.register_config(
&config,
self.skills_manager.as_ref(),
self.plugins_manager.as_ref(),
);
let CodexSpawnOk {
codex, thread_id, ..
} = Codex::spawn(CodexSpawnArgs {
+3 -2
View File
@@ -11,9 +11,9 @@ use crate::exec_env::create_env;
use crate::exec_policy::ExecApprovalRequest;
use crate::function_tool::FunctionCallError;
use crate::is_safe_command::is_known_safe_command;
use crate::maybe_emit_implicit_skill_invocation;
use crate::protocol::ExecCommandSource;
use crate::shell::Shell;
use crate::skills::maybe_emit_implicit_skill_invocation;
use crate::tools::context::FunctionToolOutput;
use crate::tools::context::ToolInvocation;
use crate::tools::context::ToolPayload;
@@ -288,11 +288,12 @@ impl ToolHandler for ShellCommandHandler {
let cwd = resolve_workdir_base_path(&arguments, turn.cwd.as_path())?;
let params: ShellCommandToolCallParams =
parse_arguments_with_base_path(&arguments, cwd.as_path())?;
let workdir = turn.resolve_path(params.workdir.clone());
maybe_emit_implicit_skill_invocation(
session.as_ref(),
turn.as_ref(),
&params.command,
params.workdir.as_deref(),
&workdir,
)
.await;
let prefix_rule = params.prefix_rule.clone();
@@ -252,45 +252,90 @@ async fn verify_tool_suggestion_completed(
auth: Option<&crate::CodexAuth>,
) -> bool {
match tool {
DiscoverableTool::Connector(connector) => {
let manager = session.services.mcp_connection_manager.read().await;
match manager.hard_refresh_codex_apps_tools_cache().await {
Ok(mcp_tools) => {
let accessible_connectors = connectors::with_app_enabled_state(
connectors::accessible_connectors_from_mcp_tools(&mcp_tools),
&turn.config,
);
connectors::refresh_accessible_connectors_cache_from_mcp_tools(
&turn.config,
auth,
&mcp_tools,
);
verified_connector_suggestion_completed(
connector.id.as_str(),
&accessible_connectors,
)
}
Err(err) => {
warn!(
"failed to refresh codex apps tools cache after tool suggestion for {}: {err:#}",
connector.id
);
false
}
}
}
DiscoverableTool::Connector(connector) => refresh_missing_suggested_connectors(
session,
turn,
auth,
std::slice::from_ref(&connector.id),
connector.id.as_str(),
)
.await
.is_some_and(|accessible_connectors| {
verified_connector_suggestion_completed(connector.id.as_str(), &accessible_connectors)
}),
DiscoverableTool::Plugin(plugin) => {
session.reload_user_config_layer().await;
let config = session.get_config().await;
verified_plugin_suggestion_completed(
let completed = verified_plugin_suggestion_completed(
plugin.id.as_str(),
config.as_ref(),
session.services.plugins_manager.as_ref(),
);
let _ = refresh_missing_suggested_connectors(
session,
turn,
auth,
&plugin.app_connector_ids,
plugin.id.as_str(),
)
.await;
completed
}
}
}
async fn refresh_missing_suggested_connectors(
session: &crate::codex::Session,
turn: &crate::codex::TurnContext,
auth: Option<&crate::CodexAuth>,
expected_connector_ids: &[String],
tool_id: &str,
) -> Option<Vec<AppInfo>> {
if expected_connector_ids.is_empty() {
return Some(Vec::new());
}
let manager = session.services.mcp_connection_manager.read().await;
let mcp_tools = manager.list_all_tools().await;
let accessible_connectors = connectors::with_app_enabled_state(
connectors::accessible_connectors_from_mcp_tools(&mcp_tools),
&turn.config,
);
if all_suggested_connectors_picked_up(expected_connector_ids, &accessible_connectors) {
return Some(accessible_connectors);
}
match manager.hard_refresh_codex_apps_tools_cache().await {
Ok(mcp_tools) => {
let accessible_connectors = connectors::with_app_enabled_state(
connectors::accessible_connectors_from_mcp_tools(&mcp_tools),
&turn.config,
);
connectors::refresh_accessible_connectors_cache_from_mcp_tools(
&turn.config,
auth,
&mcp_tools,
);
Some(accessible_connectors)
}
Err(err) => {
warn!(
"failed to refresh codex apps tools cache after tool suggestion for {tool_id}: {err:#}"
);
None
}
}
}
fn all_suggested_connectors_picked_up(
expected_connector_ids: &[String],
accessible_connectors: &[AppInfo],
) -> bool {
expected_connector_ids.iter().all(|connector_id| {
verified_connector_suggestion_completed(connector_id, accessible_connectors)
})
}
fn verified_connector_suggestion_completed(
tool_id: &str,
accessible_connectors: &[AppInfo],
@@ -235,6 +235,34 @@ fn verified_connector_suggestion_completed_requires_accessible_connector() {
));
}
#[test]
fn all_suggested_connectors_picked_up_requires_every_expected_connector() {
let accessible_connectors = vec![AppInfo {
id: "calendar".to_string(),
name: "Google Calendar".to_string(),
description: None,
logo_url: None,
logo_url_dark: None,
distribution_channel: None,
branding: None,
app_metadata: None,
labels: None,
install_url: None,
is_accessible: true,
is_enabled: false,
plugin_display_names: Vec::new(),
}];
assert!(all_suggested_connectors_picked_up(
&["calendar".to_string()],
&accessible_connectors,
));
assert!(!all_suggested_connectors_picked_up(
&["calendar".to_string(), "gmail".to_string()],
&accessible_connectors,
));
}
#[tokio::test]
async fn verified_plugin_suggestion_completed_requires_installed_plugin() {
let codex_home = tempdir().expect("tempdir should succeed");
@@ -1,11 +1,11 @@
use crate::function_tool::FunctionCallError;
use crate::is_safe_command::is_known_safe_command;
use crate::maybe_emit_implicit_skill_invocation;
use crate::protocol::EventMsg;
use crate::protocol::TerminalInteractionEvent;
use crate::sandboxing::SandboxPermissions;
use crate::shell::Shell;
use crate::shell::get_shell_by_model_provided_path;
use crate::skills::maybe_emit_implicit_skill_invocation;
use crate::tools::context::ExecCommandToolOutput;
use crate::tools::context::ToolInvocation;
use crate::tools::context::ToolPayload;
@@ -147,11 +147,12 @@ impl ToolHandler for UnifiedExecHandler {
let cwd = resolve_workdir_base_path(&arguments, context.turn.cwd.as_path())?;
let args: ExecCommandArgs =
parse_arguments_with_base_path(&arguments, cwd.as_path())?;
let workdir = context.turn.resolve_path(args.workdir.clone());
maybe_emit_implicit_skill_invocation(
session.as_ref(),
turn.as_ref(),
context.turn.as_ref(),
&args.cmd,
args.workdir.as_deref(),
&workdir,
)
.await;
let process_id = manager.allocate_process_id().await;
+1 -1
View File
@@ -4,9 +4,9 @@ Module: runtimes
Concrete ToolRuntime implementations for specific tools. Each runtime stays
small and focused and reuses the orchestrator for approvals + sandbox + retry.
*/
use crate::SkillMetadata;
use crate::path_utils;
use crate::shell::Shell;
use crate::skills::SkillMetadata;
use crate::tools::sandboxing::ToolError;
use codex_protocol::models::PermissionProfile;
use codex_sandboxing::SandboxCommand;
@@ -1,4 +1,5 @@
use super::ShellRequest;
use crate::SkillMetadata;
use crate::error::CodexErr;
use crate::error::SandboxErr;
use crate::exec::ExecCapturePolicy;
@@ -12,7 +13,7 @@ use crate::sandboxing::ExecOptions;
use crate::sandboxing::ExecRequest;
use crate::sandboxing::SandboxPermissions;
use crate::shell::ShellType;
use crate::skills::SkillMetadata;
use crate::skills_load_input_from_config;
use crate::tools::runtimes::ExecveSessionApproval;
use crate::tools::runtimes::build_sandbox_command;
use crate::tools::sandboxing::SandboxAttempt;
@@ -487,11 +488,19 @@ impl CoreShellActionProvider {
/// any skills.
async fn find_skill(&self, program: &AbsolutePathBuf) -> Option<SkillMetadata> {
let force_reload = false;
let turn_config = self.turn.config.as_ref();
let plugin_outcome = self
.session
.services
.plugins_manager
.plugins_for_config(turn_config);
let effective_skill_roots = plugin_outcome.effective_skill_roots();
let skills_input = skills_load_input_from_config(turn_config, effective_skill_roots);
let skills_outcome = self
.session
.services
.skills_manager
.skills_for_cwd(&self.turn.cwd, self.turn.config.as_ref(), force_reload)
.skills_for_cwd(&skills_input, force_reload)
.await;
let program_path = program.as_path();
@@ -8,6 +8,7 @@ use super::evaluate_intercepted_exec_policy;
use super::extract_shell_script;
use super::join_program_and_argv;
use super::map_exec_result;
use crate::SkillMetadata;
#[cfg(target_os = "macos")]
use crate::config::Constrained;
#[cfg(target_os = "macos")]
@@ -19,7 +20,6 @@ use crate::protocol::GranularApprovalConfig;
use crate::protocol::ReadOnlyAccess;
use crate::protocol::SandboxPolicy;
use crate::sandboxing::SandboxPermissions;
use crate::skills::SkillMetadata;
use codex_execpolicy::Decision;
use codex_execpolicy::Evaluation;
use codex_execpolicy::PolicyParser;
+1 -2
View File
@@ -390,8 +390,7 @@ impl ToolsConfig {
include_request_user_input && features.enabled(Feature::DefaultModeRequestUserInput);
let include_search_tool =
model_info.supports_search_tool && features.enabled(Feature::ToolSearch);
let include_tool_suggest = include_search_tool
&& features.enabled(Feature::ToolSuggest)
let include_tool_suggest = features.enabled(Feature::ToolSuggest)
&& features.enabled(Feature::Apps)
&& features.enabled(Feature::Plugins);
let include_original_image_detail = can_request_original_image_detail(features, model_info);
+57
View File
@@ -2029,6 +2029,8 @@ fn tool_suggest_is_not_registered_without_feature_flag() {
let model_info = search_capable_model_info();
let mut features = Features::with_defaults();
features.enable(Feature::ToolSearch);
features.enable(Feature::Apps);
features.enable(Feature::Plugins);
features.disable(Feature::ToolSuggest);
let available_models = Vec::new();
let tools_config = ToolsConfig::new(&ToolsConfigParams {
@@ -2060,6 +2062,54 @@ fn tool_suggest_is_not_registered_without_feature_flag() {
);
}
#[test]
fn tool_suggest_can_be_registered_without_search_tool() {
let model_info = ModelInfo {
supports_search_tool: false,
..search_capable_model_info()
};
let mut features = Features::with_defaults();
features.enable(Feature::Apps);
features.enable(Feature::Plugins);
features.enable(Feature::ToolSuggest);
let available_models = Vec::new();
let tools_config = ToolsConfig::new(&ToolsConfigParams {
model_info: &model_info,
available_models: &available_models,
features: &features,
web_search_mode: Some(WebSearchMode::Cached),
session_source: SessionSource::Cli,
sandbox_policy: &SandboxPolicy::DangerFullAccess,
windows_sandbox_level: WindowsSandboxLevel::Disabled,
});
let (tools, _) = build_specs_with_discoverable_tools(
&tools_config,
None,
None,
Some(vec![discoverable_connector(
"connector_2128aebfecb84f64a069897515042a44",
"Google Calendar",
"Plan events and schedules.",
)]),
&[],
)
.build();
assert_contains_tool_names(&tools, &[TOOL_SUGGEST_TOOL_NAME]);
assert_lacks_tool_name(&tools, TOOL_SEARCH_TOOL_NAME);
let tool_suggest = find_tool(&tools, TOOL_SUGGEST_TOOL_NAME);
let ToolSpec::Function(ResponsesApiTool { description, .. }) = &tool_suggest.spec else {
panic!("expected function tool");
};
assert!(
description.contains(
"You've already tried to find a matching available tool for the user's request"
)
);
assert!(description.contains("This includes `tool_search` (if available) and other means."));
}
#[test]
fn tool_suggest_requires_apps_and_plugins_features() {
let model_info = search_capable_model_info();
@@ -2315,7 +2365,14 @@ fn tool_suggest_description_lists_discoverable_tools() {
assert!(
description.contains("skills; MCP servers: sample-docs; app connectors: connector_sample")
);
assert!(
description.contains(
"You've already tried to find a matching available tool for the user's request"
)
);
assert!(description.contains("This includes `tool_search` (if available) and other means."));
assert!(description.contains("DO NOT explore or recommend tools that are not on this list."));
assert!(!description.contains("tool_search fails to find a good match"));
let JsonSchema::Object { required, .. } = parameters else {
panic!("expected object parameters");
};
@@ -3,8 +3,7 @@
Suggests a discoverable connector or plugin when the user clearly wants a capability that is not currently available in the active `tools` list.
Use this ONLY when:
- There's no available tool to handle the user's request
- And tool_search fails to find a good match
- You've already tried to find a matching available tool for the user's request but couldn't find a good match. This includes `tool_search` (if available) and other means.
- AND the user's request strongly matches one of the discoverable tools listed below.
Tool suggestions should only use the discoverable tools listed here. DO NOT explore or recommend tools that are not on this list.
@@ -14,12 +13,13 @@ Discoverable tools:
Workflow:
1. Match the user's request against the discoverable tools list above.
2. If one tool clearly fits, call `tool_suggest` with:
1. Ensure all possible means have been exhausted to find an existing available tool but none of them matches the request intent.
2. Match the user's request against the discoverable tools list above.
3. If one tool clearly fits, call `tool_suggest` with:
- `tool_type`: `connector` or `plugin`
- `action_type`: `install` or `enable`
- `tool_id`: exact id from the discoverable tools list above
- `suggest_reason`: concise one-line user-facing reason this tool can help with the current request
3. After the suggestion flow completes:
4. After the suggestion flow completes:
- if the user finished the install or enable flow, continue by searching again or using the newly available tool
- if the user did not finish, continue without that tool, and don't suggest that tool again unless the user explicitly asks you to.
+1
View File
@@ -130,6 +130,7 @@ mod text_encoding_fix;
mod thread_metadata;
mod tool_harness;
mod tool_parallelism;
mod tool_suggest;
mod tools;
mod truncation;
mod turn_state;
+144
View File
@@ -0,0 +1,144 @@
#![cfg(not(target_os = "windows"))]
#![allow(clippy::unwrap_used, clippy::expect_used)]
use anyhow::Result;
use codex_core::CodexAuth;
use codex_core::config::Config;
use codex_core::config::types::ToolSuggestDiscoverable;
use codex_core::config::types::ToolSuggestDiscoverableType;
use codex_features::Feature;
use codex_protocol::openai_models::ModelsResponse;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::SandboxPolicy;
use core_test_support::apps_test_server::AppsTestServer;
use core_test_support::responses::ev_assistant_message;
use core_test_support::responses::ev_completed;
use core_test_support::responses::ev_response_created;
use core_test_support::responses::mount_sse_once;
use core_test_support::responses::sse;
use core_test_support::responses::start_mock_server;
use core_test_support::skip_if_no_network;
use core_test_support::test_codex::test_codex;
use serde_json::Value;
const TOOL_SEARCH_TOOL_NAME: &str = "tool_search";
const TOOL_SUGGEST_TOOL_NAME: &str = "tool_suggest";
const DISCOVERABLE_GMAIL_ID: &str = "connector_68df038e0ba48191908c8434991bbac2";
fn tool_names(body: &Value) -> Vec<String> {
body.get("tools")
.and_then(Value::as_array)
.map(|tools| {
tools
.iter()
.filter_map(|tool| {
tool.get("name")
.or_else(|| tool.get("type"))
.and_then(Value::as_str)
.map(str::to_string)
})
.collect()
})
.unwrap_or_default()
}
fn function_tool_description(body: &Value, name: &str) -> Option<String> {
body.get("tools")
.and_then(Value::as_array)
.and_then(|tools| {
tools.iter().find_map(|tool| {
if tool.get("name").and_then(Value::as_str) == Some(name) {
tool.get("description")
.and_then(Value::as_str)
.map(str::to_string)
} else {
None
}
})
})
}
fn configure_apps_without_search_tool(config: &mut Config, apps_base_url: &str) {
config
.features
.enable(Feature::Apps)
.expect("test config should allow feature update");
config
.features
.enable(Feature::Plugins)
.expect("test config should allow feature update");
config
.features
.enable(Feature::ToolSuggest)
.expect("test config should allow feature update");
config.chatgpt_base_url = apps_base_url.to_string();
config.model = Some("gpt-5-codex".to_string());
config.tool_suggest.discoverables = vec![ToolSuggestDiscoverable {
kind: ToolSuggestDiscoverableType::Connector,
id: DISCOVERABLE_GMAIL_ID.to_string(),
}];
let mut model_catalog: ModelsResponse =
serde_json::from_str(include_str!("../../models.json")).expect("valid models.json");
let model = model_catalog
.models
.iter_mut()
.find(|model| model.slug == "gpt-5-codex")
.expect("gpt-5-codex exists in bundled models.json");
model.supports_search_tool = false;
config.model_catalog = Some(model_catalog);
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn tool_suggest_is_available_without_search_tool_after_discovery_attempts() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
let apps_server = AppsTestServer::mount(&server).await?;
let mock = mount_sse_once(
&server,
sse(vec![
ev_response_created("resp-1"),
ev_assistant_message("msg-1", "done"),
ev_completed("resp-1"),
]),
)
.await;
let mut builder = test_codex()
.with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing())
.with_config(move |config| {
configure_apps_without_search_tool(config, apps_server.chatgpt_base_url.as_str())
});
let test = builder.build(&server).await?;
test.submit_turn_with_policies(
"list tools",
AskForApproval::Never,
SandboxPolicy::DangerFullAccess,
)
.await?;
let body = mock.single_request().body_json();
let tools = tool_names(&body);
assert!(
!tools.iter().any(|name| name == TOOL_SEARCH_TOOL_NAME),
"tools list should not include {TOOL_SEARCH_TOOL_NAME}: {tools:?}"
);
assert!(
tools.iter().any(|name| name == TOOL_SUGGEST_TOOL_NAME),
"tools list should include {TOOL_SUGGEST_TOOL_NAME}: {tools:?}"
);
let description =
function_tool_description(&body, TOOL_SUGGEST_TOOL_NAME).expect("description");
assert!(
description.contains(
"You've already tried to find a matching available tool for the user's request"
)
);
assert!(description.contains("This includes `tool_search` (if available) and other means."));
assert!(!description.contains("tool_search fails to find a good match"));
Ok(())
}
+4 -4
View File
@@ -717,7 +717,7 @@ pub const FEATURES: &[FeatureSpec] = &[
id: Feature::Apps,
key: "apps",
stage: Stage::Stable,
default_enabled: true,
default_enabled: false,
},
FeatureSpec {
id: Feature::ToolSearch,
@@ -729,13 +729,13 @@ pub const FEATURES: &[FeatureSpec] = &[
id: Feature::ToolSuggest,
key: "tool_suggest",
stage: Stage::Stable,
default_enabled: true,
default_enabled: false,
},
FeatureSpec {
id: Feature::Plugins,
key: "plugins",
stage: Stage::Stable,
default_enabled: true,
default_enabled: false,
},
FeatureSpec {
id: Feature::ImageGeneration,
@@ -787,7 +787,7 @@ pub const FEATURES: &[FeatureSpec] = &[
id: Feature::ToolCallMcpElicitation,
key: "tool_call_mcp_elicitation",
stage: Stage::Stable,
default_enabled: true,
default_enabled: false,
},
FeatureSpec {
id: Feature::Personality,
+4 -4
View File
@@ -115,9 +115,9 @@ fn request_permissions_tool_is_under_development() {
}
#[test]
fn tool_suggest_is_stable_and_enabled_by_default() {
fn tool_suggest_is_stable_and_disabled_by_default() {
assert_eq!(Feature::ToolSuggest.stage(), Stage::Stable);
assert_eq!(Feature::ToolSuggest.default_enabled(), true);
assert_eq!(Feature::ToolSuggest.default_enabled(), false);
}
#[test]
@@ -145,9 +145,9 @@ fn image_generation_is_under_development() {
}
#[test]
fn tool_call_mcp_elicitation_is_stable_and_enabled_by_default() {
fn tool_call_mcp_elicitation_is_stable_and_disabled_by_default() {
assert_eq!(Feature::ToolCallMcpElicitation.stage(), Stage::Stable);
assert_eq!(Feature::ToolCallMcpElicitation.default_enabled(), true);
assert_eq!(Feature::ToolCallMcpElicitation.default_enabled(), false);
}
#[test]
+70
View File
@@ -0,0 +1,70 @@
//! Resolve plugin namespace from skill file paths by walking ancestors for `plugin.json`.
use std::fs;
use std::path::Path;
/// Relative path from a plugin root to its manifest file.
pub const PLUGIN_MANIFEST_PATH: &str = ".codex-plugin/plugin.json";
#[derive(serde::Deserialize)]
#[serde(rename_all = "camelCase")]
struct RawPluginManifestName {
#[serde(default)]
name: String,
}
fn plugin_manifest_name(plugin_root: &Path) -> Option<String> {
let manifest_path = plugin_root.join(PLUGIN_MANIFEST_PATH);
if !manifest_path.is_file() {
return None;
}
let contents = fs::read_to_string(&manifest_path).ok()?;
let RawPluginManifestName { name: raw_name } = serde_json::from_str(&contents).ok()?;
Some(
plugin_root
.file_name()
.and_then(|entry| entry.to_str())
.filter(|_| raw_name.trim().is_empty())
.unwrap_or(raw_name.as_str())
.to_string(),
)
}
/// Returns the plugin manifest `name` for the nearest ancestor of `path` that contains a valid
/// plugin manifest (same `name` rules as full manifest loading in codex-core).
pub fn plugin_namespace_for_skill_path(path: &Path) -> Option<String> {
for ancestor in path.ancestors() {
if let Some(name) = plugin_manifest_name(ancestor) {
return Some(name);
}
}
None
}
#[cfg(test)]
mod tests {
use super::plugin_namespace_for_skill_path;
use std::fs;
use tempfile::tempdir;
#[test]
fn uses_manifest_name() {
let tmp = tempdir().expect("tempdir");
let plugin_root = tmp.path().join("plugins/sample");
let skill_path = plugin_root.join("skills/search/SKILL.md");
fs::create_dir_all(skill_path.parent().expect("parent")).expect("mkdir");
fs::create_dir_all(plugin_root.join(".codex-plugin")).expect("mkdir manifest");
fs::write(
plugin_root.join(".codex-plugin/plugin.json"),
r#"{"name":"sample"}"#,
)
.expect("write manifest");
fs::write(&skill_path, "---\ndescription: search\n---\n").expect("write skill");
assert_eq!(
plugin_namespace_for_skill_path(&skill_path),
Some("sample".to_string())
);
}
}
+316 -15
View File
@@ -69,6 +69,8 @@ use codex_app_server_protocol::RequestId;
use codex_app_server_protocol::ServerNotification;
use codex_app_server_protocol::ServerRequest;
use codex_app_server_protocol::SkillsListResponse;
use codex_app_server_protocol::ThreadItem;
use codex_app_server_protocol::ThreadLoadedListParams;
use codex_app_server_protocol::ThreadRollbackResponse;
use codex_app_server_protocol::Turn;
use codex_app_server_protocol::TurnError as AppServerTurnError;
@@ -143,11 +145,13 @@ use uuid::Uuid;
mod agent_navigation;
mod app_server_adapter;
mod app_server_requests;
mod loaded_threads;
mod pending_interactive_replay;
use self::agent_navigation::AgentNavigationDirection;
use self::agent_navigation::AgentNavigationState;
use self::app_server_requests::PendingAppServerRequests;
use self::loaded_threads::find_loaded_subagent_threads_for_primary;
use self::pending_interactive_replay::PendingInteractiveReplayState;
const EXTERNAL_EDITOR_HINT: &str = "Save and close external editor to continue.";
@@ -200,6 +204,30 @@ fn command_execution_decision_to_review_decision(
}
}
/// Extracts `receiver_thread_ids` from collab agent tool-call notifications.
///
/// Only `ItemStarted` and `ItemCompleted` notifications with a `CollabAgentToolCall` item carry
/// receiver thread ids. All other notification variants return `None`.
fn collab_receiver_thread_ids(notification: &ServerNotification) -> Option<&[String]> {
match notification {
ServerNotification::ItemStarted(notification) => match &notification.item {
ThreadItem::CollabAgentToolCall {
receiver_thread_ids,
..
} => Some(receiver_thread_ids),
_ => None,
},
ServerNotification::ItemCompleted(notification) => match &notification.item {
ThreadItem::CollabAgentToolCall {
receiver_thread_ids,
..
} => Some(receiver_thread_ids),
_ => None,
},
_ => None,
}
}
fn convert_via_json<T, U>(value: T) -> Option<U>
where
T: serde::Serialize,
@@ -951,6 +979,7 @@ pub(crate) struct App {
active_thread_id: Option<ThreadId>,
active_thread_rx: Option<mpsc::Receiver<ThreadBufferedEvent>>,
primary_thread_id: Option<ThreadId>,
last_subagent_backfill_attempt: Option<ThreadId>,
primary_session_configured: Option<ThreadSessionState>,
pending_primary_events: VecDeque<ThreadBufferedEvent>,
pending_app_server_requests: PendingAppServerRequests,
@@ -2265,6 +2294,67 @@ impl App {
Ok(())
}
/// Eagerly fetches nickname and role for receiver threads referenced by a collab notification.
///
/// This runs on every buffered thread notification before it reaches rendering. For each
/// receiver thread id that the navigation cache does not yet have metadata for, it issues a
/// `thread/read` RPC and registers the result in both `AgentNavigationState` and the
/// `ChatWidget` metadata map. Threads that already have a nickname or role cached are skipped,
/// so the cost is at most one RPC per thread over the lifetime of a session.
///
/// Failures are logged and silently ignored -- the worst outcome is that a rendered item shows
/// a thread id instead of a human-readable name, which is the same behavior the TUI had before
/// this change.
async fn hydrate_collab_agent_metadata_for_notification(
&mut self,
app_server: &mut AppServerSession,
notification: &ServerNotification,
) {
let Some(receiver_thread_ids) = collab_receiver_thread_ids(notification) else {
return;
};
for receiver_thread_id in receiver_thread_ids {
let Ok(thread_id) = ThreadId::from_string(receiver_thread_id) else {
tracing::warn!(
thread_id = receiver_thread_id,
"ignoring collab receiver with invalid thread id during metadata hydration"
);
continue;
};
if self
.agent_navigation
.get(&thread_id)
.is_some_and(|entry| entry.agent_nickname.is_some() || entry.agent_role.is_some())
{
continue;
}
match app_server
.thread_read(thread_id, /*include_turns*/ false)
.await
{
Ok(thread) => {
self.ensure_thread_channel(thread_id);
self.upsert_agent_picker_thread(
thread_id,
thread.agent_nickname,
thread.agent_role,
/*is_closed*/ false,
);
}
Err(err) => {
tracing::warn!(
thread_id = %thread_id,
error = %err,
"failed to hydrate collab receiver thread metadata"
);
}
}
}
}
async fn infer_session_for_thread_notification(
&mut self,
thread_id: ThreadId,
@@ -2602,6 +2692,11 @@ impl App {
agent_role: Option<String>,
is_closed: bool,
) {
self.chat_widget.set_collab_agent_metadata(
thread_id,
agent_nickname.clone(),
agent_role.clone(),
);
self.agent_navigation
.upsert(thread_id, agent_nickname, agent_role, is_closed);
self.sync_active_agent_label();
@@ -2616,6 +2711,24 @@ impl App {
self.sync_active_agent_label();
}
/// Replaces the chat widget and re-seeds the new widget's collab metadata from the navigation
/// cache.
///
/// Thread switches reconstruct the `ChatWidget`, which loses the `collab_agent_metadata` map.
/// This helper copies every known nickname/role from `AgentNavigationState` into the
/// replacement widget so that replayed collab items render agent names immediately.
fn replace_chat_widget(&mut self, mut chat_widget: ChatWidget) {
for (thread_id, entry) in self.agent_navigation.ordered_threads() {
chat_widget.set_collab_agent_metadata(
thread_id,
entry.agent_nickname.clone(),
entry.agent_role.clone(),
);
}
self.chat_widget = chat_widget;
self.sync_active_agent_label();
}
async fn select_agent_thread(
&mut self,
tui: &mut tui::Tui,
@@ -2661,8 +2774,7 @@ impl App {
self.active_thread_rx = Some(receiver);
let init = self.chatwidget_init_for_forked_or_resumed_thread(tui, self.config.clone());
self.chat_widget = ChatWidget::new_with_app_event(init);
self.sync_active_agent_label();
self.replace_chat_widget(ChatWidget::new_with_app_event(init));
self.reset_for_thread_switch(tui)?;
self.replay_thread_snapshot(snapshot, !is_replay_only);
@@ -2697,6 +2809,7 @@ impl App {
self.active_thread_id = None;
self.active_thread_rx = None;
self.primary_thread_id = None;
self.last_subagent_backfill_attempt = None;
self.primary_session_configured = None;
self.pending_primary_events.clear();
self.pending_app_server_requests.clear();
@@ -2732,7 +2845,7 @@ impl App {
match app_server.start_thread(&config).await {
Ok(started) => {
if let Err(err) = self
.replace_chat_widget_with_app_server_thread(tui, started)
.replace_chat_widget_with_app_server_thread(tui, app_server, started)
.await
{
self.chat_widget.add_error_message(format!(
@@ -2760,13 +2873,119 @@ impl App {
async fn replace_chat_widget_with_app_server_thread(
&mut self,
tui: &mut tui::Tui,
app_server: &mut AppServerSession,
started: AppServerStartedThread,
) -> Result<()> {
let init = self.chatwidget_init_for_forked_or_resumed_thread(tui, self.config.clone());
self.chat_widget = ChatWidget::new_with_app_event(init);
self.reset_thread_event_state();
let init = self.chatwidget_init_for_forked_or_resumed_thread(tui, self.config.clone());
self.replace_chat_widget(ChatWidget::new_with_app_event(init));
self.enqueue_primary_thread_session(started.session, started.turns)
.await?;
self.backfill_loaded_subagent_threads(app_server).await;
Ok(())
}
/// Fetches all loaded threads from the app server and registers descendants of the primary
/// thread in the navigation cache and chat widget metadata.
///
/// Called after `replace_chat_widget_with_app_server_thread` during resume, fork, and new
/// thread creation so that the `/agent` picker and keyboard navigation are pre-populated even
/// if the TUI did not witness the original spawn events.
///
/// The loaded-thread list is fetched in full (no pagination) and the spawn tree is walked
/// by `find_loaded_subagent_threads_for_primary`. Each discovered subagent is registered via
/// `upsert_agent_picker_thread`, which writes to both `AgentNavigationState` and the
/// `ChatWidget` metadata map.
async fn backfill_loaded_subagent_threads(
&mut self,
app_server: &mut AppServerSession,
) -> bool {
let Some(primary_thread_id) = self.primary_thread_id else {
return false;
};
let loaded_thread_ids = match app_server
.thread_loaded_list(ThreadLoadedListParams {
cursor: None,
limit: None,
})
.await
{
Ok(response) => response.data,
Err(err) => {
tracing::warn!(%err, "failed to list loaded threads for subagent backfill");
return false;
}
};
let mut threads = Vec::new();
let mut had_read_error = false;
for thread_id in loaded_thread_ids {
let Ok(thread_id) = ThreadId::from_string(&thread_id) else {
tracing::warn!("ignoring loaded thread with invalid id during subagent backfill");
continue;
};
if thread_id == primary_thread_id {
continue;
}
match app_server
.thread_read(thread_id, /*include_turns*/ false)
.await
{
Ok(thread) => threads.push(thread),
Err(err) => {
had_read_error = true;
tracing::warn!(thread_id = %thread_id, %err, "failed to read loaded thread");
}
}
}
for thread in find_loaded_subagent_threads_for_primary(threads, primary_thread_id) {
self.ensure_thread_channel(thread.thread_id);
self.upsert_agent_picker_thread(
thread.thread_id,
thread.agent_nickname,
thread.agent_role,
/*is_closed*/ false,
);
}
!had_read_error
}
/// Returns the adjacent thread id for keyboard navigation, backfilling from the server if the
/// local cache has no neighbor.
///
/// Tries the fast path first: ask `AgentNavigationState` directly. If it returns `None` (no
/// adjacent entry exists, typically because the cache was never populated with remote
/// subagents), performs a full `backfill_loaded_subagent_threads` and retries. This ensures the
/// first next/previous keypress in a resumed remote session discovers subagents on demand
/// without requiring the user to wait for a proactive fetch.
async fn adjacent_thread_id_with_backfill(
&mut self,
app_server: &mut AppServerSession,
direction: AgentNavigationDirection,
) -> Option<ThreadId> {
let current_thread = self.current_displayed_thread_id();
if let Some(thread_id) = self
.agent_navigation
.adjacent_thread_id(current_thread, direction)
{
return Some(thread_id);
}
let primary_thread_id = self.primary_thread_id?;
if self.last_subagent_backfill_attempt == Some(primary_thread_id) {
return None;
}
if self.backfill_loaded_subagent_threads(app_server).await {
self.last_subagent_backfill_attempt = Some(primary_thread_id);
}
self.agent_navigation
.adjacent_thread_id(self.current_displayed_thread_id(), direction)
}
fn fresh_session_config(&self) -> Config {
@@ -3116,6 +3335,7 @@ impl App {
active_thread_id: None,
active_thread_rx: None,
primary_thread_id: None,
last_subagent_backfill_attempt: None,
primary_session_configured: None,
pending_primary_events: VecDeque::new(),
pending_app_server_requests: PendingAppServerRequests::default(),
@@ -3428,7 +3648,9 @@ impl App {
self.file_search
.update_search_dir(self.config.cwd.to_path_buf());
match self
.replace_chat_widget_with_app_server_thread(tui, resumed)
.replace_chat_widget_with_app_server_thread(
tui, app_server, resumed,
)
.await
{
Ok(()) => {
@@ -3488,7 +3710,7 @@ impl App {
Ok(forked) => {
self.shutdown_current_thread(app_server).await;
match self
.replace_chat_widget_with_app_server_thread(tui, forked)
.replace_chat_widget_with_app_server_thread(tui, app_server, forked)
.await
{
Ok(()) => {
@@ -4962,6 +5184,11 @@ impl App {
// thread, so unrelated shutdowns cannot consume this marker.
self.pending_shutdown_exit_thread_id = None;
}
if let ThreadBufferedEvent::Notification(notification) = &event {
self.hydrate_collab_agent_metadata_for_notification(app_server, notification)
.await;
}
self.handle_thread_event_now(event);
if self.backtrack_render_pending {
tui.frame_requester().schedule_frame();
@@ -5117,10 +5344,10 @@ impl App {
&& self.chat_widget.composer_text_with_pending().is_empty()
&& previous_agent_shortcut_matches(key_event, allow_agent_word_motion_fallback)
{
if let Some(thread_id) = self.agent_navigation.adjacent_thread_id(
self.current_displayed_thread_id(),
AgentNavigationDirection::Previous,
) {
if let Some(thread_id) = self
.adjacent_thread_id_with_backfill(app_server, AgentNavigationDirection::Previous)
.await
{
let _ = self.select_agent_thread(tui, app_server, thread_id).await;
}
return;
@@ -5132,10 +5359,10 @@ impl App {
&& self.chat_widget.composer_text_with_pending().is_empty()
&& next_agent_shortcut_matches(key_event, allow_agent_word_motion_fallback)
{
if let Some(thread_id) = self.agent_navigation.adjacent_thread_id(
self.current_displayed_thread_id(),
AgentNavigationDirection::Next,
) {
if let Some(thread_id) = self
.adjacent_thread_id_with_backfill(app_server, AgentNavigationDirection::Next)
.await
{
let _ = self.select_agent_thread(tui, app_server, thread_id).await;
}
return;
@@ -7864,6 +8091,7 @@ guardian_approval = true
active_thread_id: None,
active_thread_rx: None,
primary_thread_id: None,
last_subagent_backfill_attempt: None,
primary_session_configured: None,
pending_primary_events: VecDeque::new(),
pending_app_server_requests: PendingAppServerRequests::default(),
@@ -7915,6 +8143,7 @@ guardian_approval = true
active_thread_id: None,
active_thread_rx: None,
primary_thread_id: None,
last_subagent_backfill_attempt: None,
primary_session_configured: None,
pending_primary_events: VecDeque::new(),
pending_app_server_requests: PendingAppServerRequests::default(),
@@ -9002,6 +9231,78 @@ guardian_approval = true
);
}
#[tokio::test]
async fn replace_chat_widget_reseeds_collab_agent_metadata_for_replay() {
let (mut app, mut app_event_rx, _op_rx) = make_test_app_with_channels().await;
let receiver_thread_id =
ThreadId::from_string("019cff70-2599-75e2-af72-b958ce5dc1cc").expect("valid thread");
app.agent_navigation.upsert(
receiver_thread_id,
Some("Robie".to_string()),
Some("explorer".to_string()),
false,
);
let replacement = ChatWidget::new_with_app_event(ChatWidgetInit {
config: app.config.clone(),
frame_requester: crate::tui::FrameRequester::test_dummy(),
app_event_tx: app.app_event_tx.clone(),
initial_user_message: None,
enhanced_keys_supported: app.enhanced_keys_supported,
has_chatgpt_account: app.chat_widget.has_chatgpt_account(),
model_catalog: app.model_catalog.clone(),
feedback: app.feedback.clone(),
is_first_run: false,
feedback_audience: app.feedback_audience,
status_account_display: app.chat_widget.status_account_display().cloned(),
initial_plan_type: app.chat_widget.current_plan_type(),
model: Some(app.chat_widget.current_model().to_string()),
startup_tooltip_override: None,
status_line_invalid_items_warned: app.status_line_invalid_items_warned.clone(),
session_telemetry: app.session_telemetry.clone(),
});
app.replace_chat_widget(replacement);
app.replay_thread_snapshot(
ThreadEventSnapshot {
session: None,
turns: Vec::new(),
events: vec![ThreadBufferedEvent::Notification(
ServerNotification::ItemStarted(codex_app_server_protocol::ItemStartedNotification {
thread_id: "thread-1".to_string(),
turn_id: "turn-1".to_string(),
item: ThreadItem::CollabAgentToolCall {
id: "wait-1".to_string(),
tool: codex_app_server_protocol::CollabAgentTool::Wait,
status: codex_app_server_protocol::CollabAgentToolCallStatus::InProgress,
sender_thread_id: ThreadId::new().to_string(),
receiver_thread_ids: vec![receiver_thread_id.to_string()],
prompt: None,
model: None,
reasoning_effort: None,
agents_states: HashMap::new(),
},
}),
)],
input_state: None,
},
false,
);
let mut saw_named_wait = false;
while let Ok(event) = app_event_rx.try_recv() {
if let AppEvent::InsertHistoryCell(cell) = event {
let transcript = lines_to_single_string(&cell.transcript_lines(80));
saw_named_wait |= transcript.contains("Robie [explorer]");
}
}
assert!(
saw_named_wait,
"expected replayed wait item to keep agent name"
);
}
#[tokio::test]
async fn refreshed_snapshot_session_persists_resumed_turns() {
let mut app = make_test_app().await;
@@ -0,0 +1,208 @@
//! Discovers subagent threads that belong to a primary thread by walking spawn-tree edges.
//!
//! When the TUI resumes or switches to an existing thread, it needs to populate
//! `AgentNavigationState` and `ChatWidget` metadata for every subagent that was spawned during
//! that thread's lifetime. The app server exposes a flat list of currently loaded threads via
//! `thread/loaded/list`, but the TUI must figure out which of those are descendants of the
//! primary thread.
//!
//! This module provides the pure, synchronous tree-walk that turns that flat list into the filtered
//! set of descendants. It intentionally has no async, no I/O, and no side effects so it can be
//! unit-tested in isolation.
//!
//! The walk starts from `primary_thread_id` and repeatedly follows
//! `SessionSource::SubAgent(ThreadSpawn { parent_thread_id, .. })` edges until no new children are
//! found. The primary thread itself is never included in the output.
use codex_app_server_protocol::SessionSource;
use codex_app_server_protocol::Thread;
use codex_protocol::ThreadId;
use codex_protocol::protocol::SubAgentSource;
use std::collections::HashMap;
use std::collections::HashSet;
/// A subagent thread discovered by the spawn-tree walk, carrying just enough metadata for the
/// TUI to register it in the navigation cache and rendering metadata map.
#[derive(Debug, Clone, PartialEq, Eq)]
pub(crate) struct LoadedSubagentThread {
pub(crate) thread_id: ThreadId,
pub(crate) agent_nickname: Option<String>,
pub(crate) agent_role: Option<String>,
}
/// Walks the spawn tree rooted at `primary_thread_id` and returns every descendant subagent.
///
/// The walk is breadth-first over `SessionSource::SubAgent(ThreadSpawn { parent_thread_id })` edges.
/// Threads whose `source` is not a `ThreadSpawn`, or whose `parent_thread_id` does not chain back
/// to `primary_thread_id`, are excluded. The primary thread itself is never included.
///
/// Results are sorted by stringified thread id for deterministic output in tests and in the
/// navigation cache. Callers should not rely on this ordering for anything semantic; it exists
/// purely to make snapshot assertions stable.
///
/// If two threads claim the same parent, both are included. Cycles in the parent chain are not
/// possible because `ThreadId`s are server-assigned UUIDs and the server enforces acyclicity, but
/// the `included` set guards against re-visiting regardless.
pub(crate) fn find_loaded_subagent_threads_for_primary(
threads: Vec<Thread>,
primary_thread_id: ThreadId,
) -> Vec<LoadedSubagentThread> {
let mut threads_by_id = HashMap::new();
for thread in threads {
let Ok(thread_id) = ThreadId::from_string(&thread.id) else {
continue;
};
threads_by_id.insert(thread_id, thread);
}
let mut included = HashSet::new();
let mut pending = vec![primary_thread_id];
while let Some(parent_thread_id) = pending.pop() {
for (thread_id, thread) in &threads_by_id {
if included.contains(thread_id) {
continue;
}
let SessionSource::SubAgent(SubAgentSource::ThreadSpawn {
parent_thread_id: source_parent_thread_id,
..
}) = &thread.source
else {
continue;
};
if *source_parent_thread_id != parent_thread_id {
continue;
}
included.insert(*thread_id);
pending.push(*thread_id);
}
}
let mut loaded_threads: Vec<LoadedSubagentThread> = included
.into_iter()
.filter_map(|thread_id| {
threads_by_id
.remove(&thread_id)
.map(|thread| LoadedSubagentThread {
thread_id,
agent_nickname: thread.agent_nickname,
agent_role: thread.agent_role,
})
})
.collect();
loaded_threads.sort_by_key(|thread| thread.thread_id.to_string());
loaded_threads
}
#[cfg(test)]
mod tests {
use super::LoadedSubagentThread;
use super::find_loaded_subagent_threads_for_primary;
use codex_app_server_protocol::SessionSource;
use codex_app_server_protocol::Thread;
use codex_app_server_protocol::ThreadStatus;
use codex_protocol::ThreadId;
use codex_protocol::protocol::SubAgentSource;
use pretty_assertions::assert_eq;
use std::path::PathBuf;
fn test_thread(thread_id: ThreadId, source: SessionSource) -> Thread {
Thread {
id: thread_id.to_string(),
preview: String::new(),
ephemeral: false,
model_provider: "openai".to_string(),
created_at: 0,
updated_at: 0,
status: ThreadStatus::Idle,
path: None,
cwd: PathBuf::from("/tmp"),
cli_version: "0.0.0".to_string(),
source,
agent_nickname: None,
agent_role: None,
git_info: None,
name: None,
turns: Vec::new(),
}
}
#[test]
fn finds_loaded_subagent_tree_for_primary_thread() {
let primary_thread_id =
ThreadId::from_string("00000000-0000-0000-0000-000000000001").expect("valid thread");
let child_thread_id =
ThreadId::from_string("00000000-0000-0000-0000-000000000002").expect("valid thread");
let grandchild_thread_id =
ThreadId::from_string("00000000-0000-0000-0000-000000000003").expect("valid thread");
let unrelated_parent_id =
ThreadId::from_string("00000000-0000-0000-0000-000000000004").expect("valid thread");
let unrelated_child_id =
ThreadId::from_string("00000000-0000-0000-0000-000000000005").expect("valid thread");
let mut child = test_thread(
child_thread_id,
SessionSource::SubAgent(SubAgentSource::ThreadSpawn {
parent_thread_id: primary_thread_id,
depth: 1,
agent_path: None,
agent_nickname: Some("Scout".to_string()),
agent_role: Some("explorer".to_string()),
}),
);
child.agent_nickname = Some("Scout".to_string());
child.agent_role = Some("explorer".to_string());
let mut grandchild = test_thread(
grandchild_thread_id,
SessionSource::SubAgent(SubAgentSource::ThreadSpawn {
parent_thread_id: child_thread_id,
depth: 2,
agent_path: None,
agent_nickname: Some("Atlas".to_string()),
agent_role: Some("worker".to_string()),
}),
);
grandchild.agent_nickname = Some("Atlas".to_string());
grandchild.agent_role = Some("worker".to_string());
let unrelated_child = test_thread(
unrelated_child_id,
SessionSource::SubAgent(SubAgentSource::ThreadSpawn {
parent_thread_id: unrelated_parent_id,
depth: 1,
agent_path: None,
agent_nickname: Some("Other".to_string()),
agent_role: Some("researcher".to_string()),
}),
);
let loaded = find_loaded_subagent_threads_for_primary(
vec![
test_thread(primary_thread_id, SessionSource::Cli),
child,
grandchild,
unrelated_child,
],
primary_thread_id,
);
assert_eq!(
loaded,
vec![
LoadedSubagentThread {
thread_id: child_thread_id,
agent_nickname: Some("Scout".to_string()),
agent_role: Some("explorer".to_string()),
},
LoadedSubagentThread {
thread_id: grandchild_thread_id,
agent_nickname: Some("Atlas".to_string()),
agent_role: Some("worker".to_string()),
},
]
);
}
}
@@ -27,6 +27,8 @@ use codex_app_server_protocol::ThreadForkParams;
use codex_app_server_protocol::ThreadForkResponse;
use codex_app_server_protocol::ThreadListParams;
use codex_app_server_protocol::ThreadListResponse;
use codex_app_server_protocol::ThreadLoadedListParams;
use codex_app_server_protocol::ThreadLoadedListResponse;
use codex_app_server_protocol::ThreadReadParams;
use codex_app_server_protocol::ThreadReadResponse;
use codex_app_server_protocol::ThreadRealtimeAppendAudioParams;
@@ -354,6 +356,22 @@ impl AppServerSession {
.wrap_err("thread/list failed during TUI session lookup")
}
/// Lists thread ids that the app server currently holds in memory.
///
/// Used by `App::backfill_loaded_subagent_threads` to discover subagent threads that were
/// spawned before the TUI connected. The caller then fetches full metadata per thread via
/// `thread_read` and walks the spawn tree.
pub(crate) async fn thread_loaded_list(
&mut self,
params: ThreadLoadedListParams,
) -> Result<ThreadLoadedListResponse> {
let request_id = self.next_request_id();
self.client
.request_typed(ClientRequest::ThreadLoadedList { request_id, params })
.await
.wrap_err("failed to list loaded threads from app server")
}
pub(crate) async fn thread_read(
&mut self,
thread_id: ThreadId,
+115 -13
View File
@@ -140,6 +140,7 @@ use codex_protocol::protocol::ApplyPatchApprovalRequestEvent;
use codex_protocol::protocol::BackgroundEventEvent;
#[cfg(test)]
use codex_protocol::protocol::CodexErrorInfo as CoreCodexErrorInfo;
use codex_protocol::protocol::CollabAgentRef;
#[cfg(test)]
use codex_protocol::protocol::CollabAgentSpawnBeginEvent;
use codex_protocol::protocol::CollabAgentStatusEntry;
@@ -757,6 +758,7 @@ pub(crate) struct ChatWidget {
// Latest completed user-visible Codex output that `/copy` should place on the clipboard.
last_copyable_output: Option<String>,
running_commands: HashMap<String, RunningCommand>,
collab_agent_metadata: HashMap<ThreadId, CollabAgentMetadata>,
pending_collab_spawn_requests: HashMap<String, multi_agents::SpawnRequestSummary>,
suppressed_exec_calls: HashSet<String>,
skills_all: Vec<ProtocolSkillMetadata>,
@@ -904,6 +906,18 @@ pub(crate) struct ChatWidget {
last_non_retry_error: Option<(String, String)>,
}
/// Cached nickname and role for a collab agent thread, used to attach human-readable labels to
/// rendered tool-call items.
///
/// Populated externally by `App` via `set_collab_agent_metadata` and consulted by the
/// notification-to-core-event conversion helpers. Defaults to empty so that missing metadata
/// degrades to the previous behavior of showing raw thread ids.
#[derive(Clone, Debug, Default)]
struct CollabAgentMetadata {
agent_nickname: Option<String>,
agent_role: Option<String>,
}
#[cfg_attr(not(test), allow(dead_code))]
enum CodexOpTarget {
Direct(UnboundedSender<Op>),
@@ -1379,9 +1393,12 @@ fn app_server_collab_state_to_core(state: &AppServerCollabAgentState) -> AgentSt
}
}
/// Converts app-server collab agent states into the core protocol representation, enriching each
/// entry with cached nickname and role metadata so rendered items show human-readable names.
fn app_server_collab_agent_statuses_to_core(
receiver_thread_ids: &[String],
agents_states: &HashMap<String, AppServerCollabAgentState>,
collab_agent_metadata: &HashMap<ThreadId, CollabAgentMetadata>,
) -> (Vec<CollabAgentStatusEntry>, HashMap<ThreadId, AgentStatus>) {
let mut agent_statuses = Vec::new();
let mut statuses = HashMap::new();
@@ -1394,10 +1411,14 @@ fn app_server_collab_agent_statuses_to_core(
continue;
};
let status = app_server_collab_state_to_core(agent_state);
let metadata = collab_agent_metadata
.get(&thread_id)
.cloned()
.unwrap_or_default();
agent_statuses.push(CollabAgentStatusEntry {
thread_id,
agent_nickname: None,
agent_role: None,
agent_nickname: metadata.agent_nickname,
agent_role: metadata.agent_role,
status: status.clone(),
});
statuses.insert(thread_id, status);
@@ -1406,6 +1427,31 @@ fn app_server_collab_agent_statuses_to_core(
(agent_statuses, statuses)
}
/// Builds `CollabAgentRef` entries for every valid receiver thread, attaching cached metadata.
///
/// Used when converting collab `Wait` tool-call items so the rendered waiting list shows agent
/// names instead of bare thread ids.
fn app_server_collab_receiver_agent_refs(
receiver_thread_ids: &[String],
collab_agent_metadata: &HashMap<ThreadId, CollabAgentMetadata>,
) -> Vec<CollabAgentRef> {
receiver_thread_ids
.iter()
.filter_map(|thread_id| {
let thread_id = app_server_collab_thread_id_to_core(thread_id)?;
let metadata = collab_agent_metadata
.get(&thread_id)
.cloned()
.unwrap_or_default();
Some(CollabAgentRef {
thread_id,
agent_nickname: metadata.agent_nickname,
agent_role: metadata.agent_role,
})
})
.collect()
}
fn request_permissions_from_params(
params: codex_app_server_protocol::PermissionsRequestApprovalParams,
) -> RequestPermissionsEvent {
@@ -1489,6 +1535,35 @@ fn web_search_action_to_core(
}
impl ChatWidget {
/// Stores or overwrites the cached nickname and role for a collab agent thread.
///
/// Called by `App::upsert_agent_picker_thread` and `App::replace_chat_widget` to keep the
/// rendering metadata in sync with the navigation cache. Must be called before any
/// notification referencing this thread is processed, otherwise the rendered item will fall
/// back to showing the raw thread id.
pub(crate) fn set_collab_agent_metadata(
&mut self,
thread_id: ThreadId,
agent_nickname: Option<String>,
agent_role: Option<String>,
) {
self.collab_agent_metadata.insert(
thread_id,
CollabAgentMetadata {
agent_nickname,
agent_role,
},
);
}
/// Returns the cached metadata for a thread, defaulting to empty if none has been registered.
fn collab_agent_metadata(&self, thread_id: ThreadId) -> CollabAgentMetadata {
self.collab_agent_metadata
.get(&thread_id)
.cloned()
.unwrap_or_default()
}
fn realtime_conversation_enabled(&self) -> bool {
self.config.features.enabled(Feature::RealtimeConversation)
&& cfg!(not(target_os = "linux"))
@@ -3443,6 +3518,8 @@ impl ChatWidget {
let first_receiver = receiver_thread_ids
.first()
.and_then(|thread_id| app_server_collab_thread_id_to_core(thread_id));
let first_receiver_metadata =
first_receiver.map(|thread_id| self.collab_agent_metadata(thread_id));
match tool {
CollabAgentTool::SpawnAgent => {
@@ -3473,8 +3550,12 @@ impl ChatWidget {
call_id: id,
sender_thread_id,
new_thread_id: first_receiver,
new_agent_nickname: None,
new_agent_role: None,
new_agent_nickname: first_receiver_metadata
.as_ref()
.and_then(|metadata| metadata.agent_nickname.clone()),
new_agent_role: first_receiver_metadata
.as_ref()
.and_then(|metadata| metadata.agent_role.clone()),
prompt: prompt.unwrap_or_default(),
model: String::new(),
reasoning_effort: ReasoningEffortConfig::Medium,
@@ -3499,8 +3580,12 @@ impl ChatWidget {
call_id: id,
sender_thread_id,
receiver_thread_id,
receiver_agent_nickname: None,
receiver_agent_role: None,
receiver_agent_nickname: first_receiver_metadata
.as_ref()
.and_then(|metadata| metadata.agent_nickname.clone()),
receiver_agent_role: first_receiver_metadata
.as_ref()
.and_then(|metadata| metadata.agent_role.clone()),
prompt: prompt.unwrap_or_default(),
status: receiver_thread_ids
.iter()
@@ -3521,8 +3606,12 @@ impl ChatWidget {
call_id: id,
sender_thread_id,
receiver_thread_id,
receiver_agent_nickname: None,
receiver_agent_role: None,
receiver_agent_nickname: first_receiver_metadata
.as_ref()
.and_then(|metadata| metadata.agent_nickname.clone()),
receiver_agent_role: first_receiver_metadata
.as_ref()
.and_then(|metadata| metadata.agent_role.clone()),
},
));
} else {
@@ -3531,8 +3620,12 @@ impl ChatWidget {
call_id: id,
sender_thread_id,
receiver_thread_id,
receiver_agent_nickname: None,
receiver_agent_role: None,
receiver_agent_nickname: first_receiver_metadata
.as_ref()
.and_then(|metadata| metadata.agent_nickname.clone()),
receiver_agent_role: first_receiver_metadata
.as_ref()
.and_then(|metadata| metadata.agent_role.clone()),
status: receiver_thread_ids
.iter()
.find_map(|thread_id| agents_states.get(thread_id))
@@ -3556,7 +3649,10 @@ impl ChatWidget {
app_server_collab_thread_id_to_core(thread_id)
})
.collect(),
receiver_agents: Vec::new(),
receiver_agents: app_server_collab_receiver_agent_refs(
&receiver_thread_ids,
&self.collab_agent_metadata,
),
call_id: id,
},
));
@@ -3564,6 +3660,7 @@ impl ChatWidget {
let (agent_statuses, statuses) = app_server_collab_agent_statuses_to_core(
&receiver_thread_ids,
&agents_states,
&self.collab_agent_metadata,
);
self.on_collab_event(multi_agents::waiting_end(
codex_protocol::protocol::CollabWaitingEndEvent {
@@ -3584,8 +3681,12 @@ impl ChatWidget {
call_id: id,
sender_thread_id,
receiver_thread_id,
receiver_agent_nickname: None,
receiver_agent_role: None,
receiver_agent_nickname: first_receiver_metadata
.as_ref()
.and_then(|metadata| metadata.agent_nickname.clone()),
receiver_agent_role: first_receiver_metadata
.as_ref()
.and_then(|metadata| metadata.agent_role.clone()),
status: receiver_thread_ids
.iter()
.find_map(|thread_id| agents_states.get(thread_id))
@@ -4319,6 +4420,7 @@ impl ChatWidget {
plan_stream_controller: None,
last_copyable_output: None,
running_commands: HashMap::new(),
collab_agent_metadata: HashMap::new(),
pending_collab_spawn_requests: HashMap::new(),
suppressed_exec_calls: HashSet::new(),
last_unified_wait: None,
@@ -3,10 +3,10 @@ source: tui_app_server/src/chatwidget/tests.rs
expression: combined
---
• Waiting for 2 agents
└ 019cff70-2599-75e2-af72-b958ce5dc1cc
019cff70-2599-75e2-af72-b96db334332d
└ Robie [explorer]
Ada [reviewer]
• Finished waiting
└ 019cff70-2599-75e2-af72-b958ce5dc1cc: Completed - Done
019cff70-2599-75e2-af72-b96db334332d: Running
└ Robie [explorer]: Completed - Done
Ada [reviewer]: Running
@@ -2046,6 +2046,7 @@ async fn make_chatwidget_manual(
pending_guardian_review_status: PendingGuardianReviewStatus::default(),
last_copyable_output: None,
running_commands: HashMap::new(),
collab_agent_metadata: HashMap::new(),
pending_collab_spawn_requests: HashMap::new(),
suppressed_exec_calls: HashSet::new(),
skills_all: Vec::new(),
@@ -4677,6 +4678,16 @@ async fn live_app_server_collab_wait_items_render_history() {
ThreadId::from_string("019cff70-2599-75e2-af72-b958ce5dc1cc").expect("valid thread id");
let other_receiver_thread_id =
ThreadId::from_string("019cff70-2599-75e2-af72-b96db334332d").expect("valid thread id");
chat.set_collab_agent_metadata(
receiver_thread_id,
Some("Robie".to_string()),
Some("explorer".to_string()),
);
chat.set_collab_agent_metadata(
other_receiver_thread_id,
Some("Ada".to_string()),
Some("reviewer".to_string()),
);
chat.handle_server_notification(
ServerNotification::ItemStarted(ItemStartedNotification {