mirror of
https://github.com/openai/codex.git
synced 2026-10-28 18:58:46 +00:00
Gate capability discovery V2 on executor version (#57192)
## Why Executors older than `0.164.0` can advertise V2 discovery support without implementing the final contract. Enabling V2 also suppressed V1 discovery for incompatible executors, hiding selected capability roots and skills. ## What changed Require both the V2 capability flag and an executor version of at least `0.164.0` before issuing `capabilities/discoverV2`. Share this compatibility check between session discovery and the exec-server client. Fall back to V1 when incompatible executors provide no V2 snapshots, clearing any previously stored V2 inventory. ## Testing Add coverage for version boundaries, prereleases, missing and malformed versions, and transport recovery for compatible executors. Verify that an older executor retains selected plugins and skill instructions through V1 with V2 enabled. GitOrigin-RevId: 93adf52fc91f02b58fd14f8ff3b093712b3346b8
This commit is contained in:
committed by
copyberry
parent
245691b2dc
commit
6654f7a7ab
@@ -135,7 +135,13 @@ impl NoiseStreamHandler for NotifyingProcessor {
|
||||
ConnectionProcessor::decode(payload)
|
||||
}
|
||||
|
||||
fn encode(message: Self::Outgoing) -> Result<NoiseOutboundMessage, ExecServerError> {
|
||||
fn encode(mut message: Self::Outgoing) -> Result<NoiseOutboundMessage, ExecServerError> {
|
||||
// Advertise the stable contract implemented by this test executor.
|
||||
if let codex_exec_server_protocol::JSONRPCMessage::Response(response) = &mut message
|
||||
&& let Some(info) = response.result.get_mut("environmentInfo")
|
||||
{
|
||||
info["executorVersion"] = serde_json::json!("0.164.0");
|
||||
}
|
||||
ConnectionProcessor::encode(message)
|
||||
}
|
||||
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
mod connect_options;
|
||||
mod discovery_recovery;
|
||||
mod discovery_v2;
|
||||
pub use discovery_v2::supports_capability_discovery_v2;
|
||||
|
||||
use std::collections::HashMap;
|
||||
use std::collections::HashSet;
|
||||
|
||||
@@ -3,7 +3,16 @@
|
||||
use super::Environment;
|
||||
use crate::DiscoverV2CapabilitiesRequest;
|
||||
use crate::DiscoverV2CapabilitiesResponse;
|
||||
use crate::EnvironmentInfo;
|
||||
use crate::ExecServerError;
|
||||
use codex_build_info::BuildInfo;
|
||||
|
||||
/// The final DiscoveryV2 contract starts at stable 0.164.0.
|
||||
pub fn supports_capability_discovery_v2(info: &EnvironmentInfo) -> bool {
|
||||
let build = BuildInfo::from_version(&info.executor_version);
|
||||
let minimum = BuildInfo::from_version("0.164.0");
|
||||
info.capabilities.capability_discovery_v2 && build.version() >= minimum.version()
|
||||
}
|
||||
|
||||
impl Environment {
|
||||
/// Identifies the initialized executor session; transport recovery preserves this ID.
|
||||
@@ -28,12 +37,8 @@ impl Environment {
|
||||
let connection_state = client.subscribe_connection_state();
|
||||
let client = client.get().await?;
|
||||
let discover = || async {
|
||||
if !client
|
||||
.environment_info()
|
||||
.await?
|
||||
.capabilities
|
||||
.capability_discovery_v2
|
||||
{
|
||||
let info = client.environment_info().await?;
|
||||
if !supports_capability_discovery_v2(&info) {
|
||||
return Err(ExecServerError::Protocol(
|
||||
"capabilities/discoverV2 is unavailable in this environment session"
|
||||
.to_string(),
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
//! Real transport recovery preserves a discovery request's cwd and retries it once.
|
||||
//! Version compatibility gates discovery RPCs; supported clients retain transport recovery.
|
||||
|
||||
use super::DiscoverV2CapabilitiesRequest;
|
||||
use super::DiscoverV2CapabilitiesResponse;
|
||||
@@ -21,8 +21,18 @@ use tokio_tungstenite::accept_async;
|
||||
use tokio_tungstenite::tungstenite::Message;
|
||||
use tokio_util::task::AbortOnDropHandle;
|
||||
|
||||
#[test_case::test_case("0.164.0", true; "minimum_stable")]
|
||||
#[test_case::test_case("0.168.0+build", true; "newer_stable")]
|
||||
#[test_case::test_case("0.163.0", false; "older")]
|
||||
#[test_case::test_case("0.164.0-alpha.1", false; "prerelease_before_cutoff")]
|
||||
#[test_case::test_case("0.165.0-alpha.1", true; "next_minor_prerelease")]
|
||||
#[test_case::test_case("", false; "missing")]
|
||||
#[test_case::test_case("invalid", false; "malformed")]
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn discovery_retries_disconnect_within_the_same_request() -> anyhow::Result<()> {
|
||||
async fn discovery_checks_version_and_retries_disconnect(
|
||||
version: &'static str,
|
||||
supported: bool,
|
||||
) -> anyhow::Result<()> {
|
||||
let listener = TcpListener::bind("127.0.0.1:0").await?;
|
||||
let url = format!("ws://{}", listener.local_addr()?);
|
||||
let attempts = Arc::new(Mutex::new(Vec::new()));
|
||||
@@ -38,10 +48,13 @@ async fn discovery_retries_disconnect_within_the_same_request() -> anyhow::Resul
|
||||
if request.get("id").is_none() {
|
||||
continue;
|
||||
}
|
||||
let info = json!({
|
||||
let mut info = json!({
|
||||
"shell":{"name":"bash","path":"/bin/bash"},
|
||||
"capabilities":{"capabilityDiscoveryV2":true}
|
||||
});
|
||||
if !version.is_empty() {
|
||||
info["executorVersion"] = json!(version);
|
||||
}
|
||||
let result = match request["method"].as_str() {
|
||||
Some("initialize") => {
|
||||
// V2 support is advertised by the executor, not negotiated by the client.
|
||||
@@ -87,8 +100,17 @@ async fn discovery_retries_disconnect_within_the_same_request() -> anyhow::Resul
|
||||
Duration::from_secs(12),
|
||||
environment.discover_capabilities_v2(params.clone()),
|
||||
)
|
||||
.await??;
|
||||
assert_eq!(response, DiscoverV2CapabilitiesResponse::default());
|
||||
.await?;
|
||||
if !supported {
|
||||
assert!(
|
||||
matches!(&response, Err(crate::ExecServerError::Protocol(message))
|
||||
if message == "capabilities/discoverV2 is unavailable in this environment session"),
|
||||
"unexpected discovery response: {response:?}"
|
||||
);
|
||||
assert_eq!(*attempts.lock().await, Vec::<serde_json::Value>::new());
|
||||
return Ok(());
|
||||
}
|
||||
assert_eq!(response?, DiscoverV2CapabilitiesResponse::default());
|
||||
assert_eq!(
|
||||
*attempts.lock().await,
|
||||
vec![
|
||||
|
||||
@@ -125,6 +125,7 @@ pub use environment::LOCAL_ENVIRONMENT_ID;
|
||||
pub use environment::MAX_SELECTED_CAPABILITY_ROOTS;
|
||||
pub use environment::REMOTE_ENVIRONMENT_ID;
|
||||
pub use environment::RemoteEnvironmentOptions;
|
||||
pub use environment::supports_capability_discovery_v2;
|
||||
pub use environment_bootstrap::PreparedEnvironmentManager;
|
||||
pub use environment_provider::DefaultEnvironmentProvider;
|
||||
pub use environment_provider::EnvironmentProvider;
|
||||
|
||||
Reference in New Issue
Block a user