mirror of
https://github.com/openai/codex.git
synced 2026-08-24 13:20:07 +00:00
13ba8058f2
## Why A thread can select skill roots that live in an executor environment. `skills/list` needs a passive snapshot of the roots that are usable now: it must not start an executor, wait for recovery, or reconnect a failed environment. The initial implementation checked the immutable first startup result. After a successful connection later entered recovery or failed, that result still looked successful. A read-only catalog request could then wait for recovery or trigger a new connection while reading the filesystem. ## What - inspect readiness from the current exec-server connection state - return roots only while their environment can serve a request immediately - omit environments that have not started, are connecting, or are recovering - return warnings for missing environments and terminal connection failures - add a fail-fast filesystem view that never starts, waits for, or reconnects an environment - expose the passive selected-root snapshot through `CodexThread` ## Behavior - Local and currently connected environments are ready. - Starting and recovering environments are omitted without a warning so callers can retry later. - Missing and terminally failed environments are omitted with a warning. - A disconnect between readiness inspection and filesystem access fails promptly instead of crossing into the normal recovery path. - Normal model-turn and execution paths keep their existing reconnect behavior. ## Design The recovery policy is private to the exec-server client. Callers choose the explicit fail-fast filesystem method; the existing client and filesystem APIs remain reconnecting. This keeps the passive contract at the transport boundary instead of plumbing timeout or retry flags through the skills stack. ## Coverage - a lazy stdio environment stays unstarted during passive inspection - missing and terminally failed environments surface warnings - a real websocket disconnect proves current readiness drops, a previously acquired fail-fast filesystem handle returns promptly, and readiness returns after recovery ## Scope This PR only provides passive readiness and fail-fast filesystem primitives. It does not add app-server API fields or notifications. ## Stack - #31582 uses these primitives for experimental thread-scoped `skills/list`. - #30228 adds targeted invalidation notifications.
85 lines
2.7 KiB
Rust
85 lines
2.7 KiB
Rust
mod common;
|
|
|
|
use std::time::Duration;
|
|
|
|
use anyhow::Context;
|
|
use codex_exec_server::EnvironmentManager;
|
|
use codex_exec_server::REMOTE_ENVIRONMENT_ID;
|
|
use codex_exec_server::SelectedCapabilityRootsStatus;
|
|
use codex_protocol::capabilities::CapabilityRootLocation;
|
|
use codex_protocol::capabilities::SelectedCapabilityRoot;
|
|
use codex_utils_path_uri::PathUri;
|
|
use common::exec_server::exec_server;
|
|
use pretty_assertions::assert_eq;
|
|
use tokio::time::sleep;
|
|
use tokio::time::timeout;
|
|
|
|
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
|
#[serial_test::serial(remote_exec_server)]
|
|
async fn selected_capability_inspection_tracks_connection_recovery() -> anyhow::Result<()> {
|
|
let server = exec_server().await?;
|
|
let mut proxy = server.disconnectable_websocket_proxy().await?;
|
|
let manager = EnvironmentManager::create_for_tests(
|
|
Some(proxy.websocket_url().to_string()),
|
|
/*local_runtime_paths*/ None,
|
|
)
|
|
.await;
|
|
let environment = manager
|
|
.default_environment()
|
|
.context("remote environment")?;
|
|
environment.info().await?;
|
|
|
|
let skill_root_path = PathUri::parse("file:///plugins/demo")?;
|
|
let selected_root = SelectedCapabilityRoot {
|
|
id: "demo@1".to_string(),
|
|
location: CapabilityRootLocation::Environment {
|
|
environment_id: REMOTE_ENVIRONMENT_ID.to_string(),
|
|
path: skill_root_path.clone(),
|
|
},
|
|
};
|
|
assert_eq!(
|
|
manager.inspect_selected_capability_roots(std::slice::from_ref(&selected_root)),
|
|
SelectedCapabilityRootsStatus {
|
|
ready_roots: vec![selected_root.clone()],
|
|
warnings: Vec::new(),
|
|
}
|
|
);
|
|
let file_system = environment.get_filesystem_without_reconnect();
|
|
|
|
proxy.pause_and_disconnect().await?;
|
|
assert_eq!(
|
|
manager.inspect_selected_capability_roots(std::slice::from_ref(&selected_root)),
|
|
SelectedCapabilityRootsStatus::default()
|
|
);
|
|
let read_result = timeout(
|
|
Duration::from_secs(1),
|
|
file_system.read_directory(&skill_root_path, /*sandbox*/ None),
|
|
)
|
|
.await
|
|
.context("passive filesystem read waited for recovery")?;
|
|
assert!(read_result.is_err());
|
|
|
|
proxy.resume()?;
|
|
let recovered_status = timeout(Duration::from_secs(5), async {
|
|
loop {
|
|
let status =
|
|
manager.inspect_selected_capability_roots(std::slice::from_ref(&selected_root));
|
|
if !status.ready_roots.is_empty() {
|
|
break status;
|
|
}
|
|
sleep(Duration::from_millis(10)).await;
|
|
}
|
|
})
|
|
.await
|
|
.context("environment did not recover")?;
|
|
assert_eq!(
|
|
recovered_status,
|
|
SelectedCapabilityRootsStatus {
|
|
ready_roots: vec![selected_root],
|
|
warnings: Vec::new(),
|
|
}
|
|
);
|
|
|
|
Ok(())
|
|
}
|