mirror of
https://github.com/openai/codex.git
synced 2026-10-25 18:28:38 +00:00
Expose a stable executor build identity in environment metadata (#43513)
## Why Executor compatibility checks need a build identity that distinguishes commits and compiler targets independently of the package release version. ## What changed - Add optional `providerId` to exec-server environment metadata, cached at startup and returned by initialization and `environment/info`. - Derive the ID as SHA-256 of `git:<lowercase commit>:<target>`. It identifies a standard build configuration, not exact executable bytes, and is omitted when the commit stamp or target is unavailable or invalid. - Embed the compiler target in `BuildInfo`, preserve compatibility with historical metadata without a target, and stamp Cargo release builds with `STABLE_GIT_COMMIT`. - Group Bazel build-script argument files under one `--arg-file` flag to reduce Windows command-line length while preserving file order. ## Testing Add deterministic build-ID vectors across targets, commit normalization and invalid input coverage, and historical metadata compatibility tests. Extend exec-server coverage for metadata caching and ensure runtime environment overrides cannot replace the executor's build identity. GitOrigin-RevId: 125a18c23de7ad006571940ba305836376c983f4
This commit is contained in:
@@ -193,6 +193,7 @@ Response:
|
||||
"environmentInfo": {
|
||||
"shell": { "name": "bash", "path": "/bin/bash" },
|
||||
"executorVersion": "1.2.3-alpha.4",
|
||||
"providerId": "sha256:fb4f62da3e84f6864dcec8ede7bc66f1c96ecaeaf55f8a786b85df994057c8ac",
|
||||
"cwd": "file:///workspace"
|
||||
}
|
||||
}
|
||||
@@ -203,6 +204,12 @@ Response:
|
||||
|
||||
`executorVersion` is the executor's package release version, or `0.0.0` when unknown.
|
||||
|
||||
The executor caches optional `providerId` at startup using
|
||||
`codex_build_info::build_id(commit, target)`, which CI can also call for an
|
||||
explicit build target. This opaque compatibility key excludes package version
|
||||
and requires no manifest. It identifies a standard build configuration, not exact
|
||||
executable bytes. Unstamped and legacy executors may omit it.
|
||||
|
||||
Rust clients cache this metadata for the client's lifetime, including session
|
||||
resumption. If initialization omits it, the first metadata request fetches and
|
||||
caches `environment/info`.
|
||||
|
||||
@@ -2543,8 +2543,9 @@ mod tests {
|
||||
#[test_case::test_case(Some(EnvironmentInfo::local()); "from_initialize")]
|
||||
#[test_case::test_case(Some(EnvironmentInfo {
|
||||
executor_version: "1.2.3-alpha.4".to_string(),
|
||||
provider_id: Some("sha256:fb4f62da3e84f6864dcec8ede7bc66f1c96ecaeaf55f8a786b85df994057c8ac".to_string()),
|
||||
..EnvironmentInfo::local()
|
||||
}); "with_executor_version")]
|
||||
}); "with_executor_metadata")]
|
||||
#[test_case::test_case(None; "legacy_server")]
|
||||
#[tokio::test]
|
||||
async fn environment_info_is_cached(
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
mod build_identity;
|
||||
mod file_system_handler;
|
||||
mod handler;
|
||||
mod process_handler;
|
||||
@@ -50,6 +51,7 @@ pub async fn run_main_with_telemetry(
|
||||
http_client_factory: HttpClientFactory,
|
||||
request_dispatch_mode: RequestDispatchMode,
|
||||
) -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
|
||||
std::sync::LazyLock::force(&build_identity::PROVIDER_ID);
|
||||
transport::run_transport(
|
||||
listen_url,
|
||||
runtime_paths,
|
||||
|
||||
@@ -0,0 +1,21 @@
|
||||
//! Cache the running executor's build identity before accepting connections.
|
||||
|
||||
use std::sync::LazyLock;
|
||||
|
||||
use codex_build_info::BuildInfo;
|
||||
use codex_build_info::build_id;
|
||||
|
||||
use crate::protocol::EnvironmentInfo;
|
||||
|
||||
pub(super) static PROVIDER_ID: LazyLock<Option<String>> = LazyLock::new(|| {
|
||||
let info = BuildInfo::get();
|
||||
info.target()
|
||||
.and_then(|target| build_id(info.build_commit(), target))
|
||||
});
|
||||
|
||||
pub(super) fn local_environment_info() -> EnvironmentInfo {
|
||||
EnvironmentInfo {
|
||||
provider_id: PROVIDER_ID.clone(),
|
||||
..super::release_version::local_environment_info()
|
||||
}
|
||||
}
|
||||
@@ -67,6 +67,7 @@ use crate::rpc::RpcNotificationSender;
|
||||
use crate::rpc::internal_error;
|
||||
use crate::rpc::invalid_params;
|
||||
use crate::rpc::invalid_request;
|
||||
use crate::server::build_identity::local_environment_info;
|
||||
use crate::server::file_system_handler::FileSystemHandler;
|
||||
use crate::server::session_registry::SessionHandle;
|
||||
use crate::server::session_registry::SessionRegistry;
|
||||
@@ -162,7 +163,7 @@ impl ExecServerHandler {
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner) = Some(session);
|
||||
Ok(InitializeResponse {
|
||||
session_id,
|
||||
environment_info: Some(super::release_version::local_environment_info()),
|
||||
environment_info: Some(local_environment_info()),
|
||||
})
|
||||
}
|
||||
|
||||
@@ -197,7 +198,7 @@ impl ExecServerHandler {
|
||||
|
||||
pub(crate) fn environment_info(&self) -> Result<EnvironmentInfo, JSONRPCErrorError> {
|
||||
self.require_initialized_for("environment info")?;
|
||||
Ok(super::release_version::local_environment_info())
|
||||
Ok(local_environment_info())
|
||||
}
|
||||
|
||||
pub(crate) async fn environment_config_read(
|
||||
|
||||
@@ -21,6 +21,8 @@ use ctor::ctor;
|
||||
|
||||
pub(crate) mod exec_server;
|
||||
|
||||
pub(crate) const TEST_BUILD_COMMIT: &str = "0123456789abcdef0123456789abcdef01234567";
|
||||
|
||||
pub(crate) const DELAYED_OUTPUT_AFTER_EXIT_PARENT_ARG: &str =
|
||||
"--codex-test-delayed-output-after-exit-parent";
|
||||
pub(crate) const SYSTEM_PROXY_REQUEST_URL_ENV: &str =
|
||||
@@ -150,6 +152,8 @@ fn maybe_run_exec_server_from_test_binary(guard: Option<&TestBinaryDispatchGuard
|
||||
if command != "exec-server" {
|
||||
return;
|
||||
}
|
||||
// Initialize in the executor child, just as the real CLI does at startup.
|
||||
codex_build_info::BuildInfo::initialize(TEST_BUILD_COMMIT);
|
||||
|
||||
let Some(flag) = args.next() else {
|
||||
eprintln!("expected --listen");
|
||||
|
||||
@@ -38,7 +38,9 @@ async fn remote_environment_fetches_info_from_exec_server() -> anyhow::Result<()
|
||||
assert!(environment.is_remote());
|
||||
|
||||
let remote_info = environment.info().await?;
|
||||
let local_info = Environment::default_for_tests().info().await?;
|
||||
let mut local_info = Environment::default_for_tests().info().await?;
|
||||
// Only the remote executor advertises its optional build identity.
|
||||
local_info.provider_id = remote_info.provider_id.clone();
|
||||
assert_eq!(remote_info, local_info);
|
||||
|
||||
server.shutdown().await?;
|
||||
|
||||
@@ -1,5 +1,8 @@
|
||||
mod common;
|
||||
|
||||
use anyhow::Context;
|
||||
use codex_build_info::BuildInfo;
|
||||
use codex_build_info::build_id;
|
||||
use codex_exec_server::EnvironmentInfo;
|
||||
use codex_exec_server::InitializeParams;
|
||||
use codex_exec_server::InitializeResponse;
|
||||
@@ -7,6 +10,7 @@ use codex_exec_server_protocol::JSONRPCError;
|
||||
use codex_exec_server_protocol::JSONRPCErrorError;
|
||||
use codex_exec_server_protocol::JSONRPCMessage;
|
||||
use codex_exec_server_protocol::JSONRPCResponse;
|
||||
use common::TEST_BUILD_COMMIT;
|
||||
use common::exec_server::ExecServerHarness;
|
||||
use common::exec_server::exec_server_with_env;
|
||||
use pretty_assertions::assert_eq;
|
||||
@@ -33,6 +37,15 @@ async fn exec_server_accepts_initialize(version: Option<&str>) -> anyhow::Result
|
||||
|
||||
let mut command = Command::new(&executable);
|
||||
command.args(["exec-server", "--listen", "ws://127.0.0.1:0"]);
|
||||
// Runtime environment variables cannot replace the executable's build stamp.
|
||||
command.envs([
|
||||
(
|
||||
"STABLE_GIT_COMMIT",
|
||||
"ffffffffffffffffffffffffffffffffffffffff",
|
||||
),
|
||||
("GITHUB_SHA", "ffffffffffffffffffffffffffffffffffffffff"),
|
||||
("CODEX_BUILD_TARGET", "runtime-override"),
|
||||
]);
|
||||
let mut server = ExecServerHarness::start(command).await?;
|
||||
|
||||
// Updates after startup cannot change the advertised release version.
|
||||
@@ -56,6 +69,12 @@ async fn exec_server_accepts_initialize(version: Option<&str>) -> anyhow::Result
|
||||
Uuid::parse_str(&initialize_response.session_id)?;
|
||||
let mut expected_environment = EnvironmentInfo::local();
|
||||
expected_environment.executor_version = version.unwrap_or("0.0.0").to_string();
|
||||
let build_info = BuildInfo::get();
|
||||
let target = build_info
|
||||
.target()
|
||||
.context("the test binary has a compiled target")?;
|
||||
expected_environment.provider_id = build_id(TEST_BUILD_COMMIT, target);
|
||||
assert!(expected_environment.provider_id.is_some());
|
||||
assert_eq!(
|
||||
initialize_response.environment_info,
|
||||
Some(expected_environment.clone())
|
||||
|
||||
@@ -134,7 +134,7 @@ async fn exec_server_runs_ordinary_requests_serially_by_default() -> anyhow::Res
|
||||
})?,
|
||||
)
|
||||
.await?;
|
||||
let _ = server
|
||||
let response = server
|
||||
.wait_for_event(|event| {
|
||||
matches!(
|
||||
event,
|
||||
@@ -142,6 +142,10 @@ async fn exec_server_runs_ordinary_requests_serially_by_default() -> anyhow::Res
|
||||
)
|
||||
})
|
||||
.await?;
|
||||
let JSONRPCMessage::Response(JSONRPCResponse { result, .. }) = response else {
|
||||
panic!("expected initialize response");
|
||||
};
|
||||
let initialization: InitializeResponse = serde_json::from_value(result)?;
|
||||
server
|
||||
.send_notification("initialized", serde_json::json!({}))
|
||||
.await?;
|
||||
@@ -213,6 +217,9 @@ async fn exec_server_runs_ordinary_requests_serially_by_default() -> anyhow::Res
|
||||
};
|
||||
assert_eq!(id, queued_environment_info_id);
|
||||
let mut expected_environment_info = EnvironmentInfo::local();
|
||||
expected_environment_info.provider_id = initialization
|
||||
.environment_info
|
||||
.and_then(|info| info.provider_id);
|
||||
expected_environment_info.temporary_directories = Some(vec![PathUri::from_host_native_path(
|
||||
temporary_directory.path(),
|
||||
)?]);
|
||||
|
||||
Reference in New Issue
Block a user