Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions codex-rs/app-server/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
15 changes: 15 additions & 0 deletions codex-rs/app-server/tests/common/test_app_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -1939,6 +1940,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")
Expand Down
1 change: 1 addition & 0 deletions codex-rs/core/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
7 changes: 7 additions & 0 deletions codex-rs/core/src/test_support.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
10 changes: 10 additions & 0 deletions codex-rs/core/src/thread_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
21 changes: 21 additions & 0 deletions codex-rs/core/tests/common/test_codex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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")))
Expand Down Expand Up @@ -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());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Build or disable the host in Cargo tests

Under the documented local path just test -p codex-core, Cargo does not build a binary from the separate codex-code-mode-host package, so this .ok() path is usually None; because ThreadManager::new already selected the process-owned provider from the now-default code_mode_host, code-mode tests then try to spawn codex-code-mode-host next to the test executable and report tool errors instead of exercising the runtime. The builder needs to ensure a real host binary is available for scoped Cargo runs, or explicitly disable CodeModeHost before constructing the manager when it is not.

AGENTS.md reference: AGENTS.md:L64-L68

Useful? React with 👍 / 👎.

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();

Expand Down Expand Up @@ -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,
}
}

Expand Down
28 changes: 21 additions & 7 deletions codex-rs/core/tests/suite/code_mode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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()
Expand Down
4 changes: 2 additions & 2 deletions codex-rs/features/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Honor per-thread code-mode-host opt-outs

With code_mode_host now defaulting on, a long-lived app-server/TUI process constructs ThreadManager once from the startup config, but each thread/start later reloads and passes its own config overrides to start_thread_with_options; if that per-thread config sets features.code_mode_host=false while using code mode, the session still receives the already-selected process-owned provider and cannot opt back into the in-process runtime. This makes the advertised opt-out ineffective for app-server config overrides unless the provider choice is moved to thread/session creation or rebuilt from the thread config.

AGENTS.md reference: AGENTS.md:L102-L110

Useful? React with 👍 / 👎.

},
FeatureSpec {
id: Feature::CodeModeOnly,
Expand Down
10 changes: 10 additions & 0 deletions codex-rs/features/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
3 changes: 2 additions & 1 deletion defs.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
89 changes: 63 additions & 26 deletions workspace_root_test_launcher.bat.tpl
Original file line number Diff line number Diff line change
@@ -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%"
Expand Down Expand Up @@ -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
Loading