mirror of
https://github.com/openai/codex.git
synced 2026-08-23 13:09:46 +00:00
code-mode: move to hosted mode by default (#31500)
## Summary - Promote code_mode_host to stable and enable it by default. - Preserve features.code_mode_host = false as an opt-out to the in-process runtime. - Run core code-mode tests through the standalone host. - Keep explicit coverage for missing-host failures.
This commit is contained in:
@@ -5,6 +5,7 @@ codex_rust_crate(
|
||||
crate_name = "codex_app_server",
|
||||
extra_binaries = [
|
||||
"//codex-rs/bwrap:bwrap",
|
||||
"//codex-rs/code-mode-host:codex-code-mode-host",
|
||||
],
|
||||
extra_binaries_non_windows = [
|
||||
"//codex-rs/cli:codex",
|
||||
|
||||
@@ -147,6 +147,7 @@ pub struct TestAppServer {
|
||||
pub const DEFAULT_CLIENT_NAME: &str = "codex-app-server-tests";
|
||||
pub const DISABLE_PLUGIN_STARTUP_TASKS_ARG: &str = "--disable-plugin-startup-tasks-for-tests";
|
||||
const DISABLE_MANAGED_CONFIG_ENV_VAR: &str = "CODEX_APP_SERVER_DISABLE_MANAGED_CONFIG";
|
||||
const CODE_MODE_HOST_PATH_ENV_VAR: &str = "CODEX_CODE_MODE_HOST_PATH";
|
||||
|
||||
impl TestAppServer {
|
||||
/// Starts building a server with a temporary CODEX_HOME and the standard
|
||||
@@ -1819,6 +1820,20 @@ impl TestAppServerBuilder {
|
||||
}
|
||||
TestAppServerEnvironment::None => None,
|
||||
};
|
||||
if !env_overrides
|
||||
.iter()
|
||||
.any(|(key, _)| key == CODE_MODE_HOST_PATH_ENV_VAR)
|
||||
&& let Ok(code_mode_host_program) =
|
||||
codex_utils_cargo_bin::cargo_bin("codex-code-mode-host")
|
||||
{
|
||||
env_overrides.insert(
|
||||
0,
|
||||
(
|
||||
CODE_MODE_HOST_PATH_ENV_VAR.to_string(),
|
||||
Some(code_mode_host_program.to_string_lossy().into_owned()),
|
||||
),
|
||||
);
|
||||
}
|
||||
let program = match program {
|
||||
Some(program) => program,
|
||||
None => codex_utils_cargo_bin::cargo_bin("codex-app-server")
|
||||
|
||||
@@ -14,6 +14,7 @@ codex_rust_crate(
|
||||
crate_name = "codex_core",
|
||||
extra_binaries = [
|
||||
"//codex-rs/bwrap:bwrap",
|
||||
"//codex-rs/code-mode-host:codex-code-mode-host",
|
||||
"//codex-rs/linux-sandbox:codex-linux-sandbox",
|
||||
"//codex-rs/rmcp-client:test_stdio_server",
|
||||
"//codex-rs/rmcp-client:test_streamable_http_server",
|
||||
|
||||
@@ -71,6 +71,13 @@ pub fn auth_manager_from_auth_with_home(auth: CodexAuth, codex_home: PathBuf) ->
|
||||
AuthManager::from_auth_for_testing_with_home(auth, codex_home)
|
||||
}
|
||||
|
||||
pub fn with_code_mode_host_program(
|
||||
thread_manager: ThreadManager,
|
||||
host_program: PathBuf,
|
||||
) -> ThreadManager {
|
||||
thread_manager.with_code_mode_host_program_for_tests(host_program)
|
||||
}
|
||||
|
||||
pub fn thread_manager_with_models_provider(
|
||||
auth: CodexAuth,
|
||||
provider: ModelProviderInfo,
|
||||
|
||||
@@ -364,6 +364,16 @@ impl ThreadManager {
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn with_code_mode_host_program_for_tests(mut self, host_program: PathBuf) -> Self {
|
||||
let Some(state) = Arc::get_mut(&mut self.state) else {
|
||||
unreachable!("new thread manager state should not be shared");
|
||||
};
|
||||
state.code_mode_session_provider = Arc::new(
|
||||
ProcessOwnedCodeModeSessionProvider::with_host_program(host_program),
|
||||
);
|
||||
self
|
||||
}
|
||||
|
||||
/// Construct with a dummy AuthManager containing the provided CodexAuth.
|
||||
/// Used for integration tests: should not be used by ordinary business logic.
|
||||
pub(crate) fn with_models_provider_for_tests(
|
||||
|
||||
@@ -290,6 +290,7 @@ pub struct TestCodexBuilder {
|
||||
user_instructions_provider: Option<Arc<dyn UserInstructionsProvider>>,
|
||||
supports_openai_form_elicitation: bool,
|
||||
external_time_provider: Option<Arc<dyn TimeProvider>>,
|
||||
code_mode_host_program: Option<PathBuf>,
|
||||
}
|
||||
|
||||
impl TestCodexBuilder {
|
||||
@@ -396,6 +397,11 @@ impl TestCodexBuilder {
|
||||
self
|
||||
}
|
||||
|
||||
pub fn with_code_mode_host_program(mut self, host_program: PathBuf) -> Self {
|
||||
self.code_mode_host_program = Some(host_program);
|
||||
self
|
||||
}
|
||||
|
||||
pub fn with_windows_cmd_shell(self) -> Self {
|
||||
if cfg!(windows) {
|
||||
self.with_user_shell(get_shell_by_model_provided_path(&PathBuf::from("cmd.exe")))
|
||||
@@ -613,6 +619,20 @@ impl TestCodexBuilder {
|
||||
/*attestation_provider*/ None,
|
||||
/*external_time_provider*/ self.external_time_provider.clone(),
|
||||
);
|
||||
let code_mode_host_program = self
|
||||
.code_mode_host_program
|
||||
.take()
|
||||
.or_else(|| codex_utils_cargo_bin::cargo_bin("codex-code-mode-host").ok());
|
||||
let thread_manager = if config.features.enabled(Feature::CodeModeHost)
|
||||
&& let Some(code_mode_host_program) = code_mode_host_program
|
||||
{
|
||||
codex_core::test_support::with_code_mode_host_program(
|
||||
thread_manager,
|
||||
code_mode_host_program,
|
||||
)
|
||||
} else {
|
||||
thread_manager
|
||||
};
|
||||
let thread_manager = Arc::new(thread_manager);
|
||||
let user_shell_override = self.user_shell_override.clone();
|
||||
|
||||
@@ -1219,6 +1239,7 @@ pub fn test_codex() -> TestCodexBuilder {
|
||||
user_instructions_provider: None,
|
||||
supports_openai_form_elicitation: false,
|
||||
external_time_provider: None,
|
||||
code_mode_host_program: None,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -43,6 +43,7 @@ use core_test_support::skip_if_no_network;
|
||||
use core_test_support::skip_if_wine_exec;
|
||||
use core_test_support::stdio_server_bin;
|
||||
use core_test_support::test_codex::TestCodex;
|
||||
use core_test_support::test_codex::TestCodexBuilder;
|
||||
use core_test_support::test_codex::test_codex;
|
||||
use core_test_support::test_codex::turn_permission_fields;
|
||||
use core_test_support::wait_for_event;
|
||||
@@ -190,10 +191,19 @@ async fn run_code_mode_turn_with_model_and_config(
|
||||
model: &'static str,
|
||||
configure: impl FnOnce(&mut Config) + Send + 'static,
|
||||
) -> Result<(TestCodex, ResponseMock)> {
|
||||
let mut builder = test_codex().with_model(model).with_config(move |config| {
|
||||
let builder = test_codex().with_model(model).with_config(move |config| {
|
||||
let _ = config.features.enable(Feature::CodeMode);
|
||||
configure(config);
|
||||
});
|
||||
run_code_mode_turn_with_builder(server, prompt, code, builder).await
|
||||
}
|
||||
|
||||
async fn run_code_mode_turn_with_builder(
|
||||
server: &MockServer,
|
||||
prompt: &str,
|
||||
code: &str,
|
||||
mut builder: TestCodexBuilder,
|
||||
) -> Result<(TestCodex, ResponseMock)> {
|
||||
let test = builder.build(server).await?;
|
||||
|
||||
responses::mount_sse_once(
|
||||
@@ -224,14 +234,18 @@ async fn missing_process_host_returns_a_tool_error() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = responses::start_mock_server().await;
|
||||
let (_test, follow_up_mock) =
|
||||
run_code_mode_turn_with_config(&server, "Run code mode", "text('unreachable')", |config| {
|
||||
let builder = test_codex()
|
||||
.with_model("test-gpt-5.1-codex")
|
||||
.with_code_mode_host_program("codex-code-mode-host-does-not-exist".into())
|
||||
.with_config(|config| {
|
||||
config
|
||||
.features
|
||||
.enable(Feature::CodeModeHost)
|
||||
.expect("code mode host should be enabled");
|
||||
})
|
||||
.await?;
|
||||
.enable(Feature::CodeMode)
|
||||
.expect("code mode should be enabled");
|
||||
});
|
||||
let (_test, follow_up_mock) =
|
||||
run_code_mode_turn_with_builder(&server, "Run code mode", "text('unreachable')", builder)
|
||||
.await?;
|
||||
|
||||
let output = follow_up_mock
|
||||
.single_request()
|
||||
|
||||
@@ -857,8 +857,8 @@ pub const FEATURES: &[FeatureSpec] = &[
|
||||
FeatureSpec {
|
||||
id: Feature::CodeModeHost,
|
||||
key: "code_mode_host",
|
||||
stage: Stage::UnderDevelopment,
|
||||
default_enabled: false,
|
||||
stage: Stage::Stable,
|
||||
default_enabled: true,
|
||||
},
|
||||
FeatureSpec {
|
||||
id: Feature::CodeModeOnly,
|
||||
|
||||
@@ -123,6 +123,16 @@ fn code_mode_only_requires_code_mode() {
|
||||
assert_eq!(features.enabled(Feature::CodeMode), true);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn code_mode_host_is_stable_and_enabled_by_default() {
|
||||
assert_eq!(Feature::CodeModeHost.stage(), Stage::Stable);
|
||||
assert_eq!(Feature::CodeModeHost.default_enabled(), true);
|
||||
assert_eq!(
|
||||
feature_for_key("code_mode_host"),
|
||||
Some(Feature::CodeModeHost)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn guardian_approval_is_stable_and_enabled_by_default() {
|
||||
let spec = Feature::GuardianApproval.info();
|
||||
|
||||
3
defs.bzl
3
defs.bzl
@@ -110,8 +110,9 @@ def _windows_runfile_env_exports(ctx):
|
||||
lines = []
|
||||
for runfile_dep, env_var in ctx.attr.runfile_env.items():
|
||||
runfile = _runfile_env_file(runfile_dep)
|
||||
lines.append('call :resolve_runfile {} "{}"'.format(env_var, _runfile_logical_path(runfile)))
|
||||
lines.append('call :resolve_runfile "{}"'.format(_runfile_logical_path(runfile)))
|
||||
lines.append("if errorlevel 1 exit /b 1")
|
||||
lines.append('set "{}=!resolve_runfile_result!"'.format(env_var))
|
||||
return "\n".join(lines)
|
||||
|
||||
def _runfile_env_file(target):
|
||||
|
||||
@@ -1,17 +1,24 @@
|
||||
@echo off
|
||||
setlocal EnableExtensions EnableDelayedExpansion
|
||||
|
||||
call :resolve_runfile workspace_root_marker "__WORKSPACE_ROOT_MARKER__"
|
||||
call :resolve_runfile "__WORKSPACE_ROOT_MARKER__"
|
||||
if errorlevel 1 exit /b 1
|
||||
set "workspace_root_marker=!resolve_runfile_result!"
|
||||
|
||||
for %%I in ("%workspace_root_marker%") do set "workspace_root_marker_dir=%%~dpI"
|
||||
for %%I in ("%workspace_root_marker_dir%..\..") do set "workspace_root=%%~fI"
|
||||
|
||||
call :resolve_runfile test_bin "__TEST_BIN__"
|
||||
call :resolve_runfile "__TEST_BIN__"
|
||||
if errorlevel 1 exit /b 1
|
||||
set "test_bin=!resolve_runfile_result!"
|
||||
|
||||
__RUNFILE_ENV_EXPORTS__
|
||||
|
||||
if not defined test_bin (
|
||||
>&2 echo resolved test binary was lost while exporting runfile environment variables
|
||||
exit /b 1
|
||||
)
|
||||
|
||||
__WORKSPACE_ROOT_SETUP__
|
||||
|
||||
set "TOTAL_SHARDS=%RULES_RUST_TEST_TOTAL_SHARDS%"
|
||||
@@ -97,44 +104,74 @@ rmdir /s /q "!TEMP_DIR!" 2>nul
|
||||
exit /b !TEST_EXIT!
|
||||
|
||||
:resolve_runfile
|
||||
setlocal EnableExtensions EnableDelayedExpansion
|
||||
set "logical_path=%~2"
|
||||
set "workspace_logical_path=%logical_path%"
|
||||
if defined TEST_WORKSPACE set "workspace_logical_path=%TEST_WORKSPACE%/%logical_path%"
|
||||
set "native_logical_path=%logical_path:/=\%"
|
||||
set "native_workspace_logical_path=%workspace_logical_path:/=\%"
|
||||
set "resolve_runfile_result="
|
||||
set "resolve_runfile_logical_path=%~1"
|
||||
set "resolve_runfile_workspace_logical_path=!resolve_runfile_logical_path!"
|
||||
if defined TEST_WORKSPACE set "resolve_runfile_workspace_logical_path=%TEST_WORKSPACE%/!resolve_runfile_logical_path!"
|
||||
set "resolve_runfile_native_logical_path=!resolve_runfile_logical_path:/=\!"
|
||||
set "resolve_runfile_native_workspace_logical_path=!resolve_runfile_workspace_logical_path:/=\!"
|
||||
|
||||
for %%R in ("%RUNFILES_DIR%" "%TEST_SRCDIR%") do (
|
||||
set "runfiles_root=%%~R"
|
||||
if defined runfiles_root (
|
||||
if exist "!runfiles_root!\!native_logical_path!" (
|
||||
endlocal & set "%~1=!runfiles_root!\!native_logical_path!" & exit /b 0
|
||||
set "resolve_runfile_root=%%~R"
|
||||
if defined resolve_runfile_root (
|
||||
if exist "!resolve_runfile_root!\!resolve_runfile_native_logical_path!" (
|
||||
set "resolve_runfile_result=!resolve_runfile_root!\!resolve_runfile_native_logical_path!"
|
||||
goto :resolve_runfile_success
|
||||
)
|
||||
if exist "!runfiles_root!\!native_workspace_logical_path!" (
|
||||
endlocal & set "%~1=!runfiles_root!\!native_workspace_logical_path!" & exit /b 0
|
||||
if exist "!resolve_runfile_root!\!resolve_runfile_native_workspace_logical_path!" (
|
||||
set "resolve_runfile_result=!resolve_runfile_root!\!resolve_runfile_native_workspace_logical_path!"
|
||||
goto :resolve_runfile_success
|
||||
)
|
||||
)
|
||||
)
|
||||
|
||||
set "manifest=%RUNFILES_MANIFEST_FILE%"
|
||||
if not defined manifest if exist "%~f0.runfiles_manifest" set "manifest=%~f0.runfiles_manifest"
|
||||
if not defined manifest if exist "%~dpn0.runfiles_manifest" set "manifest=%~dpn0.runfiles_manifest"
|
||||
if not defined manifest if exist "%~f0.exe.runfiles_manifest" set "manifest=%~f0.exe.runfiles_manifest"
|
||||
set "resolve_runfile_manifest=%RUNFILES_MANIFEST_FILE%"
|
||||
if not defined resolve_runfile_manifest if exist "%~f0.runfiles_manifest" set "resolve_runfile_manifest=%~f0.runfiles_manifest"
|
||||
if not defined resolve_runfile_manifest if exist "%~dpn0.runfiles_manifest" set "resolve_runfile_manifest=%~dpn0.runfiles_manifest"
|
||||
if not defined resolve_runfile_manifest if exist "%~f0.exe.runfiles_manifest" set "resolve_runfile_manifest=%~f0.exe.runfiles_manifest"
|
||||
|
||||
if defined manifest if exist "%manifest%" (
|
||||
if defined resolve_runfile_manifest if exist "!resolve_runfile_manifest!" (
|
||||
rem Read the manifest directly instead of shelling out to findstr. In the
|
||||
rem GitHub Windows runner, the nested `findstr` path produced
|
||||
rem `FINDSTR: Cannot open D:MANIFEST`, which then broke runfile resolution for
|
||||
rem Bazel tests even though the manifest file was present.
|
||||
for /f "usebackq tokens=1,* delims= " %%A in ("%manifest%") do (
|
||||
if "%%A"=="%logical_path%" (
|
||||
endlocal & set "%~1=%%B" & exit /b 0
|
||||
rem A one-field manifest entry maps to itself, so fall back to %%A when the
|
||||
rem optional mapped path in %%B is empty.
|
||||
for /f "usebackq tokens=1,* delims= " %%A in ("!resolve_runfile_manifest!") do (
|
||||
if "%%A"=="!resolve_runfile_logical_path!" (
|
||||
set "resolve_runfile_manifest_path=%%B"
|
||||
if not defined resolve_runfile_manifest_path set "resolve_runfile_manifest_path=%%A"
|
||||
set "resolve_runfile_result=!resolve_runfile_manifest_path!"
|
||||
goto :resolve_runfile_success
|
||||
)
|
||||
if "%%A"=="%workspace_logical_path%" (
|
||||
endlocal & set "%~1=%%B" & exit /b 0
|
||||
if "%%A"=="!resolve_runfile_workspace_logical_path!" (
|
||||
set "resolve_runfile_manifest_path=%%B"
|
||||
if not defined resolve_runfile_manifest_path set "resolve_runfile_manifest_path=%%A"
|
||||
set "resolve_runfile_result=!resolve_runfile_manifest_path!"
|
||||
goto :resolve_runfile_success
|
||||
)
|
||||
)
|
||||
)
|
||||
|
||||
>&2 echo failed to resolve runfile: %logical_path%
|
||||
endlocal & exit /b 1
|
||||
>&2 echo failed to resolve runfile: !resolve_runfile_logical_path!
|
||||
call :clear_resolve_runfile_state
|
||||
exit /b 1
|
||||
|
||||
:resolve_runfile_success
|
||||
if not defined resolve_runfile_result (
|
||||
>&2 echo resolved runfile has an empty path: !resolve_runfile_logical_path!
|
||||
call :clear_resolve_runfile_state
|
||||
exit /b 1
|
||||
)
|
||||
call :clear_resolve_runfile_state
|
||||
exit /b 0
|
||||
|
||||
:clear_resolve_runfile_state
|
||||
set "resolve_runfile_logical_path="
|
||||
set "resolve_runfile_workspace_logical_path="
|
||||
set "resolve_runfile_native_logical_path="
|
||||
set "resolve_runfile_native_workspace_logical_path="
|
||||
set "resolve_runfile_root="
|
||||
set "resolve_runfile_manifest="
|
||||
set "resolve_runfile_manifest_path="
|
||||
exit /b 0
|
||||
|
||||
Reference in New Issue
Block a user