mirror of
https://github.com/openai/codex.git
synced 2026-11-02 19:43:24 +00:00
Merge branch 'main' into etraut/fix-remote-plugin-scopes
This commit is contained in:
@@ -105,9 +105,9 @@ common:ci --disk_cache=
|
||||
# Shared config for the main Bazel CI workflow.
|
||||
common:ci-bazel --config=ci
|
||||
common:ci-bazel --build_metadata=TAG_workflow=bazel
|
||||
# Bazel CI cross-compiles in several legs, and the V8-backed code-mode tests
|
||||
# are not stable in that setup yet. Keep running the rest of the Rust
|
||||
# integration suites through the workspace-root launcher.
|
||||
# Keep code-mode integration cases out of ordinary Bazel legs. The
|
||||
# Windows-cross config below re-enables them after generating its Windows V8
|
||||
# snapshot on the Windows runner.
|
||||
common:ci-bazel --test_env=CODEX_BAZEL_TEST_SKIP_FILTERS=suite::code_mode::
|
||||
|
||||
# Shared config for Bazel-backed Rust linting.
|
||||
@@ -186,12 +186,16 @@ common:ci-windows-cross --config=ci-windows
|
||||
common:ci-windows-cross --build_metadata=TAG_windows_cross_compile=true
|
||||
common:ci-windows-cross --host_platform=//:rbe
|
||||
common:ci-windows-cross --strategy=TestRunner=local
|
||||
# V8 embeds IsolateData offsets in snapshot builtins; Windows snapshots must be
|
||||
# generated by a Windows mksnapshot binary rather than the Linux RBE host tool.
|
||||
common:ci-windows-cross --strategy=V8Mksnapshot=local
|
||||
common:ci-windows-cross --local_test_jobs=4
|
||||
common:ci-windows-cross --test_env=RUST_TEST_THREADS=1
|
||||
# Native Windows CI still covers the PowerShell tests. The cross-built gnullvm
|
||||
# binaries currently hang in PowerShell AST parser tests when those binaries are
|
||||
# run on the Windows runner.
|
||||
common:ci-windows-cross --test_env=CODEX_BAZEL_TEST_SKIP_FILTERS=suite::code_mode::,powershell
|
||||
# run on the Windows runner. Keep V8-backed code-mode tests enabled except for
|
||||
# the hidden dynamic-tool callback test, which currently times out on Windows.
|
||||
common:ci-windows-cross --test_env=CODEX_BAZEL_TEST_SKIP_FILTERS=powershell,suite::code_mode::code_mode_can_call_hidden_dynamic_tools
|
||||
common:ci-windows-cross --platforms=//:windows_x86_64_gnullvm
|
||||
common:ci-windows-cross --extra_execution_platforms=//:rbe,//:windows_x86_64_msvc
|
||||
common:ci-windows-cross --extra_toolchains=//:windows_gnullvm_tests_on_msvc_host_toolchain
|
||||
|
||||
@@ -4,7 +4,8 @@ set -euo pipefail
|
||||
|
||||
# Run target-discovery queries with the same startup settings as the main
|
||||
# build/test invocation so they can reuse the same Bazel server. Queries only
|
||||
# enumerate labels, so they intentionally do not select CI or remote configs.
|
||||
# enumerate labels, so they intentionally do not select a CI build/test config
|
||||
# or remote execution.
|
||||
|
||||
if [[ $# -lt 2 || "${@: -2:1}" != "--" ]]; then
|
||||
echo "Usage: $0 [<bazel query args>...] -- <query expression>" >&2
|
||||
@@ -14,21 +15,16 @@ fi
|
||||
query_args=("${@:1:$#-2}")
|
||||
query_expression="${@: -1}"
|
||||
|
||||
bazel_startup_args=()
|
||||
if [[ -n "${BAZEL_OUTPUT_USER_ROOT:-}" ]]; then
|
||||
bazel_startup_args+=("--output_user_root=${BAZEL_OUTPUT_USER_ROOT}")
|
||||
fi
|
||||
|
||||
run_bazel() {
|
||||
if [[ "${RUNNER_OS:-}" == "Windows" ]]; then
|
||||
MSYS2_ARG_CONV_EXCL='*' bazel "$@"
|
||||
MSYS2_ARG_CONV_EXCL='*' "$(dirname "${BASH_SOURCE[0]}")/run_bazel_with_buildbuddy.py" "$@"
|
||||
return
|
||||
fi
|
||||
|
||||
bazel "$@"
|
||||
"$(dirname "${BASH_SOURCE[0]}")/run_bazel_with_buildbuddy.py" "$@"
|
||||
}
|
||||
|
||||
bazel_query_args=(--noexperimental_remote_repo_contents_cache query)
|
||||
bazel_query_args=(query)
|
||||
|
||||
if [[ -n "${BAZEL_REPO_CONTENTS_CACHE:-}" ]]; then
|
||||
bazel_query_args+=("--repo_contents_cache=${BAZEL_REPO_CONTENTS_CACHE}")
|
||||
@@ -43,8 +39,4 @@ if (( ${#query_args[@]} > 0 )); then
|
||||
fi
|
||||
bazel_query_args+=("$query_expression")
|
||||
|
||||
if (( ${#bazel_startup_args[@]} > 0 )); then
|
||||
run_bazel "${bazel_startup_args[@]}" "${bazel_query_args[@]}"
|
||||
else
|
||||
run_bazel "${bazel_query_args[@]}"
|
||||
fi
|
||||
run_bazel "${bazel_query_args[@]}"
|
||||
|
||||
@@ -22,6 +22,47 @@ REMOTE_EXECUTION_CONFIGS = {
|
||||
"--config=ci-v8",
|
||||
"--config=ci-windows-cross",
|
||||
}
|
||||
# Honor either explicit setting so the wrapper never overrides the caller's
|
||||
# choice when it supplies the CI default below.
|
||||
REMOTE_REPO_CONTENTS_CACHE_STARTUP_OPTIONS = {
|
||||
"--experimental_remote_repo_contents_cache",
|
||||
"--noexperimental_remote_repo_contents_cache",
|
||||
}
|
||||
|
||||
|
||||
def startup_args(args: Sequence[str], env: Mapping[str, str]) -> list[str]:
|
||||
"""Return shared startup options that are missing from a Bazel invocation.
|
||||
|
||||
Bazel startup options must precede the command, and changing them restarts
|
||||
the server and discards its analysis cache. GitHub Actions invokes Bazel
|
||||
through several helpers, so normalize their startup options here while
|
||||
preserving any explicit choice made by the caller.
|
||||
"""
|
||||
command_idx = next(
|
||||
(idx for idx, arg in enumerate(args) if not arg.startswith("-")),
|
||||
len(args),
|
||||
)
|
||||
configured_startup_args = args[:command_idx]
|
||||
injected_args = []
|
||||
|
||||
output_user_root = env.get("BAZEL_OUTPUT_USER_ROOT")
|
||||
if output_user_root and not any(
|
||||
arg.startswith("--output_user_root=") for arg in configured_startup_args
|
||||
):
|
||||
injected_args.append(f"--output_user_root={output_user_root}")
|
||||
|
||||
if env.get("GITHUB_ACTIONS") == "true" and not any(
|
||||
arg in REMOTE_REPO_CONTENTS_CACHE_STARTUP_OPTIONS
|
||||
for arg in configured_startup_args
|
||||
):
|
||||
# Work around Bazel 9 overlay materialization failures seen in CI. This
|
||||
# disables only the startup-level repo contents cache; keyed runs still
|
||||
# use BuildBuddy.
|
||||
injected_args.append("--noexperimental_remote_repo_contents_cache")
|
||||
|
||||
return injected_args
|
||||
|
||||
|
||||
# Only authenticated workflow runs executing trusted upstream code may use the
|
||||
# OpenAI BuildBuddy host. A pull request event without proof that its head is
|
||||
# in the upstream repository fails closed to the generic host.
|
||||
@@ -114,7 +155,7 @@ def bazel_args_with_remote_config(
|
||||
def bazel_command(*args: str, env: Mapping[str, str] | None = None) -> list[str]:
|
||||
env = os.environ if env is None else env
|
||||
bazel = env.get("CODEX_BAZEL_BIN", "bazel")
|
||||
return [bazel, *bazel_args_with_remote_config(args, env)]
|
||||
return [bazel, *startup_args(args, env), *bazel_args_with_remote_config(args, env)]
|
||||
|
||||
|
||||
def main() -> None:
|
||||
|
||||
@@ -182,6 +182,38 @@ class RunBazelWithBuildBuddyTest(unittest.TestCase):
|
||||
["fake-bazel", "info", "execution_root"],
|
||||
)
|
||||
|
||||
def test_bazel_command_normalizes_github_actions_startup_options(self) -> None:
|
||||
env = {
|
||||
"BAZEL_OUTPUT_USER_ROOT": "/tmp/bazel-output",
|
||||
"GITHUB_ACTIONS": "true",
|
||||
}
|
||||
|
||||
self.assertEqual(
|
||||
run_bazel_with_buildbuddy.bazel_command("build", "//codex-rs/...", env=env),
|
||||
[
|
||||
"bazel",
|
||||
"--output_user_root=/tmp/bazel-output",
|
||||
"--noexperimental_remote_repo_contents_cache",
|
||||
"build",
|
||||
"//codex-rs/...",
|
||||
],
|
||||
)
|
||||
self.assertEqual(
|
||||
run_bazel_with_buildbuddy.bazel_command(
|
||||
"--experimental_remote_repo_contents_cache",
|
||||
"build",
|
||||
"//codex-rs/...",
|
||||
env=env,
|
||||
),
|
||||
[
|
||||
"bazel",
|
||||
"--output_user_root=/tmp/bazel-output",
|
||||
"--experimental_remote_repo_contents_cache",
|
||||
"build",
|
||||
"//codex-rs/...",
|
||||
],
|
||||
)
|
||||
|
||||
def test_main_preserves_spaced_argument_and_child_exit_status(self) -> None:
|
||||
spaced_arg = (
|
||||
r"--test_env=PATH=C:\Program Files\PowerShell\7;C:\Program Files\Git\bin"
|
||||
@@ -191,7 +223,9 @@ class RunBazelWithBuildBuddyTest(unittest.TestCase):
|
||||
)
|
||||
env = os.environ.copy()
|
||||
env["CODEX_BAZEL_BIN"] = sys.executable
|
||||
env.pop("BAZEL_OUTPUT_USER_ROOT", None)
|
||||
env.pop("BUILDBUDDY_API_KEY", None)
|
||||
env.pop("GITHUB_ACTIONS", None)
|
||||
|
||||
result = subprocess.run(
|
||||
[
|
||||
|
||||
+13
-13
@@ -20,8 +20,9 @@ jobs:
|
||||
test:
|
||||
# PRs use the sharded Windows cross-compiled test jobs below. Post-merge
|
||||
# pushes to main also run the native Windows test job for broader Windows
|
||||
# signal without putting PR latency back on the critical path. Cargo CI
|
||||
# owns V8/code-mode test coverage for now.
|
||||
# signal without putting PR latency back on the critical path. When
|
||||
# authenticated RBE is available, the Windows-cross shards exercise the
|
||||
# source-built V8/code-mode targets.
|
||||
timeout-minutes: 30
|
||||
strategy:
|
||||
fail-fast: false
|
||||
@@ -91,9 +92,9 @@ jobs:
|
||||
# path. V8 consumers under `//codex-rs/...` still participate
|
||||
# transitively through `//...`.
|
||||
-//third_party/v8:all
|
||||
# V8-backed code-mode tests are covered by Cargo CI. Bazel CI
|
||||
# cross-compiles in several legs, and those tests are not stable in
|
||||
# that setup yet.
|
||||
# Keep V8-backed code-mode tests out of the ordinary macOS/Linux
|
||||
# legs; authenticated Windows-cross shards below exercise the
|
||||
# source-built gnullvm V8 path.
|
||||
-//codex-rs/code-mode:code-mode-unit-tests
|
||||
-//codex-rs/v8-poc:v8-poc-unit-tests
|
||||
)
|
||||
@@ -135,9 +136,9 @@ jobs:
|
||||
key: ${{ steps.prepare_bazel.outputs.repository-cache-key }}
|
||||
|
||||
test-windows-shard:
|
||||
# Split the Windows Bazel test leg across separate Windows
|
||||
# hosts. Each shard still uses Linux RBE for build actions, but the test
|
||||
# execution itself happens on its own Windows runner.
|
||||
# Split the Windows Bazel test leg across separate Windows hosts. Jobs with
|
||||
# BuildBuddy credentials use Linux RBE for build actions; test execution
|
||||
# remains on a Windows runner.
|
||||
timeout-minutes: 30
|
||||
strategy:
|
||||
fail-fast: false
|
||||
@@ -183,9 +184,9 @@ jobs:
|
||||
run: |
|
||||
set -euo pipefail
|
||||
|
||||
bazel_test_query='tests(//...) except tests(//third_party/v8:all) except //codex-rs/code-mode:code-mode-unit-tests except //codex-rs/v8-poc:v8-poc-unit-tests except attr(tags, "manual", tests(//...))'
|
||||
bazel_test_query='tests(//...) except tests(//third_party/v8:all) except attr(tags, "manual", tests(//...))'
|
||||
mapfile -t bazel_targets < <(
|
||||
MSYS2_ARG_CONV_EXCL='*' bazel query --output=label "${bazel_test_query}" \
|
||||
./.github/scripts/run-bazel-query-ci.sh --output=label -- "${bazel_test_query}" \
|
||||
| LC_ALL=C sort
|
||||
)
|
||||
|
||||
@@ -289,9 +290,8 @@ jobs:
|
||||
# path. V8 consumers under `//codex-rs/...` still participate
|
||||
# transitively through `//...`.
|
||||
-//third_party/v8:all
|
||||
# Keep this aligned with the main Bazel job. The native Windows
|
||||
# job preserves broad post-merge coverage, but code-mode/V8 tests
|
||||
# are covered by Cargo CI rather than Bazel for now.
|
||||
# Keep this job broad and cheap; authenticated Windows-cross jobs
|
||||
# add source-built V8-backed code-mode coverage.
|
||||
-//codex-rs/code-mode:code-mode-unit-tests
|
||||
-//codex-rs/v8-poc:v8-poc-unit-tests
|
||||
)
|
||||
|
||||
Generated
+3
@@ -2789,6 +2789,7 @@ dependencies = [
|
||||
"codex-file-system",
|
||||
"codex-protocol",
|
||||
"codex-sandboxing",
|
||||
"codex-shell-command",
|
||||
"codex-test-binary-support",
|
||||
"codex-utils-absolute-path",
|
||||
"codex-utils-pty",
|
||||
@@ -3597,6 +3598,7 @@ dependencies = [
|
||||
"urlencoding",
|
||||
"webbrowser",
|
||||
"which 8.0.0",
|
||||
"wiremock",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
@@ -3691,6 +3693,7 @@ dependencies = [
|
||||
"base64 0.22.1",
|
||||
"codex-protocol",
|
||||
"codex-utils-absolute-path",
|
||||
"libc",
|
||||
"once_cell",
|
||||
"pretty_assertions",
|
||||
"regex",
|
||||
|
||||
@@ -9,7 +9,10 @@ mod websocket;
|
||||
use self::auth::load_remote_control_auth;
|
||||
use self::auth::recover_remote_control_auth;
|
||||
use self::enroll::RemoteControlEnrollment;
|
||||
use self::enroll::enroll_remote_control_server;
|
||||
use self::enroll::load_persisted_remote_control_enrollment;
|
||||
use self::enroll::refresh_remote_control_server;
|
||||
use self::enroll::update_persisted_remote_control_enrollment;
|
||||
use crate::transport::remote_control::websocket::RemoteControlChannels;
|
||||
use crate::transport::remote_control::websocket::RemoteControlStatusPublisher;
|
||||
use crate::transport::remote_control::websocket::RemoteControlWebsocket;
|
||||
@@ -36,9 +39,13 @@ use gethostname::gethostname;
|
||||
use std::error::Error;
|
||||
use std::fmt;
|
||||
use std::io;
|
||||
use std::ops::Deref;
|
||||
use std::ops::DerefMut;
|
||||
use std::panic::AssertUnwindSafe;
|
||||
use std::sync::Arc;
|
||||
use std::sync::Mutex as StdMutex;
|
||||
use tokio::sync::Semaphore;
|
||||
use tokio::sync::SemaphorePermit;
|
||||
use tokio::sync::mpsc;
|
||||
use tokio::sync::oneshot;
|
||||
use tokio::sync::watch;
|
||||
@@ -65,12 +72,81 @@ pub struct RemoteControlHandle {
|
||||
enabled_tx: Arc<watch::Sender<bool>>,
|
||||
status_tx: Arc<watch::Sender<RemoteControlStatusChangedNotification>>,
|
||||
state_db_available: bool,
|
||||
state_db: Option<Arc<StateRuntime>>,
|
||||
remote_control_url: String,
|
||||
current_enrollment: CurrentRemoteControlEnrollment,
|
||||
pairing_persistence_key: RemoteControlPairingPersistenceKey,
|
||||
pairing_persistence_key_required: bool,
|
||||
auth_manager: Arc<AuthManager>,
|
||||
}
|
||||
|
||||
type CurrentRemoteControlEnrollment = Arc<StdMutex<Option<RemoteControlEnrollment>>>;
|
||||
// Pairing and websocket connect share one selected server so they cannot enroll or clear
|
||||
// different persisted rows while either path is awaiting backend I/O.
|
||||
type CurrentRemoteControlEnrollment = Arc<RemoteControlEnrollmentState>;
|
||||
type RemoteControlPairingPersistenceKey = watch::Sender<Option<String>>;
|
||||
|
||||
struct RemoteControlEnrollmentState {
|
||||
enrollment: StdMutex<Option<RemoteControlEnrollment>>,
|
||||
lock: Semaphore,
|
||||
}
|
||||
|
||||
impl RemoteControlEnrollmentState {
|
||||
fn new(enrollment: Option<RemoteControlEnrollment>) -> Self {
|
||||
Self {
|
||||
enrollment: StdMutex::new(enrollment),
|
||||
lock: Semaphore::new(1),
|
||||
}
|
||||
}
|
||||
|
||||
async fn lock(&self) -> RemoteControlEnrollmentLease<'_> {
|
||||
let permit = match self.lock.acquire().await {
|
||||
Ok(permit) => permit,
|
||||
Err(_) => unreachable!("remote control enrollment lock should stay open"),
|
||||
};
|
||||
RemoteControlEnrollmentLease {
|
||||
state: self,
|
||||
enrollment: self.snapshot(),
|
||||
_permit: permit,
|
||||
}
|
||||
}
|
||||
|
||||
fn snapshot(&self) -> Option<RemoteControlEnrollment> {
|
||||
self.enrollment
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner)
|
||||
.clone()
|
||||
}
|
||||
}
|
||||
|
||||
struct RemoteControlEnrollmentLease<'a> {
|
||||
state: &'a RemoteControlEnrollmentState,
|
||||
enrollment: Option<RemoteControlEnrollment>,
|
||||
_permit: SemaphorePermit<'a>,
|
||||
}
|
||||
|
||||
impl Deref for RemoteControlEnrollmentLease<'_> {
|
||||
type Target = Option<RemoteControlEnrollment>;
|
||||
|
||||
fn deref(&self) -> &Self::Target {
|
||||
&self.enrollment
|
||||
}
|
||||
}
|
||||
|
||||
impl DerefMut for RemoteControlEnrollmentLease<'_> {
|
||||
fn deref_mut(&mut self) -> &mut Self::Target {
|
||||
&mut self.enrollment
|
||||
}
|
||||
}
|
||||
|
||||
impl Drop for RemoteControlEnrollmentLease<'_> {
|
||||
fn drop(&mut self) {
|
||||
*self
|
||||
.state
|
||||
.enrollment
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner) = self.enrollment.take();
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub struct RemoteControlUnavailable;
|
||||
@@ -126,8 +202,6 @@ impl RemoteControlHandle {
|
||||
*state = false;
|
||||
changed
|
||||
});
|
||||
clear_current_enrollment(&self.current_enrollment);
|
||||
|
||||
let status = self.status();
|
||||
info!(
|
||||
enabled_changed,
|
||||
@@ -151,28 +225,30 @@ impl RemoteControlHandle {
|
||||
pub async fn start_pairing(
|
||||
&self,
|
||||
params: RemoteControlPairingStartParams,
|
||||
app_server_client_name: Option<&str>,
|
||||
) -> io::Result<RemoteControlPairingStartResponse> {
|
||||
if !*self.enabled_tx.borrow() {
|
||||
return Err(Self::pairing_disabled_error());
|
||||
}
|
||||
let mut auth = load_remote_control_auth(&self.auth_manager)
|
||||
.await
|
||||
.map_err(|_| pairing_unavailable_error())?;
|
||||
let mut enrollment = {
|
||||
let current_enrollment = self
|
||||
.current_enrollment
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner);
|
||||
current_enrollment
|
||||
.as_ref()
|
||||
.filter(|enrollment| enrollment.account_id == auth.account_id)
|
||||
.cloned()
|
||||
}
|
||||
.ok_or_else(pairing_unavailable_error)?;
|
||||
let installation_id = self.status().installation_id;
|
||||
let status = self.status();
|
||||
let installation_id = status.installation_id;
|
||||
let app_server_client_name = self.pairing_persistence_key(app_server_client_name)?;
|
||||
let app_server_client_name = app_server_client_name.as_deref();
|
||||
let mut current_enrollment = self.current_enrollment.lock().await;
|
||||
let mut enrollment = self
|
||||
.load_or_enroll_pairing_server(
|
||||
&mut current_enrollment,
|
||||
&mut auth,
|
||||
&installation_id,
|
||||
&status.server_name,
|
||||
app_server_client_name,
|
||||
)
|
||||
.await?;
|
||||
if enrollment.should_refresh_server_token() {
|
||||
refresh_pairing_enrollment(
|
||||
&self.current_enrollment,
|
||||
&mut current_enrollment,
|
||||
self.state_db.as_deref(),
|
||||
app_server_client_name,
|
||||
&self.auth_manager,
|
||||
&mut auth,
|
||||
&installation_id,
|
||||
@@ -185,9 +261,11 @@ impl RemoteControlHandle {
|
||||
};
|
||||
let pairing_response = match enrollment.start_pairing(pairing_request()).await {
|
||||
Err(err) if err.kind() == io::ErrorKind::PermissionDenied => {
|
||||
clear_pairing_server_token(&self.current_enrollment, &mut enrollment)?;
|
||||
clear_pairing_server_token(&mut current_enrollment, &mut enrollment)?;
|
||||
refresh_pairing_enrollment(
|
||||
&self.current_enrollment,
|
||||
&mut current_enrollment,
|
||||
self.state_db.as_deref(),
|
||||
app_server_client_name,
|
||||
&self.auth_manager,
|
||||
&mut auth,
|
||||
&installation_id,
|
||||
@@ -196,24 +274,46 @@ impl RemoteControlHandle {
|
||||
.await?;
|
||||
enrollment.start_pairing(pairing_request()).await
|
||||
}
|
||||
Err(err) if err.kind() == io::ErrorKind::NotFound => {
|
||||
clear_pairing_enrollment(
|
||||
&mut current_enrollment,
|
||||
self.state_db.as_deref(),
|
||||
app_server_client_name,
|
||||
&enrollment,
|
||||
)
|
||||
.await;
|
||||
enrollment = self
|
||||
.load_or_enroll_pairing_server(
|
||||
&mut current_enrollment,
|
||||
&mut auth,
|
||||
&installation_id,
|
||||
&status.server_name,
|
||||
app_server_client_name,
|
||||
)
|
||||
.await?;
|
||||
enrollment.start_pairing(pairing_request()).await
|
||||
}
|
||||
pairing_response => pairing_response,
|
||||
};
|
||||
if let Err(err) = &pairing_response {
|
||||
match err.kind() {
|
||||
io::ErrorKind::NotFound => {
|
||||
clear_current_enrollment_if_matches(&self.current_enrollment, &enrollment);
|
||||
clear_pairing_enrollment(
|
||||
&mut current_enrollment,
|
||||
self.state_db.as_deref(),
|
||||
app_server_client_name,
|
||||
&enrollment,
|
||||
)
|
||||
.await;
|
||||
return Err(pairing_unavailable_error());
|
||||
}
|
||||
io::ErrorKind::PermissionDenied => {
|
||||
clear_pairing_server_token(&self.current_enrollment, &mut enrollment)?;
|
||||
clear_pairing_server_token(&mut current_enrollment, &mut enrollment)?;
|
||||
return Err(pairing_unavailable_error());
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
if !*self.enabled_tx.borrow() {
|
||||
return Err(Self::pairing_disabled_error());
|
||||
}
|
||||
let current_auth = load_remote_control_auth(&self.auth_manager)
|
||||
.await
|
||||
.map_err(|_| pairing_unavailable_error())?;
|
||||
@@ -223,6 +323,74 @@ impl RemoteControlHandle {
|
||||
pairing_response
|
||||
}
|
||||
|
||||
async fn load_or_enroll_pairing_server(
|
||||
&self,
|
||||
current_enrollment: &mut Option<RemoteControlEnrollment>,
|
||||
auth: &mut auth::RemoteControlConnectionAuth,
|
||||
installation_id: &str,
|
||||
server_name: &str,
|
||||
app_server_client_name: Option<&str>,
|
||||
) -> io::Result<RemoteControlEnrollment> {
|
||||
if let Some(enrollment) = current_enrollment
|
||||
.as_ref()
|
||||
.filter(|enrollment| enrollment.account_id == auth.account_id)
|
||||
.cloned()
|
||||
{
|
||||
return Ok(enrollment);
|
||||
}
|
||||
|
||||
let remote_control_target = normalize_remote_control_url(&self.remote_control_url)?;
|
||||
let state_db = self
|
||||
.state_db
|
||||
.as_deref()
|
||||
.ok_or_else(pairing_unavailable_error)?;
|
||||
if let Some(mut enrollment) = load_persisted_remote_control_enrollment(
|
||||
Some(state_db),
|
||||
&remote_control_target,
|
||||
&auth.account_id,
|
||||
app_server_client_name,
|
||||
)
|
||||
.await?
|
||||
{
|
||||
enrollment.server_name = server_name.to_string();
|
||||
publish_current_enrollment(current_enrollment, &enrollment);
|
||||
return Ok(enrollment);
|
||||
}
|
||||
|
||||
let enrollment = enroll_pairing_server(
|
||||
&self.auth_manager,
|
||||
auth,
|
||||
&remote_control_target,
|
||||
installation_id,
|
||||
server_name,
|
||||
)
|
||||
.await?;
|
||||
update_persisted_remote_control_enrollment(
|
||||
Some(state_db),
|
||||
&remote_control_target,
|
||||
&auth.account_id,
|
||||
app_server_client_name,
|
||||
Some(&enrollment),
|
||||
)
|
||||
.await?;
|
||||
publish_current_enrollment(current_enrollment, &enrollment);
|
||||
Ok(enrollment)
|
||||
}
|
||||
|
||||
fn pairing_persistence_key(
|
||||
&self,
|
||||
app_server_client_name: Option<&str>,
|
||||
) -> io::Result<Option<String>> {
|
||||
if self.pairing_persistence_key_required && self.pairing_persistence_key.borrow().is_none()
|
||||
{
|
||||
let app_server_client_name =
|
||||
app_server_client_name.ok_or_else(pairing_unavailable_error)?;
|
||||
self.pairing_persistence_key
|
||||
.send_replace(Some(app_server_client_name.to_string()));
|
||||
}
|
||||
Ok(self.pairing_persistence_key.borrow().clone())
|
||||
}
|
||||
|
||||
pub async fn list_clients(
|
||||
&self,
|
||||
params: RemoteControlClientsListParams,
|
||||
@@ -239,13 +407,6 @@ impl RemoteControlHandle {
|
||||
.await
|
||||
}
|
||||
|
||||
fn pairing_disabled_error() -> io::Error {
|
||||
io::Error::new(
|
||||
io::ErrorKind::InvalidInput,
|
||||
"remote control pairing requires remote control to be enabled",
|
||||
)
|
||||
}
|
||||
|
||||
fn publish_status(
|
||||
&self,
|
||||
connection_status: RemoteControlConnectionStatus,
|
||||
@@ -277,8 +438,36 @@ impl RemoteControlHandle {
|
||||
}
|
||||
}
|
||||
|
||||
async fn enroll_pairing_server(
|
||||
auth_manager: &Arc<AuthManager>,
|
||||
auth: &mut auth::RemoteControlConnectionAuth,
|
||||
remote_control_target: &protocol::RemoteControlTarget,
|
||||
installation_id: &str,
|
||||
server_name: &str,
|
||||
) -> io::Result<RemoteControlEnrollment> {
|
||||
match enroll_remote_control_server(remote_control_target, auth, installation_id, server_name)
|
||||
.await
|
||||
{
|
||||
Ok(enrollment) => return Ok(enrollment),
|
||||
Err(err) if err.kind() == io::ErrorKind::PermissionDenied => {
|
||||
let mut auth_recovery = auth_manager.unauthorized_recovery();
|
||||
let mut auth_change_rx = auth_manager.auth_change_receiver();
|
||||
if !recover_remote_control_auth(&mut auth_recovery, &mut auth_change_rx).await {
|
||||
return Err(err);
|
||||
}
|
||||
*auth = load_remote_control_auth(auth_manager)
|
||||
.await
|
||||
.map_err(|_| pairing_unavailable_error())?;
|
||||
}
|
||||
Err(err) => return Err(err),
|
||||
}
|
||||
enroll_remote_control_server(remote_control_target, auth, installation_id, server_name).await
|
||||
}
|
||||
|
||||
async fn refresh_pairing_enrollment(
|
||||
current_enrollment: &CurrentRemoteControlEnrollment,
|
||||
current_enrollment: &mut Option<RemoteControlEnrollment>,
|
||||
state_db: Option<&StateRuntime>,
|
||||
app_server_client_name: Option<&str>,
|
||||
auth_manager: &Arc<AuthManager>,
|
||||
auth: &mut auth::RemoteControlConnectionAuth,
|
||||
installation_id: &str,
|
||||
@@ -286,7 +475,14 @@ async fn refresh_pairing_enrollment(
|
||||
) -> io::Result<()> {
|
||||
if let Err(err) = refresh_remote_control_server(auth, installation_id, enrollment).await {
|
||||
if err.kind() != io::ErrorKind::PermissionDenied {
|
||||
return handle_pairing_refresh_error(current_enrollment, enrollment, err);
|
||||
return handle_pairing_refresh_error(
|
||||
current_enrollment,
|
||||
state_db,
|
||||
app_server_client_name,
|
||||
enrollment,
|
||||
err,
|
||||
)
|
||||
.await;
|
||||
}
|
||||
let mut auth_recovery = auth_manager.unauthorized_recovery();
|
||||
let mut auth_change_rx = auth_manager.auth_change_receiver();
|
||||
@@ -300,7 +496,14 @@ async fn refresh_pairing_enrollment(
|
||||
return Err(pairing_unavailable_error());
|
||||
}
|
||||
if let Err(err) = refresh_remote_control_server(auth, installation_id, enrollment).await {
|
||||
return handle_pairing_refresh_error(current_enrollment, enrollment, err);
|
||||
return handle_pairing_refresh_error(
|
||||
current_enrollment,
|
||||
state_db,
|
||||
app_server_client_name,
|
||||
enrollment,
|
||||
err,
|
||||
)
|
||||
.await;
|
||||
}
|
||||
}
|
||||
if replace_current_enrollment(current_enrollment, enrollment) {
|
||||
@@ -310,21 +513,54 @@ async fn refresh_pairing_enrollment(
|
||||
}
|
||||
}
|
||||
|
||||
fn handle_pairing_refresh_error(
|
||||
current_enrollment: &CurrentRemoteControlEnrollment,
|
||||
async fn handle_pairing_refresh_error(
|
||||
current_enrollment: &mut Option<RemoteControlEnrollment>,
|
||||
state_db: Option<&StateRuntime>,
|
||||
app_server_client_name: Option<&str>,
|
||||
enrollment: &RemoteControlEnrollment,
|
||||
err: io::Error,
|
||||
) -> io::Result<()> {
|
||||
if err.kind() == io::ErrorKind::NotFound {
|
||||
clear_current_enrollment_if_matches(current_enrollment, enrollment);
|
||||
clear_pairing_enrollment(
|
||||
current_enrollment,
|
||||
state_db,
|
||||
app_server_client_name,
|
||||
enrollment,
|
||||
)
|
||||
.await;
|
||||
Err(pairing_unavailable_error())
|
||||
} else {
|
||||
Err(err)
|
||||
}
|
||||
}
|
||||
|
||||
async fn clear_pairing_enrollment(
|
||||
current_enrollment: &mut Option<RemoteControlEnrollment>,
|
||||
state_db: Option<&StateRuntime>,
|
||||
app_server_client_name: Option<&str>,
|
||||
enrollment: &RemoteControlEnrollment,
|
||||
) {
|
||||
if !clear_current_enrollment_if_matches(current_enrollment, enrollment) {
|
||||
return;
|
||||
}
|
||||
let Some(state_db) = state_db else {
|
||||
return;
|
||||
};
|
||||
if let Err(err) = update_persisted_remote_control_enrollment(
|
||||
Some(state_db),
|
||||
&enrollment.remote_control_target,
|
||||
&enrollment.account_id,
|
||||
app_server_client_name,
|
||||
/*enrollment*/ None,
|
||||
)
|
||||
.await
|
||||
{
|
||||
warn!("failed to clear stale pairing enrollment: {err}");
|
||||
}
|
||||
}
|
||||
|
||||
fn clear_pairing_server_token(
|
||||
current_enrollment: &CurrentRemoteControlEnrollment,
|
||||
current_enrollment: &mut Option<RemoteControlEnrollment>,
|
||||
enrollment: &mut RemoteControlEnrollment,
|
||||
) -> io::Result<()> {
|
||||
enrollment.clear_server_token();
|
||||
@@ -359,27 +595,16 @@ fn remote_control_status_with_connection_status(
|
||||
}
|
||||
|
||||
fn publish_current_enrollment(
|
||||
current_enrollment: &CurrentRemoteControlEnrollment,
|
||||
current_enrollment: &mut Option<RemoteControlEnrollment>,
|
||||
enrollment: &RemoteControlEnrollment,
|
||||
) {
|
||||
*current_enrollment
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner) = Some(enrollment.clone());
|
||||
}
|
||||
|
||||
fn clear_current_enrollment(current_enrollment: &CurrentRemoteControlEnrollment) {
|
||||
*current_enrollment
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner) = None;
|
||||
*current_enrollment = Some(enrollment.clone());
|
||||
}
|
||||
|
||||
fn replace_current_enrollment(
|
||||
current_enrollment: &CurrentRemoteControlEnrollment,
|
||||
current_enrollment: &mut Option<RemoteControlEnrollment>,
|
||||
enrollment: &RemoteControlEnrollment,
|
||||
) -> bool {
|
||||
let mut current_enrollment = current_enrollment
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner);
|
||||
if !current_enrollment
|
||||
.as_ref()
|
||||
.is_some_and(|current| same_remote_control_enrollment(current, enrollment))
|
||||
@@ -391,17 +616,17 @@ fn replace_current_enrollment(
|
||||
}
|
||||
|
||||
fn clear_current_enrollment_if_matches(
|
||||
current_enrollment: &CurrentRemoteControlEnrollment,
|
||||
current_enrollment: &mut Option<RemoteControlEnrollment>,
|
||||
enrollment: &RemoteControlEnrollment,
|
||||
) {
|
||||
let mut current_enrollment = current_enrollment
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner);
|
||||
) -> bool {
|
||||
if current_enrollment
|
||||
.as_ref()
|
||||
.is_some_and(|current| same_remote_control_enrollment(current, enrollment))
|
||||
{
|
||||
*current_enrollment = None;
|
||||
true
|
||||
} else {
|
||||
false
|
||||
}
|
||||
}
|
||||
|
||||
@@ -438,9 +663,13 @@ pub async fn start_remote_control(
|
||||
};
|
||||
|
||||
let (enabled_tx, enabled_rx) = watch::channel(initial_enabled);
|
||||
let current_enrollment = Arc::new(StdMutex::new(None));
|
||||
let current_enrollment = Arc::new(RemoteControlEnrollmentState::new(/*enrollment*/ None));
|
||||
let websocket_current_enrollment = current_enrollment.clone();
|
||||
let pairing_persistence_key_required = app_server_client_name_rx.is_some();
|
||||
let (pairing_persistence_key, _pairing_persistence_key_rx) = watch::channel(None);
|
||||
let websocket_pairing_persistence_key = pairing_persistence_key.clone();
|
||||
let handle_auth_manager = auth_manager.clone();
|
||||
let handle_state_db = state_db.clone();
|
||||
let server_name = gethostname().to_string_lossy().trim().to_string();
|
||||
let remote_control_url = config.remote_control_url;
|
||||
let installation_id = config.installation_id;
|
||||
@@ -490,6 +719,7 @@ pub async fn start_remote_control(
|
||||
transport_event_tx,
|
||||
status_publisher,
|
||||
current_enrollment: websocket_current_enrollment,
|
||||
pairing_persistence_key: websocket_pairing_persistence_key,
|
||||
},
|
||||
shutdown_token,
|
||||
enabled_rx,
|
||||
@@ -534,8 +764,11 @@ pub async fn start_remote_control(
|
||||
enabled_tx: Arc::new(enabled_tx),
|
||||
status_tx: Arc::new(status_tx),
|
||||
state_db_available,
|
||||
state_db: handle_state_db,
|
||||
remote_control_url: handle_remote_control_url,
|
||||
current_enrollment,
|
||||
pairing_persistence_key,
|
||||
pairing_persistence_key_required,
|
||||
auth_manager: handle_auth_manager,
|
||||
},
|
||||
))
|
||||
|
||||
@@ -40,7 +40,6 @@ use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
use std::collections::BTreeMap;
|
||||
use std::sync::Arc;
|
||||
use std::sync::Mutex as StdMutex;
|
||||
use tempfile::TempDir;
|
||||
use time::OffsetDateTime;
|
||||
use tokio::io::AsyncBufReadExt;
|
||||
@@ -148,24 +147,29 @@ fn remote_control_handle_with_current_enrollment(
|
||||
});
|
||||
let remote_control_target = normalize_remote_control_url(remote_control_url)
|
||||
.expect("remote control target should normalize");
|
||||
let current_enrollment = Arc::new(StdMutex::new(Some(RemoteControlEnrollment {
|
||||
remote_control_target,
|
||||
account_id: "account_id".to_string(),
|
||||
environment_id: "env_test".to_string(),
|
||||
server_id: "srv_e_test".to_string(),
|
||||
server_name: test_server_name(),
|
||||
remote_control_token: Some(TEST_REMOTE_CONTROL_SERVER_TOKEN.to_string()),
|
||||
expires_at: Some(
|
||||
OffsetDateTime::from_unix_timestamp(33_336_362_096)
|
||||
.expect("future timestamp should parse"),
|
||||
),
|
||||
})));
|
||||
let current_enrollment = Arc::new(RemoteControlEnrollmentState::new(Some(
|
||||
RemoteControlEnrollment {
|
||||
remote_control_target,
|
||||
account_id: "account_id".to_string(),
|
||||
environment_id: "env_test".to_string(),
|
||||
server_id: "srv_e_test".to_string(),
|
||||
server_name: test_server_name(),
|
||||
remote_control_token: Some(TEST_REMOTE_CONTROL_SERVER_TOKEN.to_string()),
|
||||
expires_at: Some(
|
||||
OffsetDateTime::from_unix_timestamp(33_336_362_096)
|
||||
.expect("future timestamp should parse"),
|
||||
),
|
||||
},
|
||||
)));
|
||||
RemoteControlHandle {
|
||||
enabled_tx: Arc::new(enabled_tx),
|
||||
status_tx: Arc::new(status_tx),
|
||||
state_db_available: true,
|
||||
state_db: None,
|
||||
remote_control_url: remote_control_url.to_string(),
|
||||
current_enrollment,
|
||||
pairing_persistence_key: watch::channel(None).0,
|
||||
pairing_persistence_key_required: false,
|
||||
auth_manager,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -24,8 +24,11 @@ fn client_management_handle(
|
||||
enabled_tx: Arc::new(enabled_tx),
|
||||
status_tx: Arc::new(status_tx),
|
||||
state_db_available: false,
|
||||
state_db: None,
|
||||
remote_control_url,
|
||||
current_enrollment: Arc::new(StdMutex::new(None)),
|
||||
current_enrollment: Arc::new(RemoteControlEnrollmentState::new(/*enrollment*/ None)),
|
||||
pairing_persistence_key: watch::channel(None).0,
|
||||
pairing_persistence_key_required: false,
|
||||
auth_manager,
|
||||
}
|
||||
}
|
||||
|
||||
+142
-14
@@ -131,13 +131,16 @@ async fn remote_control_handle_starts_pairing_before_websocket_connects() {
|
||||
remote_handle
|
||||
.current_enrollment
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner)
|
||||
.await
|
||||
.as_mut()
|
||||
.expect("current enrollment should exist")
|
||||
.expires_at = Some(OffsetDateTime::now_utc() + time::Duration::seconds(29));
|
||||
|
||||
let response = remote_handle
|
||||
.start_pairing(RemoteControlPairingStartParams { manual_code: true })
|
||||
.start_pairing(
|
||||
RemoteControlPairingStartParams { manual_code: true },
|
||||
/*app_server_client_name*/ None,
|
||||
)
|
||||
.await
|
||||
.expect("pairing should use the current server before websocket connect");
|
||||
server_task.await.expect("server task should finish");
|
||||
@@ -219,7 +222,10 @@ async fn remote_control_handle_refreshes_after_pairing_auth_failure() {
|
||||
);
|
||||
|
||||
let response = remote_handle
|
||||
.start_pairing(RemoteControlPairingStartParams::default())
|
||||
.start_pairing(
|
||||
RemoteControlPairingStartParams::default(),
|
||||
/*app_server_client_name*/ None,
|
||||
)
|
||||
.await
|
||||
.expect("pairing should refresh after server token auth failure");
|
||||
server_task.await.expect("server task should finish");
|
||||
@@ -332,13 +338,16 @@ async fn remote_control_handle_recovers_auth_before_refreshing_pairing() {
|
||||
remote_handle
|
||||
.current_enrollment
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner)
|
||||
.await
|
||||
.as_mut()
|
||||
.expect("current enrollment should exist")
|
||||
.expires_at = Some(OffsetDateTime::now_utc() + time::Duration::seconds(29));
|
||||
|
||||
let response = remote_handle
|
||||
.start_pairing(RemoteControlPairingStartParams::default())
|
||||
.start_pairing(
|
||||
RemoteControlPairingStartParams::default(),
|
||||
/*app_server_client_name*/ None,
|
||||
)
|
||||
.await
|
||||
.expect("pairing should refresh after auth recovery");
|
||||
server_task.await.expect("server task should finish");
|
||||
@@ -414,21 +423,137 @@ async fn start_remote_control_pairing_preserves_expiry_parse_error_context() {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn remote_control_handle_disable_clears_current_enrollment() {
|
||||
async fn remote_control_handle_disable_keeps_current_enrollment() {
|
||||
let remote_handle = remote_control_handle_with_current_enrollment(
|
||||
TEST_REMOTE_CONTROL_URL,
|
||||
remote_control_auth_manager(),
|
||||
);
|
||||
|
||||
remote_handle.disable();
|
||||
remote_handle.enable().expect("enable should succeed");
|
||||
assert!(
|
||||
remote_handle.current_enrollment.lock().await.is_some(),
|
||||
"disabled remote control should keep the selected pairing server"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn remote_control_handle_reenrolls_after_stale_pairing_enrollment() {
|
||||
let listener = TcpListener::bind("127.0.0.1:0")
|
||||
.await
|
||||
.expect("listener should bind");
|
||||
let remote_control_url = remote_control_url_for_listener(&listener);
|
||||
let codex_home = TempDir::new().expect("temp dir should create");
|
||||
let state_db = remote_control_state_runtime(&codex_home).await;
|
||||
let mut remote_handle = remote_control_handle_with_current_enrollment(
|
||||
&remote_control_url,
|
||||
remote_control_auth_manager_with_home(&codex_home),
|
||||
);
|
||||
remote_handle.state_db = Some(state_db.clone());
|
||||
remote_handle.disable();
|
||||
let stale_enrollment = remote_handle
|
||||
.current_enrollment
|
||||
.lock()
|
||||
.await
|
||||
.clone()
|
||||
.expect("current enrollment should exist");
|
||||
let remote_control_target = stale_enrollment.remote_control_target.clone();
|
||||
let refreshed_enrollment = RemoteControlEnrollment {
|
||||
remote_control_target: remote_control_target.clone(),
|
||||
account_id: "account_id".to_string(),
|
||||
environment_id: "env_refreshed".to_string(),
|
||||
server_id: "srv_e_refreshed".to_string(),
|
||||
server_name: test_server_name(),
|
||||
remote_control_token: None,
|
||||
expires_at: None,
|
||||
};
|
||||
update_persisted_remote_control_enrollment(
|
||||
Some(state_db.as_ref()),
|
||||
&remote_control_target,
|
||||
"account_id",
|
||||
/*app_server_client_name*/ None,
|
||||
Some(&stale_enrollment),
|
||||
)
|
||||
.await
|
||||
.expect("stale enrollment should save");
|
||||
let server_refreshed_enrollment = refreshed_enrollment.clone();
|
||||
let server_task = tokio::spawn(async move {
|
||||
let stale_pairing_request = accept_http_request(&listener).await;
|
||||
assert_eq!(
|
||||
stale_pairing_request.request_line,
|
||||
"POST /backend-api/wham/remote/control/server/pair HTTP/1.1"
|
||||
);
|
||||
assert_eq!(
|
||||
stale_pairing_request.headers.get("authorization"),
|
||||
Some(&format!("Bearer {TEST_REMOTE_CONTROL_SERVER_TOKEN}"))
|
||||
);
|
||||
respond_with_status(stale_pairing_request.stream, "404 Not Found", "").await;
|
||||
|
||||
let enroll_request = accept_http_request(&listener).await;
|
||||
assert_eq!(
|
||||
enroll_request.request_line,
|
||||
"POST /backend-api/wham/remote/control/server/enroll HTTP/1.1"
|
||||
);
|
||||
respond_with_json(
|
||||
enroll_request.stream,
|
||||
remote_control_server_token_response(
|
||||
&server_refreshed_enrollment.server_id,
|
||||
&server_refreshed_enrollment.environment_id,
|
||||
TEST_REFRESHED_REMOTE_CONTROL_SERVER_TOKEN,
|
||||
),
|
||||
)
|
||||
.await;
|
||||
|
||||
let refreshed_pairing_request = accept_http_request(&listener).await;
|
||||
assert_eq!(
|
||||
refreshed_pairing_request.request_line,
|
||||
"POST /backend-api/wham/remote/control/server/pair HTTP/1.1"
|
||||
);
|
||||
assert_eq!(
|
||||
refreshed_pairing_request.headers.get("authorization"),
|
||||
Some(&format!(
|
||||
"Bearer {TEST_REFRESHED_REMOTE_CONTROL_SERVER_TOKEN}"
|
||||
))
|
||||
);
|
||||
respond_with_json(
|
||||
refreshed_pairing_request.stream,
|
||||
json!({
|
||||
"pairing_code": "pairing-code",
|
||||
"manual_pairing_code": "ABCD-EFGH",
|
||||
"server_id": server_refreshed_enrollment.server_id,
|
||||
"environment_id": server_refreshed_enrollment.environment_id,
|
||||
"expires_at": "3026-05-22T12:34:56Z",
|
||||
}),
|
||||
)
|
||||
.await;
|
||||
});
|
||||
let response = remote_handle
|
||||
.start_pairing(
|
||||
RemoteControlPairingStartParams::default(),
|
||||
/*app_server_client_name*/ None,
|
||||
)
|
||||
.await
|
||||
.expect("pairing should re-enroll after stale enrollment");
|
||||
server_task.await.expect("server task should finish");
|
||||
|
||||
assert_eq!(
|
||||
remote_handle
|
||||
.start_pairing(RemoteControlPairingStartParams::default())
|
||||
.await
|
||||
.expect_err("re-enabled remote control should wait for a current server")
|
||||
.to_string(),
|
||||
"remote control pairing is unavailable until enrollment completes"
|
||||
response,
|
||||
RemoteControlPairingStartResponse {
|
||||
pairing_code: "pairing-code".to_string(),
|
||||
manual_pairing_code: Some("ABCD-EFGH".to_string()),
|
||||
environment_id: "env_refreshed".to_string(),
|
||||
expires_at: 33_336_362_096,
|
||||
}
|
||||
);
|
||||
assert_eq!(
|
||||
load_persisted_remote_control_enrollment(
|
||||
Some(state_db.as_ref()),
|
||||
&remote_control_target,
|
||||
"account_id",
|
||||
/*app_server_client_name*/ None,
|
||||
)
|
||||
.await
|
||||
.expect("refreshed enrollment should load"),
|
||||
Some(refreshed_enrollment)
|
||||
);
|
||||
}
|
||||
|
||||
@@ -458,7 +583,10 @@ async fn remote_control_handle_discards_pairing_response_after_auth_change() {
|
||||
let remote_handle = remote_handle.clone();
|
||||
async move {
|
||||
remote_handle
|
||||
.start_pairing(RemoteControlPairingStartParams::default())
|
||||
.start_pairing(
|
||||
RemoteControlPairingStartParams::default(),
|
||||
/*app_server_client_name*/ None,
|
||||
)
|
||||
.await
|
||||
}
|
||||
});
|
||||
|
||||
@@ -1,13 +1,13 @@
|
||||
use super::CurrentRemoteControlEnrollment;
|
||||
use super::clear_current_enrollment;
|
||||
use super::RemoteControlPairingPersistenceKey;
|
||||
use super::protocol::ClientEnvelope;
|
||||
use super::protocol::ClientEvent;
|
||||
use super::protocol::ClientId;
|
||||
use super::protocol::RemoteControlTarget;
|
||||
use super::protocol::ServerEnvelope;
|
||||
use super::protocol::StreamId;
|
||||
use super::publish_current_enrollment;
|
||||
use super::remote_control_status_with_connection_status;
|
||||
use super::same_remote_control_enrollment;
|
||||
use super::segment::ClientSegmentObservation;
|
||||
use super::segment::ClientSegmentReassembler;
|
||||
use super::segment::REMOTE_CONTROL_SEGMENT_MAX_BYTES;
|
||||
@@ -55,6 +55,9 @@ use tokio_tungstenite::connect_async;
|
||||
use tokio_tungstenite::tungstenite;
|
||||
use tokio_tungstenite::tungstenite::client::IntoClientRequest;
|
||||
use tokio_util::sync::CancellationToken;
|
||||
|
||||
#[cfg(test)]
|
||||
use super::RemoteControlEnrollmentState;
|
||||
use tracing::error;
|
||||
use tracing::info;
|
||||
use tracing::warn;
|
||||
@@ -252,10 +255,10 @@ pub(crate) struct RemoteControlWebsocket {
|
||||
status_publisher: RemoteControlStatusPublisher,
|
||||
shutdown_token: CancellationToken,
|
||||
reconnect_attempt: u64,
|
||||
enrollment: Option<RemoteControlEnrollment>,
|
||||
auth_recovery: UnauthorizedRecovery,
|
||||
auth_change_rx: watch::Receiver<u64>,
|
||||
current_enrollment: CurrentRemoteControlEnrollment,
|
||||
pairing_persistence_key: RemoteControlPairingPersistenceKey,
|
||||
client_tracker: Arc<Mutex<ClientTracker>>,
|
||||
state: Arc<Mutex<WebsocketState>>,
|
||||
server_event_rx: Arc<Mutex<mpsc::Receiver<super::QueuedServerEnvelope>>>,
|
||||
@@ -294,6 +297,7 @@ pub(super) struct RemoteControlChannels {
|
||||
pub(super) transport_event_tx: mpsc::Sender<TransportEvent>,
|
||||
pub(super) status_publisher: RemoteControlStatusPublisher,
|
||||
pub(super) current_enrollment: CurrentRemoteControlEnrollment,
|
||||
pub(super) pairing_persistence_key: RemoteControlPairingPersistenceKey,
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
@@ -407,10 +411,10 @@ impl RemoteControlWebsocket {
|
||||
status_publisher: channels.status_publisher,
|
||||
shutdown_token,
|
||||
reconnect_attempt: 0,
|
||||
enrollment: None,
|
||||
auth_recovery,
|
||||
auth_change_rx,
|
||||
current_enrollment: channels.current_enrollment,
|
||||
pairing_persistence_key: channels.pairing_persistence_key,
|
||||
client_tracker: Arc::new(Mutex::new(client_tracker)),
|
||||
state: Arc::new(Mutex::new(WebsocketState {
|
||||
outbound_buffer,
|
||||
@@ -457,6 +461,8 @@ impl RemoteControlWebsocket {
|
||||
return;
|
||||
}
|
||||
};
|
||||
self.pairing_persistence_key
|
||||
.send_replace(app_server_client_name.clone());
|
||||
|
||||
loop {
|
||||
if !self.wait_until_enabled().await {
|
||||
@@ -579,15 +585,15 @@ impl RemoteControlWebsocket {
|
||||
|
||||
loop {
|
||||
let subscribe_cursor = self.state.lock().await.subscribe_cursor.clone();
|
||||
let enrollment = self.enrollment.as_ref();
|
||||
let enrollment = self.current_enrollment.snapshot();
|
||||
info!(
|
||||
websocket_url = %remote_control_target.websocket_url,
|
||||
installation_id = %self.installation_id,
|
||||
server_name = %self.server_name,
|
||||
reconnect_attempt = self.reconnect_attempt.saturating_add(1),
|
||||
has_enrollment = enrollment.is_some(),
|
||||
server_id = ?enrollment.map(|enrollment| enrollment.server_id.as_str()),
|
||||
environment_id = ?enrollment.map(|enrollment| enrollment.environment_id.as_str()),
|
||||
server_id = ?enrollment.as_ref().map(|enrollment| enrollment.server_id.as_str()),
|
||||
environment_id = ?enrollment.as_ref().map(|enrollment| enrollment.environment_id.as_str()),
|
||||
subscribe_cursor_present = subscribe_cursor.is_some(),
|
||||
app_server_client_name = ?app_server_client_name,
|
||||
"connecting to app-server remote control websocket"
|
||||
@@ -611,15 +617,17 @@ impl RemoteControlWebsocket {
|
||||
}
|
||||
return ConnectOutcome::Disabled;
|
||||
}
|
||||
connect_result = connect_remote_control_websocket(
|
||||
&remote_control_target,
|
||||
self.state_db.as_deref(),
|
||||
auth_context,
|
||||
&mut self.enrollment,
|
||||
connect_options,
|
||||
&self.status_publisher,
|
||||
&self.current_enrollment,
|
||||
) => connect_result,
|
||||
connect_result = async {
|
||||
connect_remote_control_websocket(
|
||||
&remote_control_target,
|
||||
self.state_db.as_deref(),
|
||||
auth_context,
|
||||
&self.current_enrollment,
|
||||
connect_options,
|
||||
&self.status_publisher,
|
||||
)
|
||||
.await
|
||||
} => connect_result,
|
||||
};
|
||||
|
||||
match connect_result {
|
||||
@@ -631,13 +639,13 @@ impl RemoteControlWebsocket {
|
||||
self.auth_recovery = self.auth_manager.unauthorized_recovery();
|
||||
self.status_publisher
|
||||
.publish_status(RemoteControlConnectionStatus::Connected);
|
||||
let enrollment = self.enrollment.as_ref();
|
||||
let enrollment = self.current_enrollment.snapshot();
|
||||
info!(
|
||||
websocket_url = %remote_control_target.websocket_url,
|
||||
installation_id = %self.installation_id,
|
||||
server_name = %self.server_name,
|
||||
server_id = ?enrollment.map(|enrollment| enrollment.server_id.as_str()),
|
||||
environment_id = ?enrollment.map(|enrollment| enrollment.environment_id.as_str()),
|
||||
server_id = ?enrollment.as_ref().map(|enrollment| enrollment.server_id.as_str()),
|
||||
environment_id = ?enrollment.as_ref().map(|enrollment| enrollment.environment_id.as_str()),
|
||||
subscribe_cursor_present = subscribe_cursor.is_some(),
|
||||
response_headers = %format_headers(response.headers()),
|
||||
"connected to app-server remote control websocket"
|
||||
@@ -656,7 +664,7 @@ impl RemoteControlWebsocket {
|
||||
let reconnect_attempt = self.reconnect_attempt.saturating_add(1);
|
||||
let (reconnect_delay, reconnect_backoff_reset) =
|
||||
next_reconnect_delay(&mut self.reconnect_attempt);
|
||||
let enrollment = self.enrollment.as_ref();
|
||||
let enrollment = self.current_enrollment.snapshot();
|
||||
warn!(
|
||||
websocket_url = %remote_control_target.websocket_url,
|
||||
installation_id = %self.installation_id,
|
||||
@@ -667,8 +675,8 @@ impl RemoteControlWebsocket {
|
||||
reconnect_delay = ?reconnect_delay,
|
||||
reconnect_backoff_reset,
|
||||
has_enrollment = enrollment.is_some(),
|
||||
server_id = ?enrollment.map(|enrollment| enrollment.server_id.as_str()),
|
||||
environment_id = ?enrollment.map(|enrollment| enrollment.environment_id.as_str()),
|
||||
server_id = ?enrollment.as_ref().map(|enrollment| enrollment.server_id.as_str()),
|
||||
environment_id = ?enrollment.as_ref().map(|enrollment| enrollment.environment_id.as_str()),
|
||||
subscribe_cursor_present = subscribe_cursor.is_some(),
|
||||
"failed to connect to app-server remote control websocket"
|
||||
);
|
||||
@@ -1189,19 +1197,108 @@ pub(super) async fn connect_remote_control_websocket(
|
||||
remote_control_target: &RemoteControlTarget,
|
||||
state_db: Option<&StateRuntime>,
|
||||
mut auth_context: RemoteControlAuthContext<'_>,
|
||||
enrollment: &mut Option<RemoteControlEnrollment>,
|
||||
current_enrollment: &CurrentRemoteControlEnrollment,
|
||||
connect_options: RemoteControlConnectOptions<'_>,
|
||||
status_publisher: &RemoteControlStatusPublisher,
|
||||
current_enrollment: &CurrentRemoteControlEnrollment,
|
||||
) -> io::Result<(
|
||||
WebSocketStream<MaybeTlsStream<TcpStream>>,
|
||||
tungstenite::http::Response<()>,
|
||||
)> {
|
||||
ensure_rustls_crypto_provider();
|
||||
|
||||
let (auth, enrollment) = {
|
||||
let mut current_enrollment = current_enrollment.lock().await;
|
||||
let auth = prepare_remote_control_enrollment(
|
||||
remote_control_target,
|
||||
state_db,
|
||||
&mut auth_context,
|
||||
&mut current_enrollment,
|
||||
connect_options,
|
||||
status_publisher,
|
||||
)
|
||||
.await?;
|
||||
let enrollment = current_enrollment.as_ref().cloned().ok_or_else(|| {
|
||||
io::Error::other("missing remote control enrollment after enrollment step")
|
||||
})?;
|
||||
(auth, enrollment)
|
||||
};
|
||||
let request = build_remote_control_websocket_request(
|
||||
&remote_control_target.websocket_url,
|
||||
&enrollment,
|
||||
connect_options.installation_id,
|
||||
connect_options.subscribe_cursor,
|
||||
)?;
|
||||
|
||||
let websocket_connect_result = tokio::time::timeout(
|
||||
REMOTE_CONTROL_WEBSOCKET_CONNECT_TIMEOUT,
|
||||
connect_async(request),
|
||||
)
|
||||
.await
|
||||
.map_err(|_| {
|
||||
io::Error::new(
|
||||
ErrorKind::TimedOut,
|
||||
format!(
|
||||
"timed out connecting to remote control websocket at `{}` after {:?}",
|
||||
remote_control_target.websocket_url, REMOTE_CONTROL_WEBSOCKET_CONNECT_TIMEOUT
|
||||
),
|
||||
)
|
||||
})?;
|
||||
|
||||
match websocket_connect_result {
|
||||
Ok((websocket_stream, response)) => Ok((websocket_stream, response.map(|_| ()))),
|
||||
Err(err) => {
|
||||
match &err {
|
||||
tungstenite::Error::Http(response) if response.status().as_u16() == 404 => {
|
||||
info!(
|
||||
"remote control websocket returned HTTP 404; clearing stale enrollment before re-enrolling: websocket_url={}, account_id={}, server_id={}, environment_id={}",
|
||||
remote_control_target.websocket_url,
|
||||
auth.account_id,
|
||||
enrollment.server_id,
|
||||
enrollment.environment_id
|
||||
);
|
||||
clear_remote_control_enrollment_if_matches(
|
||||
state_db,
|
||||
remote_control_target,
|
||||
&auth.account_id,
|
||||
connect_options.app_server_client_name,
|
||||
current_enrollment,
|
||||
&enrollment,
|
||||
status_publisher,
|
||||
)
|
||||
.await;
|
||||
}
|
||||
tungstenite::Error::Http(response)
|
||||
if matches!(response.status().as_u16(), 401 | 403) =>
|
||||
{
|
||||
clear_remote_control_server_token_if_matches(current_enrollment, &enrollment)
|
||||
.await?;
|
||||
return Err(io::Error::other(format!(
|
||||
"remote control websocket auth failed with HTTP {}; refreshing server token before reconnect",
|
||||
response.status()
|
||||
)));
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
Err(io::Error::other(
|
||||
format_remote_control_websocket_connect_error(
|
||||
&remote_control_target.websocket_url,
|
||||
&err,
|
||||
),
|
||||
))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
async fn prepare_remote_control_enrollment(
|
||||
remote_control_target: &RemoteControlTarget,
|
||||
state_db: Option<&StateRuntime>,
|
||||
auth_context: &mut RemoteControlAuthContext<'_>,
|
||||
enrollment: &mut Option<RemoteControlEnrollment>,
|
||||
connect_options: RemoteControlConnectOptions<'_>,
|
||||
status_publisher: &RemoteControlStatusPublisher,
|
||||
) -> io::Result<RemoteControlConnectionAuth> {
|
||||
let Some(state_db) = state_db else {
|
||||
*enrollment = None;
|
||||
clear_current_enrollment(current_enrollment);
|
||||
return Err(io::Error::new(
|
||||
ErrorKind::NotFound,
|
||||
"remote control requires sqlite state db",
|
||||
@@ -1214,7 +1311,6 @@ pub(super) async fn connect_remote_control_websocket(
|
||||
if err.kind() == ErrorKind::PermissionDenied {
|
||||
*enrollment = None;
|
||||
status_publisher.publish_environment_id(/*environment_id*/ None);
|
||||
clear_current_enrollment(current_enrollment);
|
||||
}
|
||||
return Err(err);
|
||||
}
|
||||
@@ -1231,7 +1327,6 @@ pub(super) async fn connect_remote_control_websocket(
|
||||
);
|
||||
*enrollment = None;
|
||||
status_publisher.publish_environment_id(/*environment_id*/ None);
|
||||
clear_current_enrollment(current_enrollment);
|
||||
}
|
||||
if let Some(enrollment) = enrollment.as_mut() {
|
||||
enrollment.remote_control_target = remote_control_target.clone();
|
||||
@@ -1262,7 +1357,7 @@ pub(super) async fn connect_remote_control_websocket(
|
||||
remote_control_target,
|
||||
state_db,
|
||||
&auth,
|
||||
&mut auth_context,
|
||||
auth_context,
|
||||
enrollment,
|
||||
connect_options,
|
||||
status_publisher,
|
||||
@@ -1307,14 +1402,13 @@ pub(super) async fn connect_remote_control_websocket(
|
||||
connect_options.app_server_client_name,
|
||||
enrollment,
|
||||
status_publisher,
|
||||
current_enrollment,
|
||||
)
|
||||
.await;
|
||||
enroll_remote_control_server_if_missing(
|
||||
remote_control_target,
|
||||
state_db,
|
||||
&auth,
|
||||
&mut auth_context,
|
||||
auth_context,
|
||||
enrollment,
|
||||
connect_options,
|
||||
status_publisher,
|
||||
@@ -1337,82 +1431,7 @@ pub(super) async fn connect_remote_control_websocket(
|
||||
}
|
||||
}
|
||||
|
||||
let enrollment_ref = enrollment.as_ref().ok_or_else(|| {
|
||||
io::Error::other("missing remote control enrollment after enrollment step")
|
||||
})?;
|
||||
publish_current_enrollment(current_enrollment, enrollment_ref);
|
||||
let request = build_remote_control_websocket_request(
|
||||
&remote_control_target.websocket_url,
|
||||
enrollment_ref,
|
||||
connect_options.installation_id,
|
||||
connect_options.subscribe_cursor,
|
||||
)?;
|
||||
|
||||
let websocket_connect_result = tokio::time::timeout(
|
||||
REMOTE_CONTROL_WEBSOCKET_CONNECT_TIMEOUT,
|
||||
connect_async(request),
|
||||
)
|
||||
.await
|
||||
.map_err(|_| {
|
||||
io::Error::new(
|
||||
ErrorKind::TimedOut,
|
||||
format!(
|
||||
"timed out connecting to remote control websocket at `{}` after {:?}",
|
||||
remote_control_target.websocket_url, REMOTE_CONTROL_WEBSOCKET_CONNECT_TIMEOUT
|
||||
),
|
||||
)
|
||||
})?;
|
||||
|
||||
match websocket_connect_result {
|
||||
Ok((websocket_stream, response)) => Ok((websocket_stream, response.map(|_| ()))),
|
||||
Err(err) => {
|
||||
match &err {
|
||||
tungstenite::Error::Http(response) if response.status().as_u16() == 404 => {
|
||||
info!(
|
||||
"remote control websocket returned HTTP 404; clearing stale enrollment before re-enrolling: websocket_url={}, account_id={}, server_id={}, environment_id={}",
|
||||
remote_control_target.websocket_url,
|
||||
auth.account_id,
|
||||
enrollment_ref.server_id,
|
||||
enrollment_ref.environment_id
|
||||
);
|
||||
clear_remote_control_enrollment(
|
||||
state_db,
|
||||
remote_control_target,
|
||||
&auth.account_id,
|
||||
connect_options.app_server_client_name,
|
||||
enrollment,
|
||||
status_publisher,
|
||||
current_enrollment,
|
||||
)
|
||||
.await;
|
||||
}
|
||||
tungstenite::Error::Http(response)
|
||||
if matches!(response.status().as_u16(), 401 | 403) =>
|
||||
{
|
||||
enrollment
|
||||
.as_mut()
|
||||
.ok_or_else(|| {
|
||||
io::Error::other(
|
||||
"missing remote control enrollment after websocket auth failure",
|
||||
)
|
||||
})?
|
||||
.clear_server_token();
|
||||
clear_current_enrollment(current_enrollment);
|
||||
return Err(io::Error::other(format!(
|
||||
"remote control websocket auth failed with HTTP {}; refreshing server token before reconnect",
|
||||
response.status()
|
||||
)));
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
Err(io::Error::other(
|
||||
format_remote_control_websocket_connect_error(
|
||||
&remote_control_target.websocket_url,
|
||||
&err,
|
||||
),
|
||||
))
|
||||
}
|
||||
}
|
||||
Ok(auth)
|
||||
}
|
||||
|
||||
async fn clear_remote_control_enrollment(
|
||||
@@ -1422,7 +1441,6 @@ async fn clear_remote_control_enrollment(
|
||||
app_server_client_name: Option<&str>,
|
||||
enrollment: &mut Option<RemoteControlEnrollment>,
|
||||
status_publisher: &RemoteControlStatusPublisher,
|
||||
current_enrollment: &CurrentRemoteControlEnrollment,
|
||||
) {
|
||||
if let Err(clear_err) = update_persisted_remote_control_enrollment(
|
||||
Some(state_db),
|
||||
@@ -1437,7 +1455,51 @@ async fn clear_remote_control_enrollment(
|
||||
}
|
||||
*enrollment = None;
|
||||
status_publisher.publish_environment_id(/*environment_id*/ None);
|
||||
clear_current_enrollment(current_enrollment);
|
||||
}
|
||||
|
||||
async fn clear_remote_control_enrollment_if_matches(
|
||||
state_db: Option<&StateRuntime>,
|
||||
remote_control_target: &RemoteControlTarget,
|
||||
account_id: &str,
|
||||
app_server_client_name: Option<&str>,
|
||||
current_enrollment: &CurrentRemoteControlEnrollment,
|
||||
enrollment: &RemoteControlEnrollment,
|
||||
status_publisher: &RemoteControlStatusPublisher,
|
||||
) {
|
||||
let Some(state_db) = state_db else {
|
||||
return;
|
||||
};
|
||||
let mut current_enrollment = current_enrollment.lock().await;
|
||||
if !current_enrollment
|
||||
.as_ref()
|
||||
.is_some_and(|current| same_remote_control_enrollment(current, enrollment))
|
||||
{
|
||||
return;
|
||||
}
|
||||
clear_remote_control_enrollment(
|
||||
state_db,
|
||||
remote_control_target,
|
||||
account_id,
|
||||
app_server_client_name,
|
||||
&mut current_enrollment,
|
||||
status_publisher,
|
||||
)
|
||||
.await;
|
||||
}
|
||||
|
||||
async fn clear_remote_control_server_token_if_matches(
|
||||
current_enrollment: &CurrentRemoteControlEnrollment,
|
||||
enrollment: &RemoteControlEnrollment,
|
||||
) -> io::Result<()> {
|
||||
let mut current_enrollment = current_enrollment.lock().await;
|
||||
current_enrollment
|
||||
.as_mut()
|
||||
.filter(|current| same_remote_control_enrollment(current, enrollment))
|
||||
.ok_or_else(|| {
|
||||
io::Error::other("missing remote control enrollment after websocket auth failure")
|
||||
})?
|
||||
.clear_server_token();
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn enroll_remote_control_server_if_missing(
|
||||
@@ -1585,8 +1647,10 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
fn test_current_enrollment() -> CurrentRemoteControlEnrollment {
|
||||
Arc::new(std::sync::Mutex::new(None))
|
||||
fn test_current_enrollment(
|
||||
enrollment: Option<RemoteControlEnrollment>,
|
||||
) -> CurrentRemoteControlEnrollment {
|
||||
Arc::new(RemoteControlEnrollmentState::new(enrollment))
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -1739,10 +1803,9 @@ mod tests {
|
||||
let auth_manager = remote_control_auth_manager();
|
||||
let mut auth_recovery = auth_manager.unauthorized_recovery();
|
||||
let mut auth_change_rx = auth_manager.auth_change_receiver();
|
||||
let mut enrollment = Some(remote_control_enrollment(Some(
|
||||
let current_enrollment = test_current_enrollment(Some(remote_control_enrollment(Some(
|
||||
TEST_REMOTE_CONTROL_SERVER_TOKEN,
|
||||
)));
|
||||
let current_enrollment = test_current_enrollment();
|
||||
))));
|
||||
let (status_publisher, status_rx) = remote_control_status_channel();
|
||||
|
||||
let err = match connect_remote_control_websocket(
|
||||
@@ -1753,7 +1816,7 @@ mod tests {
|
||||
auth_recovery: &mut auth_recovery,
|
||||
auth_change_rx: &mut auth_change_rx,
|
||||
},
|
||||
&mut enrollment,
|
||||
¤t_enrollment,
|
||||
RemoteControlConnectOptions {
|
||||
installation_id: TEST_INSTALLATION_ID,
|
||||
server_name: "test-server",
|
||||
@@ -1761,7 +1824,6 @@ mod tests {
|
||||
app_server_client_name: None,
|
||||
},
|
||||
&status_publisher,
|
||||
¤t_enrollment,
|
||||
)
|
||||
.await
|
||||
{
|
||||
@@ -1771,12 +1833,7 @@ mod tests {
|
||||
|
||||
server_task.await.expect("server task should succeed");
|
||||
assert_eq!(err.to_string(), expected_error);
|
||||
assert!(
|
||||
current_enrollment
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner)
|
||||
.is_some()
|
||||
);
|
||||
assert!(current_enrollment.lock().await.is_some());
|
||||
assert_eq!(
|
||||
status_rx.borrow().clone(),
|
||||
RemoteControlStatusChangedNotification {
|
||||
@@ -1801,10 +1858,9 @@ mod tests {
|
||||
let auth_manager = remote_control_auth_manager();
|
||||
let mut auth_recovery = auth_manager.unauthorized_recovery();
|
||||
let mut auth_change_rx = auth_manager.auth_change_receiver();
|
||||
let mut enrollment = Some(remote_control_enrollment(Some(
|
||||
let current_enrollment = test_current_enrollment(Some(remote_control_enrollment(Some(
|
||||
TEST_REMOTE_CONTROL_SERVER_TOKEN,
|
||||
)));
|
||||
let current_enrollment = test_current_enrollment();
|
||||
))));
|
||||
let (status_publisher, status_rx) = remote_control_status_channel();
|
||||
|
||||
let server_task = tokio::spawn(async move {
|
||||
@@ -1824,7 +1880,7 @@ mod tests {
|
||||
auth_recovery: &mut auth_recovery,
|
||||
auth_change_rx: &mut auth_change_rx,
|
||||
},
|
||||
&mut enrollment,
|
||||
¤t_enrollment,
|
||||
RemoteControlConnectOptions {
|
||||
installation_id: TEST_INSTALLATION_ID,
|
||||
server_name: "test-server",
|
||||
@@ -1832,7 +1888,6 @@ mod tests {
|
||||
app_server_client_name: None,
|
||||
},
|
||||
&status_publisher,
|
||||
¤t_enrollment,
|
||||
)
|
||||
.await
|
||||
.expect_err("unauthorized response should fail the websocket connect");
|
||||
@@ -1853,13 +1908,7 @@ mod tests {
|
||||
);
|
||||
let mut expected_enrollment = remote_control_enrollment(/*remote_control_token*/ None);
|
||||
expected_enrollment.remote_control_target = remote_control_target;
|
||||
assert_eq!(enrollment, Some(expected_enrollment));
|
||||
assert!(
|
||||
current_enrollment
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner)
|
||||
.is_none()
|
||||
);
|
||||
assert_eq!(*current_enrollment.lock().await, Some(expected_enrollment));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -1896,7 +1945,7 @@ mod tests {
|
||||
.await;
|
||||
let mut auth_recovery = auth_manager.unauthorized_recovery();
|
||||
let mut auth_change_rx = auth_manager.auth_change_receiver();
|
||||
let mut enrollment = None;
|
||||
let current_enrollment = test_current_enrollment(/*enrollment*/ None);
|
||||
let (status_publisher, status_rx) = remote_control_status_channel();
|
||||
save_auth(
|
||||
codex_home.path(),
|
||||
@@ -1913,7 +1962,7 @@ mod tests {
|
||||
auth_recovery: &mut auth_recovery,
|
||||
auth_change_rx: &mut auth_change_rx,
|
||||
},
|
||||
&mut enrollment,
|
||||
¤t_enrollment,
|
||||
RemoteControlConnectOptions {
|
||||
installation_id: TEST_INSTALLATION_ID,
|
||||
server_name: "test-server",
|
||||
@@ -1921,7 +1970,6 @@ mod tests {
|
||||
app_server_client_name: None,
|
||||
},
|
||||
&status_publisher,
|
||||
&test_current_enrollment(),
|
||||
)
|
||||
.await
|
||||
.expect_err("unauthorized enrollment should fail the websocket connect");
|
||||
@@ -1989,9 +2037,9 @@ mod tests {
|
||||
.await;
|
||||
let mut auth_recovery = auth_manager.unauthorized_recovery();
|
||||
let mut auth_change_rx = auth_manager.auth_change_receiver();
|
||||
let mut enrollment = Some(remote_control_enrollment(
|
||||
let current_enrollment = test_current_enrollment(Some(remote_control_enrollment(
|
||||
/*remote_control_token*/ None,
|
||||
));
|
||||
)));
|
||||
let (status_publisher, status_rx) = remote_control_status_channel();
|
||||
save_auth(
|
||||
codex_home.path(),
|
||||
@@ -2008,7 +2056,7 @@ mod tests {
|
||||
auth_recovery: &mut auth_recovery,
|
||||
auth_change_rx: &mut auth_change_rx,
|
||||
},
|
||||
&mut enrollment,
|
||||
¤t_enrollment,
|
||||
RemoteControlConnectOptions {
|
||||
installation_id: TEST_INSTALLATION_ID,
|
||||
server_name: "test-server",
|
||||
@@ -2016,7 +2064,6 @@ mod tests {
|
||||
app_server_client_name: None,
|
||||
},
|
||||
&status_publisher,
|
||||
&test_current_enrollment(),
|
||||
)
|
||||
.await
|
||||
.expect_err("unauthorized refresh should fail the websocket connect");
|
||||
@@ -2061,9 +2108,9 @@ mod tests {
|
||||
let auth_manager = remote_control_auth_manager();
|
||||
let mut auth_recovery = auth_manager.unauthorized_recovery();
|
||||
let mut auth_change_rx = auth_manager.auth_change_receiver();
|
||||
let mut enrollment = Some(remote_control_enrollment(Some(
|
||||
let current_enrollment = test_current_enrollment(Some(remote_control_enrollment(Some(
|
||||
TEST_REMOTE_CONTROL_SERVER_TOKEN,
|
||||
)));
|
||||
))));
|
||||
let (status_publisher, _status_rx) = remote_control_status_channel();
|
||||
|
||||
let err = connect_remote_control_websocket(
|
||||
@@ -2074,7 +2121,7 @@ mod tests {
|
||||
auth_recovery: &mut auth_recovery,
|
||||
auth_change_rx: &mut auth_change_rx,
|
||||
},
|
||||
&mut enrollment,
|
||||
¤t_enrollment,
|
||||
RemoteControlConnectOptions {
|
||||
installation_id: TEST_INSTALLATION_ID,
|
||||
server_name: "test-server",
|
||||
@@ -2082,14 +2129,13 @@ mod tests {
|
||||
app_server_client_name: None,
|
||||
},
|
||||
&status_publisher,
|
||||
&test_current_enrollment(),
|
||||
)
|
||||
.await
|
||||
.expect_err("missing sqlite state db should fail remote control");
|
||||
|
||||
assert_eq!(err.kind(), ErrorKind::NotFound);
|
||||
assert_eq!(err.to_string(), "remote control requires sqlite state db");
|
||||
assert_eq!(enrollment, None);
|
||||
assert_eq!(*current_enrollment.lock().await, None);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -2107,9 +2153,9 @@ mod tests {
|
||||
.await;
|
||||
let mut auth_recovery = auth_manager.unauthorized_recovery();
|
||||
let mut auth_change_rx = auth_manager.auth_change_receiver();
|
||||
let mut enrollment = Some(remote_control_enrollment(Some(
|
||||
let current_enrollment = test_current_enrollment(Some(remote_control_enrollment(Some(
|
||||
TEST_REMOTE_CONTROL_SERVER_TOKEN,
|
||||
)));
|
||||
))));
|
||||
let (status_publisher, mut status_rx) = remote_control_status_channel();
|
||||
status_publisher.publish_environment_id(Some("env_test".to_string()));
|
||||
status_rx
|
||||
@@ -2125,7 +2171,7 @@ mod tests {
|
||||
auth_recovery: &mut auth_recovery,
|
||||
auth_change_rx: &mut auth_change_rx,
|
||||
},
|
||||
&mut enrollment,
|
||||
¤t_enrollment,
|
||||
RemoteControlConnectOptions {
|
||||
installation_id: TEST_INSTALLATION_ID,
|
||||
server_name: "test-server",
|
||||
@@ -2133,7 +2179,6 @@ mod tests {
|
||||
app_server_client_name: None,
|
||||
},
|
||||
&status_publisher,
|
||||
&test_current_enrollment(),
|
||||
)
|
||||
.await
|
||||
.expect_err("missing auth should fail remote control");
|
||||
@@ -2143,7 +2188,7 @@ mod tests {
|
||||
err.to_string(),
|
||||
"remote control requires ChatGPT authentication"
|
||||
);
|
||||
assert_eq!(enrollment, None);
|
||||
assert_eq!(*current_enrollment.lock().await, None);
|
||||
assert_eq!(
|
||||
status_rx.borrow().clone(),
|
||||
RemoteControlStatusChangedNotification {
|
||||
@@ -2185,7 +2230,8 @@ mod tests {
|
||||
RemoteControlChannels {
|
||||
transport_event_tx,
|
||||
status_publisher,
|
||||
current_enrollment: test_current_enrollment(),
|
||||
current_enrollment: test_current_enrollment(/*enrollment*/ None),
|
||||
pairing_persistence_key: watch::channel(None).0,
|
||||
},
|
||||
shutdown_token,
|
||||
enabled_rx,
|
||||
|
||||
@@ -918,7 +918,7 @@ impl MessageProcessor {
|
||||
.map(|response| Some(response.into())),
|
||||
ClientRequest::RemoteControlPairingStart { params, .. } => self
|
||||
.remote_control_processor
|
||||
.pairing_start(params)
|
||||
.pairing_start(params, app_server_client_name.as_deref())
|
||||
.await
|
||||
.map(|response| Some(response.into())),
|
||||
ClientRequest::RemoteControlClientsList { params, .. } => self
|
||||
|
||||
@@ -52,9 +52,10 @@ impl RemoteControlRequestProcessor {
|
||||
pub(crate) async fn pairing_start(
|
||||
&self,
|
||||
params: RemoteControlPairingStartParams,
|
||||
app_server_client_name: Option<&str>,
|
||||
) -> Result<RemoteControlPairingStartResponse, JSONRPCErrorError> {
|
||||
self.handle()?
|
||||
.start_pairing(params)
|
||||
.start_pairing(params, app_server_client_name)
|
||||
.await
|
||||
.map_err(map_pairing_start_error)
|
||||
}
|
||||
|
||||
+4
-1
@@ -6,7 +6,10 @@ use pretty_assertions::assert_eq;
|
||||
#[tokio::test]
|
||||
async fn pairing_start_returns_internal_error_when_remote_control_is_unavailable() {
|
||||
let err = RemoteControlRequestProcessor::new(/*remote_control_handle*/ None)
|
||||
.pairing_start(RemoteControlPairingStartParams::default())
|
||||
.pairing_start(
|
||||
RemoteControlPairingStartParams::default(),
|
||||
/*app_server_client_name*/ None,
|
||||
)
|
||||
.await
|
||||
.expect_err("missing remote control should fail pairing");
|
||||
|
||||
|
||||
@@ -52,6 +52,7 @@ fn preset_to_info(preset: &ModelPreset, priority: i32) -> ModelInfo {
|
||||
input_modalities: default_input_modalities(),
|
||||
used_fallback_model_metadata: false,
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
auto_review_model_override: None,
|
||||
tool_mode: None,
|
||||
multi_agent_version: None,
|
||||
|
||||
@@ -193,6 +193,42 @@ async fn remote_control_pairing_start_returns_pairing_artifacts() -> Result<()>
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn remote_control_pairing_start_returns_pairing_artifacts_while_disabled() -> Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let mut backend = PairingRemoteControlBackend::start(codex_home.path()).await?;
|
||||
let mut mcp = TestAppServer::new(codex_home.path()).await?;
|
||||
timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??;
|
||||
|
||||
let request_id = mcp
|
||||
.send_remote_control_pairing_start_request(RemoteControlPairingStartParams {
|
||||
manual_code: true,
|
||||
})
|
||||
.await?;
|
||||
let response: JSONRPCResponse = timeout(
|
||||
DEFAULT_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(request_id)),
|
||||
)
|
||||
.await??;
|
||||
assert_eq!(
|
||||
timeout(DEFAULT_TIMEOUT, backend.wait_for_enroll_request()).await??,
|
||||
"POST /backend-api/wham/remote/control/server/enroll HTTP/1.1"
|
||||
);
|
||||
assert_eq!(response.result.get("serverId"), None);
|
||||
let received: RemoteControlPairingStartResponse = to_response(response)?;
|
||||
|
||||
assert_eq!(
|
||||
received,
|
||||
RemoteControlPairingStartResponse {
|
||||
pairing_code: "pairing-code".to_string(),
|
||||
manual_pairing_code: Some("ABCD-EFGH".to_string()),
|
||||
environment_id: "environment-id".to_string(),
|
||||
expires_at: 33_336_362_096,
|
||||
}
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn remote_control_client_management_works_while_disabled() -> Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
@@ -376,8 +412,12 @@ impl PairingRemoteControlBackend {
|
||||
)
|
||||
.await?;
|
||||
|
||||
let _websocket_request = read_http_request(&listener).await?;
|
||||
let pair_http_request = read_http_request(&listener).await?;
|
||||
let request_after_enroll = read_http_request(&listener).await?;
|
||||
let pair_http_request = if request_after_enroll.request_line.starts_with("GET ") {
|
||||
read_http_request(&listener).await?
|
||||
} else {
|
||||
request_after_enroll
|
||||
};
|
||||
respond_with_json(
|
||||
pair_http_request.reader.into_inner(),
|
||||
serde_json::json!({
|
||||
|
||||
@@ -110,12 +110,22 @@ pub enum ResponseEvent {
|
||||
ModelsEtag(String),
|
||||
}
|
||||
|
||||
#[derive(Debug, Serialize, Clone, PartialEq)]
|
||||
#[serde(rename_all = "snake_case")]
|
||||
pub enum ReasoningContext {
|
||||
Auto,
|
||||
CurrentTurn,
|
||||
AllTurns,
|
||||
}
|
||||
|
||||
#[derive(Debug, Serialize, Clone, PartialEq)]
|
||||
pub struct Reasoning {
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub effort: Option<ReasoningEffortConfig>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub summary: Option<ReasoningSummaryConfig>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub context: Option<ReasoningContext>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Serialize, Default, Clone, PartialEq)]
|
||||
|
||||
@@ -30,6 +30,7 @@ pub use crate::common::OpenAiVerbosity;
|
||||
pub use crate::common::RawMemory;
|
||||
pub use crate::common::RawMemoryMetadata;
|
||||
pub use crate::common::Reasoning;
|
||||
pub use crate::common::ReasoningContext;
|
||||
pub use crate::common::ResponseCreateWsRequest;
|
||||
pub use crate::common::ResponseEvent;
|
||||
pub use crate::common::ResponseStream;
|
||||
|
||||
@@ -98,6 +98,7 @@ async fn models_client_hits_models_endpoint() {
|
||||
input_modalities: default_input_modalities(),
|
||||
used_fallback_model_metadata: false,
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
auto_review_model_override: None,
|
||||
tool_mode: None,
|
||||
multi_agent_version: None,
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
use std::fs::File;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use std::process::Command;
|
||||
@@ -22,8 +23,11 @@ const CURATED_PLUGINS_BACKUP_ARCHIVE_API_URL: &str =
|
||||
"https://chatgpt.com/backend-api/plugins/export/curated";
|
||||
const OPENAI_PLUGINS_OWNER: &str = "openai";
|
||||
const OPENAI_PLUGINS_REPO: &str = "plugins";
|
||||
const OPENAI_PLUGINS_GIT_URL: &str = "https://github.com/openai/plugins.git";
|
||||
const CURATED_PLUGINS_FETCH_REF: &str = "refs/codex/curated-sync";
|
||||
const CURATED_PLUGINS_RELATIVE_DIR: &str = ".tmp/plugins";
|
||||
const CURATED_PLUGINS_SHA_FILE: &str = ".tmp/plugins.sha";
|
||||
const CURATED_PLUGINS_SYNC_LOCK_FILE: &str = ".tmp/plugins.sync.lock";
|
||||
const CURATED_PLUGINS_BACKUP_ARCHIVE_FALLBACK_VERSION: &str = "export-backup";
|
||||
const CURATED_PLUGINS_GIT_TIMEOUT: Duration = Duration::from_secs(30);
|
||||
const CURATED_PLUGINS_HTTP_TIMEOUT: Duration = Duration::from_secs(30);
|
||||
@@ -78,6 +82,8 @@ fn sync_openai_plugins_repo_with_transport_overrides(
|
||||
api_base_url: &str,
|
||||
backup_archive_api_url: &str,
|
||||
) -> Result<String, String> {
|
||||
let _file_guard = lock_curated_plugins_startup_sync(codex_home)?;
|
||||
|
||||
match sync_openai_plugins_repo_via_git(codex_home, git_binary) {
|
||||
Ok(remote_sha) => {
|
||||
emit_curated_plugins_startup_sync_metric("git", "success");
|
||||
@@ -135,6 +141,22 @@ fn sync_openai_plugins_repo_with_transport_overrides(
|
||||
}
|
||||
}
|
||||
|
||||
fn lock_curated_plugins_startup_sync(codex_home: &Path) -> Result<File, String> {
|
||||
let lock_path = codex_home.join(CURATED_PLUGINS_SYNC_LOCK_FILE);
|
||||
std::fs::create_dir_all(codex_home.join(".tmp"))
|
||||
.map_err(|err| format!("failed to create curated plugins sync directory: {err}"))?;
|
||||
let lock_file = File::options()
|
||||
.write(true)
|
||||
.create(true)
|
||||
.truncate(false)
|
||||
.open(&lock_path)
|
||||
.map_err(|err| format!("failed to open curated plugins sync lock: {err}"))?;
|
||||
lock_file
|
||||
.lock()
|
||||
.map_err(|err| format!("failed to lock curated plugins sync: {err}"))?;
|
||||
Ok(lock_file)
|
||||
}
|
||||
|
||||
fn sync_openai_plugins_repo_via_git(codex_home: &Path, git_binary: &str) -> Result<String, String> {
|
||||
let repo_path = curated_plugins_repo_path(codex_home);
|
||||
let sha_path = codex_home.join(CURATED_PLUGINS_SHA_FILE);
|
||||
@@ -146,23 +168,30 @@ fn sync_openai_plugins_repo_via_git(codex_home: &Path, git_binary: &str) -> Resu
|
||||
}
|
||||
|
||||
let staged_repo_dir = prepare_curated_repo_parent_and_temp_dir(&repo_path)?;
|
||||
let clone_output = run_git_command_with_timeout(
|
||||
Command::new(git_binary)
|
||||
.env("GIT_OPTIONAL_LOCKS", "0")
|
||||
.arg("clone")
|
||||
.arg("--depth")
|
||||
.arg("1")
|
||||
.arg("https://github.com/openai/plugins.git")
|
||||
.arg(staged_repo_dir.path()),
|
||||
"git clone curated plugins repo",
|
||||
CURATED_PLUGINS_GIT_TIMEOUT,
|
||||
run_git_in_repo(
|
||||
staged_repo_dir.path(),
|
||||
git_binary,
|
||||
&["init"],
|
||||
"git init curated plugins repo",
|
||||
)?;
|
||||
ensure_git_success(&clone_output, "git clone curated plugins repo")?;
|
||||
|
||||
let cloned_sha = git_head_sha(staged_repo_dir.path(), git_binary)?;
|
||||
if cloned_sha != remote_sha {
|
||||
if repo_path.join(".git").is_dir() {
|
||||
fetch_curated_plugins_commit(&repo_path, &remote_sha, git_binary)?;
|
||||
fetch_curated_plugins_commit_from_source(
|
||||
staged_repo_dir.path(),
|
||||
&repo_path,
|
||||
CURATED_PLUGINS_FETCH_REF,
|
||||
git_binary,
|
||||
)?;
|
||||
} else {
|
||||
fetch_curated_plugins_commit(staged_repo_dir.path(), &remote_sha, git_binary)?;
|
||||
}
|
||||
|
||||
reset_curated_plugins_checkout(staged_repo_dir.path(), git_binary)?;
|
||||
let fetched_sha = git_head_sha(staged_repo_dir.path(), git_binary)?;
|
||||
if fetched_sha != remote_sha {
|
||||
return Err(format!(
|
||||
"curated plugins clone HEAD mismatch: expected {remote_sha}, got {cloned_sha}"
|
||||
"curated plugins fetch HEAD mismatch: expected {remote_sha}, got {fetched_sha}"
|
||||
));
|
||||
}
|
||||
|
||||
@@ -172,6 +201,90 @@ fn sync_openai_plugins_repo_via_git(codex_home: &Path, git_binary: &str) -> Resu
|
||||
Ok(remote_sha)
|
||||
}
|
||||
|
||||
fn fetch_curated_plugins_commit(
|
||||
repo_path: &Path,
|
||||
remote_sha: &str,
|
||||
git_binary: &str,
|
||||
) -> Result<(), String> {
|
||||
fetch_curated_plugins_commit_from(
|
||||
repo_path,
|
||||
OPENAI_PLUGINS_GIT_URL.as_ref(),
|
||||
remote_sha,
|
||||
git_binary,
|
||||
"git fetch curated plugins repo",
|
||||
)
|
||||
}
|
||||
|
||||
fn fetch_curated_plugins_commit_from_source(
|
||||
repo_path: &Path,
|
||||
source_repo_path: &Path,
|
||||
remote_sha: &str,
|
||||
git_binary: &str,
|
||||
) -> Result<(), String> {
|
||||
fetch_curated_plugins_commit_from(
|
||||
repo_path,
|
||||
source_repo_path,
|
||||
remote_sha,
|
||||
git_binary,
|
||||
"git copy fetched curated plugins commit",
|
||||
)
|
||||
}
|
||||
|
||||
fn fetch_curated_plugins_commit_from(
|
||||
repo_path: &Path,
|
||||
source: &Path,
|
||||
source_revision: &str,
|
||||
git_binary: &str,
|
||||
context: &str,
|
||||
) -> Result<(), String> {
|
||||
let fetch_refspec = format!("+{source_revision}:{CURATED_PLUGINS_FETCH_REF}");
|
||||
let output = run_git_command_with_timeout(
|
||||
Command::new(git_binary)
|
||||
.env("GIT_OPTIONAL_LOCKS", "0")
|
||||
.arg("-C")
|
||||
.arg(repo_path)
|
||||
.args(["fetch", "--depth", "1", "--no-tags"])
|
||||
.arg(source)
|
||||
.arg(fetch_refspec),
|
||||
context,
|
||||
CURATED_PLUGINS_GIT_TIMEOUT,
|
||||
)?;
|
||||
ensure_git_success(&output, context)
|
||||
}
|
||||
|
||||
fn reset_curated_plugins_checkout(repo_path: &Path, git_binary: &str) -> Result<(), String> {
|
||||
run_git_in_repo(
|
||||
repo_path,
|
||||
git_binary,
|
||||
&["reset", "--hard", CURATED_PLUGINS_FETCH_REF],
|
||||
"git reset curated plugins repo",
|
||||
)?;
|
||||
run_git_in_repo(
|
||||
repo_path,
|
||||
git_binary,
|
||||
&["clean", "-fdx"],
|
||||
"git clean curated plugins repo",
|
||||
)
|
||||
}
|
||||
|
||||
fn run_git_in_repo(
|
||||
repo_path: &Path,
|
||||
git_binary: &str,
|
||||
args: &[&str],
|
||||
context: &str,
|
||||
) -> Result<(), String> {
|
||||
let output = run_git_command_with_timeout(
|
||||
Command::new(git_binary)
|
||||
.env("GIT_OPTIONAL_LOCKS", "0")
|
||||
.arg("-C")
|
||||
.arg(repo_path)
|
||||
.args(args),
|
||||
context,
|
||||
CURATED_PLUGINS_GIT_TIMEOUT,
|
||||
)?;
|
||||
ensure_git_success(&output, context)
|
||||
}
|
||||
|
||||
fn sync_openai_plugins_repo_via_http(
|
||||
codex_home: &Path,
|
||||
api_base_url: &str,
|
||||
|
||||
@@ -3,6 +3,8 @@ use pretty_assertions::assert_eq;
|
||||
use std::io::Write;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
#[cfg(unix)]
|
||||
use std::sync::Barrier;
|
||||
use tempfile::tempdir;
|
||||
use wiremock::Mock;
|
||||
use wiremock::MockServer;
|
||||
@@ -95,6 +97,22 @@ fn write_executable_script(path: &Path, contents: &str) {
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
fn run_git(repo: &Path, args: &[&str]) -> std::process::Output {
|
||||
let output = Command::new("git")
|
||||
.arg("-C")
|
||||
.arg(repo)
|
||||
.args(args)
|
||||
.output()
|
||||
.expect("run git");
|
||||
assert!(
|
||||
output.status.success(),
|
||||
"git {args:?} failed: {}",
|
||||
String::from_utf8_lossy(&output.stderr)
|
||||
);
|
||||
output
|
||||
}
|
||||
|
||||
async fn mount_github_repo_and_ref(server: &MockServer, sha: &str) {
|
||||
Mock::given(method("GET"))
|
||||
.and(path("/repos/openai/plugins"))
|
||||
@@ -253,30 +271,42 @@ fn remove_stale_curated_repo_temp_dirs_removes_only_matching_directories() {
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn sync_openai_plugins_repo_prefers_git_when_available() {
|
||||
fn concurrent_syncs_serialize_fetches_without_skipping_remote_checks() {
|
||||
let tmp = tempdir().expect("tempdir");
|
||||
let bin_dir = tempfile::Builder::new()
|
||||
.prefix("fake-git-")
|
||||
.tempdir()
|
||||
.expect("tempdir");
|
||||
let git_path = bin_dir.path().join("git");
|
||||
let invocation_log = bin_dir.path().join("invocations.log");
|
||||
let sha = "0123456789abcdef0123456789abcdef01234567";
|
||||
|
||||
write_executable_script(
|
||||
&git_path,
|
||||
&format!(
|
||||
r#"#!/bin/sh
|
||||
printf '%s\n' "$*" >> '{}'
|
||||
if [ "$1" = "ls-remote" ]; then
|
||||
sleep 1
|
||||
printf '%s\tHEAD\n' "{sha}"
|
||||
exit 0
|
||||
fi
|
||||
if [ "$1" = "clone" ]; then
|
||||
dest="$5"
|
||||
mkdir -p "$dest/.git" "$dest/.agents/plugins" "$dest/plugins/gmail/.codex-plugin"
|
||||
cat > "$dest/.agents/plugins/marketplace.json" <<'EOF'
|
||||
if [ "$1" = "-C" ] && [ "$3" = "init" ]; then
|
||||
mkdir -p "$2/.git"
|
||||
exit 0
|
||||
fi
|
||||
if [ "$1" = "-C" ] && [ "$3" = "fetch" ]; then
|
||||
exit 0
|
||||
fi
|
||||
if [ "$1" = "-C" ] && [ "$3" = "reset" ]; then
|
||||
mkdir -p "$2/.agents/plugins" "$2/plugins/gmail/.codex-plugin"
|
||||
cat > "$2/.agents/plugins/marketplace.json" <<'EOF'
|
||||
{{"name":"openai-curated","plugins":[{{"name":"gmail","source":{{"source":"local","path":"./plugins/gmail"}}}}]}}
|
||||
EOF
|
||||
printf '%s\n' '{{"name":"gmail"}}' > "$dest/plugins/gmail/.codex-plugin/plugin.json"
|
||||
printf '%s\n' '{{"name":"gmail"}}' > "$2/plugins/gmail/.codex-plugin/plugin.json"
|
||||
exit 0
|
||||
fi
|
||||
if [ "$1" = "-C" ] && [ "$3" = "clean" ]; then
|
||||
exit 0
|
||||
fi
|
||||
if [ "$1" = "-C" ] && [ "$3" = "rev-parse" ] && [ "$4" = "HEAD" ]; then
|
||||
@@ -285,23 +315,55 @@ if [ "$1" = "-C" ] && [ "$3" = "rev-parse" ] && [ "$4" = "HEAD" ]; then
|
||||
fi
|
||||
echo "unexpected git invocation: $@" >&2
|
||||
exit 1
|
||||
"#
|
||||
"#,
|
||||
invocation_log.display()
|
||||
),
|
||||
);
|
||||
|
||||
let synced_sha = sync_openai_plugins_repo_with_transport_overrides(
|
||||
tmp.path(),
|
||||
git_path.to_str().expect("utf8 path"),
|
||||
"http://127.0.0.1:9",
|
||||
"http://127.0.0.1:9/backend-api/plugins/export/curated",
|
||||
)
|
||||
.expect("git sync should succeed");
|
||||
let barrier = Barrier::new(2);
|
||||
let results = std::thread::scope(|scope| {
|
||||
let run_sync = || {
|
||||
barrier.wait();
|
||||
sync_openai_plugins_repo_with_transport_overrides(
|
||||
tmp.path(),
|
||||
git_path.to_str().expect("utf8 path"),
|
||||
"http://127.0.0.1:9",
|
||||
"http://127.0.0.1:9/backend-api/plugins/export/curated",
|
||||
)
|
||||
};
|
||||
let first = scope.spawn(run_sync);
|
||||
let second = scope.spawn(run_sync);
|
||||
[
|
||||
first.join().expect("first sync thread"),
|
||||
second.join().expect("second sync thread"),
|
||||
]
|
||||
});
|
||||
|
||||
assert_eq!(synced_sha, sha);
|
||||
assert_eq!(results, [Ok(sha.to_string()), Ok(sha.to_string())]);
|
||||
let repo_path = curated_plugins_repo_path(tmp.path());
|
||||
assert!(repo_path.join(".git").is_dir());
|
||||
assert_curated_gmail_repo(&repo_path);
|
||||
assert_eq!(read_curated_plugins_sha(tmp.path()).as_deref(), Some(sha));
|
||||
let invocations = std::fs::read_to_string(invocation_log).expect("read invocation log");
|
||||
assert_eq!(
|
||||
invocations
|
||||
.lines()
|
||||
.filter(|invocation| invocation.starts_with("ls-remote "))
|
||||
.count(),
|
||||
2
|
||||
);
|
||||
assert_eq!(
|
||||
invocations
|
||||
.lines()
|
||||
.filter(|invocation| invocation.contains(" fetch --depth 1 --no-tags "))
|
||||
.count(),
|
||||
1
|
||||
);
|
||||
assert!(
|
||||
!invocations
|
||||
.lines()
|
||||
.any(|invocation| invocation.split_whitespace().any(|arg| arg == "clone"))
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
@@ -328,36 +390,20 @@ fn sync_openai_plugins_repo_via_git_succeeds_with_local_rewritten_remote() {
|
||||
)
|
||||
.expect("write plugin manifest");
|
||||
|
||||
let init_status = Command::new("git")
|
||||
.arg("-C")
|
||||
.arg(&work_repo)
|
||||
.arg("init")
|
||||
.status()
|
||||
.expect("run git init");
|
||||
assert!(init_status.success());
|
||||
|
||||
let add_status = Command::new("git")
|
||||
.arg("-C")
|
||||
.arg(&work_repo)
|
||||
.arg("add")
|
||||
.arg(".")
|
||||
.status()
|
||||
.expect("run git add");
|
||||
assert!(add_status.success());
|
||||
|
||||
let commit_status = Command::new("git")
|
||||
.arg("-C")
|
||||
.arg(&work_repo)
|
||||
.arg("-c")
|
||||
.arg("user.name=Codex Test")
|
||||
.arg("-c")
|
||||
.arg("user.email=codex@example.com")
|
||||
.arg("commit")
|
||||
.arg("-m")
|
||||
.arg("init")
|
||||
.status()
|
||||
.expect("run git commit");
|
||||
assert!(commit_status.success());
|
||||
run_git(&work_repo, &["init"]);
|
||||
run_git(&work_repo, &["add", "."]);
|
||||
run_git(
|
||||
&work_repo,
|
||||
&[
|
||||
"-c",
|
||||
"user.name=Codex Test",
|
||||
"-c",
|
||||
"user.email=codex@example.com",
|
||||
"commit",
|
||||
"-m",
|
||||
"init",
|
||||
],
|
||||
);
|
||||
|
||||
std::fs::create_dir_all(remote_repo.parent().expect("remote parent"))
|
||||
.expect("create remote parent");
|
||||
@@ -370,14 +416,7 @@ fn sync_openai_plugins_repo_via_git_succeeds_with_local_rewritten_remote() {
|
||||
.expect("run git clone --bare");
|
||||
assert!(clone_status.success());
|
||||
|
||||
let sha_output = Command::new("git")
|
||||
.arg("-C")
|
||||
.arg(&work_repo)
|
||||
.arg("rev-parse")
|
||||
.arg("HEAD")
|
||||
.output()
|
||||
.expect("run git rev-parse");
|
||||
assert!(sha_output.status.success());
|
||||
let sha_output = run_git(&work_repo, &["rev-parse", "HEAD"]);
|
||||
let sha = String::from_utf8_lossy(&sha_output.stdout)
|
||||
.trim()
|
||||
.to_string();
|
||||
@@ -397,10 +436,12 @@ fn sync_openai_plugins_repo_via_git_succeeds_with_local_rewritten_remote() {
|
||||
.tempdir()
|
||||
.expect("tempdir");
|
||||
let git_wrapper = bin_dir.path().join("git");
|
||||
let invocation_log = bin_dir.path().join("invocations.log");
|
||||
write_executable_script(
|
||||
&git_wrapper,
|
||||
&format!(
|
||||
"#!/bin/sh\nGIT_CONFIG_GLOBAL='{}' exec git \"$@\"\n",
|
||||
"#!/bin/sh\nprintf '%s\\n' \"$*\" >> '{}'\nGIT_CONFIG_GLOBAL='{}' exec git \"$@\"\n",
|
||||
invocation_log.display(),
|
||||
git_config_path.display()
|
||||
),
|
||||
);
|
||||
@@ -416,6 +457,125 @@ fn sync_openai_plugins_repo_via_git_succeeds_with_local_rewritten_remote() {
|
||||
Some(sha.as_str())
|
||||
);
|
||||
assert!(!has_plugins_clone_dirs(tmp.path()));
|
||||
|
||||
let first_sync_invocation_count = std::fs::read_to_string(&invocation_log)
|
||||
.expect("read first sync invocations")
|
||||
.lines()
|
||||
.count();
|
||||
let first_sync_invocations =
|
||||
std::fs::read_to_string(&invocation_log).expect("read first sync invocations");
|
||||
assert!(
|
||||
first_sync_invocations
|
||||
.lines()
|
||||
.any(|invocation| invocation.contains(" fetch --depth 1 --no-tags "))
|
||||
);
|
||||
assert!(
|
||||
!first_sync_invocations
|
||||
.lines()
|
||||
.any(|invocation| invocation.split_whitespace().any(|arg| arg == "clone"))
|
||||
);
|
||||
write_openai_curated_marketplace(&work_repo, &["gmail", "linear"]);
|
||||
run_git(&work_repo, &["add", "."]);
|
||||
run_git(
|
||||
&work_repo,
|
||||
&[
|
||||
"-c",
|
||||
"user.name=Codex Test",
|
||||
"-c",
|
||||
"user.email=codex@example.com",
|
||||
"commit",
|
||||
"-m",
|
||||
"update",
|
||||
],
|
||||
);
|
||||
let branch_output = run_git(&work_repo, &["symbolic-ref", "--short", "HEAD"]);
|
||||
let branch = String::from_utf8_lossy(&branch_output.stdout)
|
||||
.trim()
|
||||
.to_string();
|
||||
let remote_repo = remote_repo.to_str().expect("utf8 remote repo");
|
||||
let push_ref = format!("HEAD:refs/heads/{branch}");
|
||||
run_git(&work_repo, &["push", remote_repo, &push_ref]);
|
||||
let updated_sha_output = run_git(&work_repo, &["rev-parse", "HEAD"]);
|
||||
let updated_sha = String::from_utf8_lossy(&updated_sha_output.stdout)
|
||||
.trim()
|
||||
.to_string();
|
||||
|
||||
let synced_sha =
|
||||
sync_openai_plugins_repo_via_git(tmp.path(), git_wrapper.to_str().expect("utf8 path"))
|
||||
.expect("incremental git sync should succeed");
|
||||
|
||||
assert_eq!(synced_sha, updated_sha);
|
||||
assert!(
|
||||
curated_plugins_repo_path(tmp.path())
|
||||
.join("plugins/linear/.codex-plugin/plugin.json")
|
||||
.is_file()
|
||||
);
|
||||
assert_eq!(
|
||||
read_curated_plugins_sha(tmp.path()).as_deref(),
|
||||
Some(updated_sha.as_str())
|
||||
);
|
||||
assert!(
|
||||
!curated_plugins_repo_path(tmp.path())
|
||||
.join(".git/objects/info/alternates")
|
||||
.exists()
|
||||
);
|
||||
let invocation_log_contents =
|
||||
std::fs::read_to_string(&invocation_log).expect("read sync invocations");
|
||||
let incremental_sync_invocations = invocation_log_contents
|
||||
.lines()
|
||||
.skip(first_sync_invocation_count)
|
||||
.collect::<Vec<_>>();
|
||||
let curated_repo_path = curated_plugins_repo_path(tmp.path());
|
||||
assert!(incremental_sync_invocations.iter().any(|invocation| {
|
||||
invocation.starts_with(&format!("-C {} fetch ", curated_repo_path.display()))
|
||||
&& invocation.contains(" https://github.com/openai/plugins.git ")
|
||||
&& invocation.contains(updated_sha.as_str())
|
||||
&& invocation.ends_with(CURATED_PLUGINS_FETCH_REF)
|
||||
}));
|
||||
assert!(incremental_sync_invocations.iter().any(|invocation| {
|
||||
invocation.contains(" fetch --depth 1 --no-tags ")
|
||||
&& invocation.contains(&format!(" {} ", curated_repo_path.display()))
|
||||
&& invocation.ends_with(&format!(
|
||||
"{CURATED_PLUGINS_FETCH_REF}:{CURATED_PLUGINS_FETCH_REF}"
|
||||
))
|
||||
}));
|
||||
assert!(
|
||||
incremental_sync_invocations
|
||||
.iter()
|
||||
.any(|invocation| invocation.ends_with(" init"))
|
||||
);
|
||||
assert!(
|
||||
!incremental_sync_invocations
|
||||
.iter()
|
||||
.any(|invocation| invocation.split_whitespace().any(|arg| arg == "clone"))
|
||||
);
|
||||
assert!(!incremental_sync_invocations.iter().any(|invocation| {
|
||||
invocation.starts_with(&format!("-C {} reset ", curated_repo_path.display()))
|
||||
|| invocation.starts_with(&format!("-C {} clean ", curated_repo_path.display()))
|
||||
}));
|
||||
assert!(!has_plugins_clone_dirs(tmp.path()));
|
||||
|
||||
let unchanged_sync_invocation_count = invocation_log_contents.lines().count();
|
||||
let synced_sha =
|
||||
sync_openai_plugins_repo_via_git(tmp.path(), git_wrapper.to_str().expect("utf8 path"))
|
||||
.expect("unchanged git sync should succeed");
|
||||
|
||||
assert_eq!(synced_sha, updated_sha);
|
||||
let invocation_log = std::fs::read_to_string(&invocation_log).expect("read sync invocations");
|
||||
let unchanged_sync_invocations = invocation_log
|
||||
.lines()
|
||||
.skip(unchanged_sync_invocation_count)
|
||||
.collect::<Vec<_>>();
|
||||
assert!(
|
||||
unchanged_sync_invocations
|
||||
.iter()
|
||||
.any(|invocation| invocation.starts_with("ls-remote "))
|
||||
);
|
||||
assert!(
|
||||
!unchanged_sync_invocations
|
||||
.iter()
|
||||
.any(|invocation| invocation.contains(" fetch "))
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -482,7 +642,7 @@ exit 1
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn sync_openai_plugins_repo_via_git_cleans_up_staged_dir_on_clone_failure() {
|
||||
fn sync_openai_plugins_repo_via_git_cleans_up_staged_dir_on_fetch_failure() {
|
||||
let tmp = tempdir().expect("tempdir");
|
||||
let bin_dir = tempfile::Builder::new()
|
||||
.prefix("fake-git-partial-fail-")
|
||||
@@ -499,9 +659,11 @@ if [ "$1" = "ls-remote" ]; then
|
||||
printf '%s\tHEAD\n' "{sha}"
|
||||
exit 0
|
||||
fi
|
||||
if [ "$1" = "clone" ]; then
|
||||
dest="$5"
|
||||
mkdir -p "$dest/.git"
|
||||
if [ "$1" = "-C" ] && [ "$3" = "init" ]; then
|
||||
mkdir -p "$2/.git"
|
||||
exit 0
|
||||
fi
|
||||
if [ "$1" = "-C" ] && [ "$3" = "fetch" ]; then
|
||||
echo "fatal: early EOF" >&2
|
||||
exit 128
|
||||
fi
|
||||
@@ -518,6 +680,77 @@ exit 1
|
||||
assert!(!has_plugins_clone_dirs(tmp.path()));
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn sync_openai_plugins_repo_via_git_preserves_existing_snapshot_on_validation_failure() {
|
||||
let tmp = tempdir().expect("tempdir");
|
||||
let repo_path = curated_plugins_repo_path(tmp.path());
|
||||
write_openai_curated_marketplace(&repo_path, &["gmail"]);
|
||||
std::fs::create_dir_all(repo_path.join(".git")).expect("create git dir");
|
||||
write_curated_plugin_sha(tmp.path());
|
||||
|
||||
let bin_dir = tempfile::Builder::new()
|
||||
.prefix("fake-git-invalid-update-")
|
||||
.tempdir()
|
||||
.expect("tempdir");
|
||||
let git_path = bin_dir.path().join("git");
|
||||
let remote_sha = "fedcba9876543210fedcba9876543210fedcba98";
|
||||
|
||||
write_executable_script(
|
||||
&git_path,
|
||||
&format!(
|
||||
r#"#!/bin/sh
|
||||
if [ "$1" = "ls-remote" ]; then
|
||||
printf '%s\tHEAD\n' "{remote_sha}"
|
||||
exit 0
|
||||
fi
|
||||
if [ "$1" = "-C" ] && [ "$2" = "{}" ] && [ "$3" = "rev-parse" ]; then
|
||||
printf '%s\n' "{TEST_CURATED_PLUGIN_SHA}"
|
||||
exit 0
|
||||
fi
|
||||
if [ "$1" = "-C" ] && [ "$2" = "{}" ] && [ "$3" = "fetch" ]; then
|
||||
exit 0
|
||||
fi
|
||||
if [ "$1" = "-C" ] && [ "$3" = "init" ]; then
|
||||
mkdir -p "$2/.git"
|
||||
exit 0
|
||||
fi
|
||||
if [ "$1" = "-C" ] && [ "$3" = "fetch" ]; then
|
||||
exit 0
|
||||
fi
|
||||
if [ "$1" = "-C" ] && [ "$3" = "reset" ]; then
|
||||
mkdir -p "$2/plugins/linear/.codex-plugin"
|
||||
printf '%s\n' '{{"name":"linear"}}' > "$2/plugins/linear/.codex-plugin/plugin.json"
|
||||
exit 0
|
||||
fi
|
||||
if [ "$1" = "-C" ] && [ "$3" = "clean" ]; then
|
||||
exit 0
|
||||
fi
|
||||
if [ "$1" = "-C" ] && [ "$3" = "rev-parse" ]; then
|
||||
printf '%s\n' "{remote_sha}"
|
||||
exit 0
|
||||
fi
|
||||
echo "unexpected git invocation: $@" >&2
|
||||
exit 1
|
||||
"#,
|
||||
repo_path.display(),
|
||||
repo_path.display(),
|
||||
),
|
||||
);
|
||||
|
||||
let err = sync_openai_plugins_repo_via_git(tmp.path(), git_path.to_str().expect("utf8 path"))
|
||||
.expect_err("invalid staged checkout should fail");
|
||||
|
||||
assert!(err.contains("curated plugins archive missing marketplace manifest"));
|
||||
assert_curated_gmail_repo(&repo_path);
|
||||
assert!(!repo_path.join("plugins/linear").exists());
|
||||
assert_eq!(
|
||||
read_curated_plugins_sha(tmp.path()).as_deref(),
|
||||
Some(TEST_CURATED_PLUGIN_SHA)
|
||||
);
|
||||
assert!(!has_plugins_clone_dirs(tmp.path()));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn sync_openai_plugins_repo_via_http_cleans_up_staged_dir_on_extract_failure() {
|
||||
let tmp = tempdir().expect("tempdir");
|
||||
|
||||
@@ -44,6 +44,7 @@ use codex_api::RawMemory as ApiRawMemory;
|
||||
use codex_api::RealtimeCallClient as ApiRealtimeCallClient;
|
||||
use codex_api::RealtimeSessionConfig as ApiRealtimeSessionConfig;
|
||||
use codex_api::Reasoning;
|
||||
use codex_api::ReasoningContext;
|
||||
use codex_api::RequestTelemetry;
|
||||
use codex_api::ReqwestTransport;
|
||||
use codex_api::ResponseCreateWsRequest;
|
||||
@@ -609,6 +610,7 @@ impl ModelClient {
|
||||
reasoning: effort.map(|effort| Reasoning {
|
||||
effort: Some(effort),
|
||||
summary: None,
|
||||
context: None,
|
||||
}),
|
||||
};
|
||||
|
||||
@@ -727,6 +729,11 @@ impl ModelClient {
|
||||
} else {
|
||||
Some(summary)
|
||||
},
|
||||
// When Responses Lite is disabled, omit context so Responses uses the default,
|
||||
// which is currently `current_turn`.
|
||||
context: model_info
|
||||
.use_responses_lite
|
||||
.then_some(ReasoningContext::AllTurns),
|
||||
})
|
||||
} else {
|
||||
None
|
||||
@@ -775,7 +782,7 @@ impl ModelClient {
|
||||
input,
|
||||
tools,
|
||||
tool_choice: "auto".to_string(),
|
||||
parallel_tool_calls: prompt.parallel_tool_calls,
|
||||
parallel_tool_calls: prompt.parallel_tool_calls && !model_info.use_responses_lite,
|
||||
reasoning,
|
||||
store: provider.is_azure_responses_endpoint(),
|
||||
stream: true,
|
||||
|
||||
@@ -22,7 +22,6 @@ use codex_execpolicy::blocking_append_network_rule;
|
||||
use codex_protocol::approvals::ExecPolicyAmendment;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::permissions::FileSystemSandboxKind;
|
||||
use codex_protocol::permissions::FileSystemSandboxPolicy;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_shell_command::is_dangerous_command::command_might_be_dangerous;
|
||||
use codex_shell_command::is_safe_command::is_known_safe_command;
|
||||
@@ -121,7 +120,6 @@ pub(crate) enum ExecPolicyCommandOrigin {
|
||||
pub(crate) struct UnmatchedCommandContext<'a> {
|
||||
pub(crate) approval_policy: AskForApproval,
|
||||
pub(crate) permission_profile: &'a PermissionProfile,
|
||||
pub(crate) file_system_sandbox_policy: &'a FileSystemSandboxPolicy,
|
||||
pub(crate) sandbox_cwd: &'a Path,
|
||||
pub(crate) sandbox_permissions: SandboxPermissions,
|
||||
pub(crate) used_complex_parsing: bool,
|
||||
@@ -242,7 +240,6 @@ pub(crate) struct ExecApprovalRequest<'a> {
|
||||
pub(crate) command: &'a [String],
|
||||
pub(crate) approval_policy: AskForApproval,
|
||||
pub(crate) permission_profile: PermissionProfile,
|
||||
pub(crate) file_system_sandbox_policy: &'a FileSystemSandboxPolicy,
|
||||
pub(crate) sandbox_cwd: &'a Path,
|
||||
pub(crate) sandbox_permissions: SandboxPermissions,
|
||||
pub(crate) prefix_rule: Option<Vec<String>>,
|
||||
@@ -277,7 +274,6 @@ impl ExecPolicyManager {
|
||||
command,
|
||||
approval_policy,
|
||||
permission_profile,
|
||||
file_system_sandbox_policy,
|
||||
sandbox_cwd,
|
||||
sandbox_permissions,
|
||||
prefix_rule,
|
||||
@@ -298,7 +294,6 @@ impl ExecPolicyManager {
|
||||
UnmatchedCommandContext {
|
||||
approval_policy,
|
||||
permission_profile: &permission_profile,
|
||||
file_system_sandbox_policy,
|
||||
sandbox_cwd,
|
||||
sandbox_permissions,
|
||||
used_complex_parsing,
|
||||
@@ -636,12 +631,12 @@ pub(crate) fn render_decision_for_unmatched_command(
|
||||
let UnmatchedCommandContext {
|
||||
approval_policy,
|
||||
permission_profile,
|
||||
file_system_sandbox_policy,
|
||||
sandbox_cwd,
|
||||
sandbox_permissions,
|
||||
used_complex_parsing,
|
||||
command_origin,
|
||||
} = context;
|
||||
let file_system_sandbox_policy = permission_profile.file_system_sandbox_policy();
|
||||
let is_known_safe = match command_origin {
|
||||
ExecPolicyCommandOrigin::Generic => is_known_safe_command(command),
|
||||
#[cfg(windows)]
|
||||
@@ -652,12 +647,8 @@ pub(crate) fn render_decision_for_unmatched_command(
|
||||
|
||||
// On Windows, ReadOnly sandbox is not a real sandbox, so special-case it
|
||||
// here.
|
||||
let environment_lacks_sandbox_protections = cfg!(windows)
|
||||
&& profile_is_managed_read_only(
|
||||
permission_profile,
|
||||
file_system_sandbox_policy,
|
||||
sandbox_cwd,
|
||||
);
|
||||
let environment_lacks_sandbox_protections =
|
||||
cfg!(windows) && profile_is_managed_read_only(permission_profile, sandbox_cwd);
|
||||
|
||||
if is_known_safe
|
||||
&& !used_complex_parsing
|
||||
@@ -751,9 +742,9 @@ pub(crate) fn render_decision_for_unmatched_command(
|
||||
|
||||
fn profile_is_managed_read_only(
|
||||
permission_profile: &PermissionProfile,
|
||||
file_system_sandbox_policy: &FileSystemSandboxPolicy,
|
||||
sandbox_cwd: &Path,
|
||||
) -> bool {
|
||||
let file_system_sandbox_policy = permission_profile.file_system_sandbox_policy();
|
||||
matches!(permission_profile, PermissionProfile::Managed { .. })
|
||||
&& matches!(
|
||||
file_system_sandbox_policy.kind,
|
||||
|
||||
@@ -19,6 +19,7 @@ use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::permissions::FileSystemAccessMode;
|
||||
use codex_protocol::permissions::FileSystemPath;
|
||||
use codex_protocol::permissions::FileSystemSandboxEntry;
|
||||
use codex_protocol::permissions::FileSystemSandboxPolicy;
|
||||
use codex_protocol::permissions::FileSystemSpecialPath;
|
||||
use codex_protocol::permissions::NetworkSandboxPolicy;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
@@ -104,31 +105,6 @@ async fn write_project_trust_config(
|
||||
.await
|
||||
}
|
||||
|
||||
fn read_only_file_system_sandbox_policy() -> FileSystemSandboxPolicy {
|
||||
FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Special {
|
||||
value: FileSystemSpecialPath::Root,
|
||||
},
|
||||
access: FileSystemAccessMode::Read,
|
||||
}])
|
||||
}
|
||||
|
||||
fn workspace_write_file_system_sandbox_policy() -> FileSystemSandboxPolicy {
|
||||
FileSystemSandboxPolicy::workspace_write(
|
||||
&[],
|
||||
/*exclude_tmpdir_env_var*/ false,
|
||||
/*exclude_slash_tmp*/ false,
|
||||
)
|
||||
}
|
||||
|
||||
fn unrestricted_file_system_sandbox_policy() -> FileSystemSandboxPolicy {
|
||||
FileSystemSandboxPolicy::unrestricted()
|
||||
}
|
||||
|
||||
fn external_file_system_sandbox_policy() -> FileSystemSandboxPolicy {
|
||||
FileSystemSandboxPolicy::external_sandbox()
|
||||
}
|
||||
|
||||
async fn test_config() -> (TempDir, Config) {
|
||||
let home = TempDir::new().expect("create temp dir");
|
||||
let config = ConfigBuilder::without_managed_config_for_tests()
|
||||
@@ -665,7 +641,6 @@ async fn evaluates_bash_lc_inner_commands() {
|
||||
],
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
permission_profile: PermissionProfile::Disabled,
|
||||
file_system_sandbox_policy: unrestricted_file_system_sandbox_policy(),
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
prefix_rule: None,
|
||||
},
|
||||
@@ -761,7 +736,6 @@ async fn evaluates_heredoc_script_against_prefix_rules() {
|
||||
command,
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
file_system_sandbox_policy: read_only_file_system_sandbox_policy(),
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
prefix_rule: None,
|
||||
},
|
||||
@@ -785,7 +759,6 @@ async fn omits_auto_amendment_for_heredoc_fallback_prompts() {
|
||||
],
|
||||
approval_policy: AskForApproval::UnlessTrusted,
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
file_system_sandbox_policy: read_only_file_system_sandbox_policy(),
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
prefix_rule: None,
|
||||
},
|
||||
@@ -809,7 +782,6 @@ async fn drops_requested_amendment_for_heredoc_fallback_prompts_when_it_wont_mat
|
||||
],
|
||||
approval_policy: AskForApproval::UnlessTrusted,
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
file_system_sandbox_policy: read_only_file_system_sandbox_policy(),
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
prefix_rule: Some(vec![
|
||||
"python3".to_string(),
|
||||
@@ -837,7 +809,6 @@ async fn drops_requested_amendment_for_heredoc_fallback_prompts_when_it_matches(
|
||||
],
|
||||
approval_policy: AskForApproval::UnlessTrusted,
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
file_system_sandbox_policy: read_only_file_system_sandbox_policy(),
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
prefix_rule: Some(vec!["python3".to_string()]),
|
||||
},
|
||||
@@ -862,7 +833,6 @@ async fn heredoc_with_variable_assignment_is_not_reduced_to_allowed_prefix() {
|
||||
],
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
file_system_sandbox_policy: read_only_file_system_sandbox_policy(),
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
prefix_rule: None,
|
||||
},
|
||||
@@ -893,7 +863,6 @@ EOF"#
|
||||
],
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
permission_profile: PermissionProfile::workspace_write(),
|
||||
file_system_sandbox_policy: workspace_write_file_system_sandbox_policy(),
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
prefix_rule: None,
|
||||
},
|
||||
@@ -927,7 +896,6 @@ EOF"#
|
||||
],
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
permission_profile: PermissionProfile::workspace_write(),
|
||||
file_system_sandbox_policy: workspace_write_file_system_sandbox_policy(),
|
||||
sandbox_permissions: SandboxPermissions::RequireEscalated,
|
||||
prefix_rule: None,
|
||||
},
|
||||
@@ -967,7 +935,6 @@ prefix_rule(
|
||||
],
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
permission_profile: PermissionProfile::Disabled,
|
||||
file_system_sandbox_policy: unrestricted_file_system_sandbox_policy(),
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
prefix_rule: None,
|
||||
},
|
||||
@@ -986,7 +953,6 @@ async fn exec_approval_requirement_prefers_execpolicy_match() {
|
||||
command: vec!["rm".to_string()],
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
permission_profile: PermissionProfile::Disabled,
|
||||
file_system_sandbox_policy: unrestricted_file_system_sandbox_policy(),
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
prefix_rule: None,
|
||||
},
|
||||
@@ -1014,7 +980,6 @@ prefix_rule(pattern=["git"], decision="allow")
|
||||
command: vec![git_path, "status".to_string()],
|
||||
approval_policy: AskForApproval::UnlessTrusted,
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
file_system_sandbox_policy: read_only_file_system_sandbox_policy(),
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
prefix_rule: None,
|
||||
},
|
||||
@@ -1048,7 +1013,6 @@ prefix_rule(pattern=["git"], decision="prompt")
|
||||
command: vec![disallowed_git_path.clone(), "status".to_string()],
|
||||
approval_policy: AskForApproval::UnlessTrusted,
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
file_system_sandbox_policy: read_only_file_system_sandbox_policy(),
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
prefix_rule: None,
|
||||
},
|
||||
@@ -1075,7 +1039,6 @@ async fn requested_prefix_rule_can_approve_absolute_path_commands() {
|
||||
],
|
||||
approval_policy: AskForApproval::UnlessTrusted,
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
file_system_sandbox_policy: read_only_file_system_sandbox_policy(),
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
prefix_rule: Some(vec!["cargo".to_string(), "install".to_string()]),
|
||||
},
|
||||
@@ -1098,7 +1061,6 @@ async fn exec_approval_requirement_respects_approval_policy() {
|
||||
command: vec!["rm".to_string()],
|
||||
approval_policy: AskForApproval::Never,
|
||||
permission_profile: PermissionProfile::Disabled,
|
||||
file_system_sandbox_policy: unrestricted_file_system_sandbox_policy(),
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
prefix_rule: None,
|
||||
},
|
||||
@@ -1126,7 +1088,6 @@ fn unmatched_granular_policy_still_prompts_for_restricted_sandbox_escalation() {
|
||||
mcp_elicitations: true,
|
||||
}),
|
||||
permission_profile: &PermissionProfile::read_only(),
|
||||
file_system_sandbox_policy: &read_only_file_system_sandbox_policy(),
|
||||
sandbox_cwd: Path::new("/tmp"),
|
||||
sandbox_permissions: SandboxPermissions::RequireEscalated,
|
||||
used_complex_parsing: false,
|
||||
@@ -1137,9 +1098,8 @@ fn unmatched_granular_policy_still_prompts_for_restricted_sandbox_escalation() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unmatched_on_request_uses_split_filesystem_policy_for_escalation_prompts() {
|
||||
fn unmatched_on_request_uses_permission_profile_file_system_policy_for_escalation_prompts() {
|
||||
let command = vec!["madeup-cmd".to_string()];
|
||||
let restricted_file_system_policy = FileSystemSandboxPolicy::restricted(vec![]);
|
||||
|
||||
assert_eq!(
|
||||
Decision::Prompt,
|
||||
@@ -1147,8 +1107,7 @@ fn unmatched_on_request_uses_split_filesystem_policy_for_escalation_prompts() {
|
||||
&command,
|
||||
UnmatchedCommandContext {
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
permission_profile: &PermissionProfile::Disabled,
|
||||
file_system_sandbox_policy: &restricted_file_system_policy,
|
||||
permission_profile: &PermissionProfile::read_only(),
|
||||
sandbox_cwd: Path::new("/tmp"),
|
||||
sandbox_permissions: SandboxPermissions::RequireEscalated,
|
||||
used_complex_parsing: false,
|
||||
@@ -1169,7 +1128,6 @@ fn known_safe_on_request_still_prompts_for_restricted_sandbox_escalation() {
|
||||
UnmatchedCommandContext {
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
permission_profile: &PermissionProfile::workspace_write(),
|
||||
file_system_sandbox_policy: &workspace_write_file_system_sandbox_policy(),
|
||||
sandbox_cwd: Path::new("/tmp"),
|
||||
sandbox_permissions: SandboxPermissions::RequireEscalated,
|
||||
used_complex_parsing: false,
|
||||
@@ -1202,7 +1160,6 @@ fn managed_cwd_write_profile_is_not_read_only() {
|
||||
|
||||
assert!(!profile_is_managed_read_only(
|
||||
&permission_profile,
|
||||
&file_system_sandbox_policy,
|
||||
Path::new("/tmp/project")
|
||||
));
|
||||
}
|
||||
@@ -1233,7 +1190,6 @@ fn managed_unresolvable_write_profile_is_still_read_only() {
|
||||
|
||||
assert!(profile_is_managed_read_only(
|
||||
&permission_profile,
|
||||
&file_system_sandbox_policy,
|
||||
Path::new("/tmp/project")
|
||||
));
|
||||
}
|
||||
@@ -1250,7 +1206,6 @@ async fn exec_approval_requirement_prompts_for_inline_additional_permissions_und
|
||||
],
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
file_system_sandbox_policy: read_only_file_system_sandbox_policy(),
|
||||
sandbox_permissions: SandboxPermissions::WithAdditionalPermissions,
|
||||
prefix_rule: None,
|
||||
},
|
||||
@@ -1273,7 +1228,6 @@ async fn exec_approval_requirement_prompts_for_known_safe_escalation_under_on_re
|
||||
command: vec!["echo".to_string(), "hello".to_string()],
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
permission_profile: PermissionProfile::workspace_write(),
|
||||
file_system_sandbox_policy: workspace_write_file_system_sandbox_policy(),
|
||||
sandbox_permissions: SandboxPermissions::RequireEscalated,
|
||||
prefix_rule: None,
|
||||
},
|
||||
@@ -1303,7 +1257,6 @@ async fn exec_approval_requirement_rejects_known_safe_escalation_when_granular_s
|
||||
mcp_elicitations: true,
|
||||
}),
|
||||
permission_profile: PermissionProfile::workspace_write(),
|
||||
file_system_sandbox_policy: workspace_write_file_system_sandbox_policy(),
|
||||
sandbox_permissions: SandboxPermissions::RequireEscalated,
|
||||
prefix_rule: None,
|
||||
},
|
||||
@@ -1329,7 +1282,6 @@ async fn exec_approval_requirement_rejects_unmatched_sandbox_escalation_when_gra
|
||||
mcp_elicitations: true,
|
||||
}),
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
file_system_sandbox_policy: read_only_file_system_sandbox_policy(),
|
||||
sandbox_permissions: SandboxPermissions::RequireEscalated,
|
||||
prefix_rule: None,
|
||||
},
|
||||
@@ -1365,7 +1317,6 @@ async fn mixed_rule_and_sandbox_prompt_prioritizes_rule_for_rejection_decision()
|
||||
mcp_elicitations: true,
|
||||
}),
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
file_system_sandbox_policy: &read_only_file_system_sandbox_policy(),
|
||||
sandbox_cwd: Path::new("/tmp"),
|
||||
sandbox_permissions: SandboxPermissions::RequireEscalated,
|
||||
prefix_rule: None,
|
||||
@@ -1403,7 +1354,6 @@ async fn mixed_rule_and_sandbox_prompt_rejects_when_granular_rules_are_disabled(
|
||||
mcp_elicitations: true,
|
||||
}),
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
file_system_sandbox_policy: &read_only_file_system_sandbox_policy(),
|
||||
sandbox_cwd: Path::new("/tmp"),
|
||||
sandbox_permissions: SandboxPermissions::RequireEscalated,
|
||||
prefix_rule: None,
|
||||
@@ -1428,7 +1378,6 @@ async fn exec_approval_requirement_falls_back_to_heuristics() {
|
||||
command: &command,
|
||||
approval_policy: AskForApproval::UnlessTrusted,
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
file_system_sandbox_policy: &read_only_file_system_sandbox_policy(),
|
||||
sandbox_cwd: Path::new("/tmp"),
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
prefix_rule: None,
|
||||
@@ -1454,7 +1403,6 @@ async fn empty_bash_lc_script_falls_back_to_original_command() {
|
||||
command: &command,
|
||||
approval_policy: AskForApproval::UnlessTrusted,
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
file_system_sandbox_policy: &read_only_file_system_sandbox_policy(),
|
||||
sandbox_cwd: Path::new("/tmp"),
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
prefix_rule: None,
|
||||
@@ -1484,7 +1432,6 @@ async fn whitespace_bash_lc_script_falls_back_to_original_command() {
|
||||
command: &command,
|
||||
approval_policy: AskForApproval::UnlessTrusted,
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
file_system_sandbox_policy: &read_only_file_system_sandbox_policy(),
|
||||
sandbox_cwd: Path::new("/tmp"),
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
prefix_rule: None,
|
||||
@@ -1514,7 +1461,6 @@ async fn request_rule_uses_prefix_rule() {
|
||||
command: &command,
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
file_system_sandbox_policy: &read_only_file_system_sandbox_policy(),
|
||||
sandbox_cwd: Path::new("/tmp"),
|
||||
sandbox_permissions: SandboxPermissions::RequireEscalated,
|
||||
prefix_rule: Some(vec!["cargo".to_string(), "install".to_string()]),
|
||||
@@ -1547,7 +1493,6 @@ async fn request_rule_falls_back_when_prefix_rule_does_not_approve_all_commands(
|
||||
command: &command,
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
permission_profile: PermissionProfile::Disabled,
|
||||
file_system_sandbox_policy: &unrestricted_file_system_sandbox_policy(),
|
||||
sandbox_cwd: Path::new("/tmp"),
|
||||
sandbox_permissions: SandboxPermissions::RequireEscalated,
|
||||
prefix_rule: Some(vec!["cargo".to_string(), "install".to_string()]),
|
||||
@@ -1587,7 +1532,6 @@ async fn heuristics_apply_when_other_commands_match_policy() {
|
||||
command: &command,
|
||||
approval_policy: AskForApproval::UnlessTrusted,
|
||||
permission_profile: PermissionProfile::Disabled,
|
||||
file_system_sandbox_policy: &unrestricted_file_system_sandbox_policy(),
|
||||
sandbox_cwd: Path::new("/tmp"),
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
prefix_rule: None,
|
||||
@@ -1663,7 +1607,6 @@ async fn proposed_execpolicy_amendment_is_present_for_single_command_without_pol
|
||||
command: command.clone(),
|
||||
approval_policy: AskForApproval::UnlessTrusted,
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
file_system_sandbox_policy: read_only_file_system_sandbox_policy(),
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
prefix_rule: None,
|
||||
},
|
||||
@@ -1683,7 +1626,6 @@ async fn proposed_execpolicy_amendment_is_omitted_when_policy_prompts() {
|
||||
command: vec!["rm".to_string()],
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
permission_profile: PermissionProfile::Disabled,
|
||||
file_system_sandbox_policy: unrestricted_file_system_sandbox_policy(),
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
prefix_rule: None,
|
||||
},
|
||||
@@ -1707,7 +1649,6 @@ async fn proposed_execpolicy_amendment_is_present_for_multi_command_scripts() {
|
||||
],
|
||||
approval_policy: AskForApproval::UnlessTrusted,
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
file_system_sandbox_policy: read_only_file_system_sandbox_policy(),
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
prefix_rule: None,
|
||||
},
|
||||
@@ -1737,7 +1678,6 @@ async fn proposed_execpolicy_amendment_uses_first_no_match_in_multi_command_scri
|
||||
command,
|
||||
approval_policy: AskForApproval::UnlessTrusted,
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
file_system_sandbox_policy: read_only_file_system_sandbox_policy(),
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
prefix_rule: None,
|
||||
},
|
||||
@@ -1761,7 +1701,6 @@ async fn proposed_execpolicy_amendment_is_present_when_heuristics_allow() {
|
||||
command: command.clone(),
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
file_system_sandbox_policy: read_only_file_system_sandbox_policy(),
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
prefix_rule: None,
|
||||
},
|
||||
@@ -1785,7 +1724,6 @@ async fn proposed_execpolicy_amendment_is_suppressed_when_policy_matches_allow()
|
||||
],
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
file_system_sandbox_policy: read_only_file_system_sandbox_policy(),
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
prefix_rule: None,
|
||||
},
|
||||
@@ -1816,7 +1754,6 @@ prefix_rule(pattern=["cat"], decision="allow")
|
||||
command: command.clone(),
|
||||
approval_policy,
|
||||
permission_profile: PermissionProfile::workspace_write(),
|
||||
file_system_sandbox_policy: workspace_write_file_system_sandbox_policy(),
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
prefix_rule: None,
|
||||
},
|
||||
@@ -1848,7 +1785,6 @@ prefix_rule(pattern=["bash"], decision="allow")
|
||||
],
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
file_system_sandbox_policy: read_only_file_system_sandbox_policy(),
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
prefix_rule: None,
|
||||
},
|
||||
@@ -2014,7 +1950,6 @@ async fn dangerous_rm_rf_requires_approval_in_danger_full_access() {
|
||||
command: command.clone(),
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
permission_profile: PermissionProfile::Disabled,
|
||||
file_system_sandbox_policy: unrestricted_file_system_sandbox_policy(),
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
prefix_rule: None,
|
||||
},
|
||||
@@ -2078,7 +2013,6 @@ async fn verify_approval_requirement_for_unsafe_powershell_command() {
|
||||
command: &sneaky_command,
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
file_system_sandbox_policy: &read_only_file_system_sandbox_policy(),
|
||||
sandbox_cwd: Path::new("/tmp"),
|
||||
sandbox_permissions: permissions,
|
||||
prefix_rule: None,
|
||||
@@ -2103,7 +2037,6 @@ async fn verify_approval_requirement_for_unsafe_powershell_command() {
|
||||
command: &dangerous_command,
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
file_system_sandbox_policy: &read_only_file_system_sandbox_policy(),
|
||||
sandbox_cwd: Path::new("/tmp"),
|
||||
sandbox_permissions: permissions,
|
||||
prefix_rule: None,
|
||||
@@ -2124,7 +2057,6 @@ async fn verify_approval_requirement_for_unsafe_powershell_command() {
|
||||
command: &dangerous_command,
|
||||
approval_policy: AskForApproval::Never,
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
file_system_sandbox_policy: &read_only_file_system_sandbox_policy(),
|
||||
sandbox_cwd: Path::new("/tmp"),
|
||||
sandbox_permissions: permissions,
|
||||
prefix_rule: None,
|
||||
@@ -2146,7 +2078,6 @@ async fn dangerous_command_allowed_when_sandbox_is_explicitly_disabled() {
|
||||
permission_profile: PermissionProfile::External {
|
||||
network: NetworkSandboxPolicy::Restricted,
|
||||
},
|
||||
file_system_sandbox_policy: external_file_system_sandbox_policy(),
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
prefix_rule: None,
|
||||
},
|
||||
@@ -2171,7 +2102,6 @@ async fn dangerous_command_forbidden_in_external_sandbox_when_policy_matches() {
|
||||
permission_profile: PermissionProfile::External {
|
||||
network: NetworkSandboxPolicy::Restricted,
|
||||
},
|
||||
file_system_sandbox_policy: external_file_system_sandbox_policy(),
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
prefix_rule: None,
|
||||
},
|
||||
@@ -2188,7 +2118,6 @@ struct ExecApprovalRequirementScenario {
|
||||
command: Vec<String>,
|
||||
approval_policy: AskForApproval,
|
||||
permission_profile: PermissionProfile,
|
||||
file_system_sandbox_policy: FileSystemSandboxPolicy,
|
||||
sandbox_permissions: SandboxPermissions,
|
||||
prefix_rule: Option<Vec<String>>,
|
||||
}
|
||||
@@ -2212,7 +2141,6 @@ async fn exec_approval_requirement_for_command(
|
||||
command,
|
||||
approval_policy,
|
||||
permission_profile,
|
||||
file_system_sandbox_policy,
|
||||
sandbox_permissions,
|
||||
prefix_rule,
|
||||
} = test;
|
||||
@@ -2224,7 +2152,6 @@ async fn exec_approval_requirement_for_command(
|
||||
command: &command,
|
||||
approval_policy,
|
||||
permission_profile,
|
||||
file_system_sandbox_policy: &file_system_sandbox_policy,
|
||||
sandbox_cwd: Path::new("/tmp"),
|
||||
sandbox_permissions,
|
||||
prefix_rule,
|
||||
|
||||
@@ -15,7 +15,6 @@ async fn evaluates_powershell_inner_commands_against_prompt_rules() {
|
||||
],
|
||||
approval_policy: AskForApproval::Never,
|
||||
permission_profile: PermissionProfile::Disabled,
|
||||
file_system_sandbox_policy: unrestricted_file_system_sandbox_policy(),
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
prefix_rule: None,
|
||||
},
|
||||
@@ -39,7 +38,6 @@ async fn evaluates_powershell_inner_commands_against_allow_rules() {
|
||||
],
|
||||
approval_policy: AskForApproval::UnlessTrusted,
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
file_system_sandbox_policy: read_only_file_system_sandbox_policy(),
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
prefix_rule: None,
|
||||
},
|
||||
@@ -81,7 +79,6 @@ fn unmatched_safe_powershell_words_are_allowed() {
|
||||
UnmatchedCommandContext {
|
||||
approval_policy: AskForApproval::UnlessTrusted,
|
||||
permission_profile: &PermissionProfile::read_only(),
|
||||
file_system_sandbox_policy: &read_only_file_system_sandbox_policy(),
|
||||
sandbox_cwd: Path::new("/tmp"),
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
used_complex_parsing: false,
|
||||
@@ -110,7 +107,6 @@ async fn unmatched_dangerous_powershell_inner_commands_require_approval() {
|
||||
],
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
permission_profile: PermissionProfile::Disabled,
|
||||
file_system_sandbox_policy: unrestricted_file_system_sandbox_policy(),
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
prefix_rule: None,
|
||||
},
|
||||
|
||||
@@ -83,7 +83,6 @@ mod sandbox_tags;
|
||||
pub mod sandboxing;
|
||||
mod session_prefix;
|
||||
mod session_startup_prewarm;
|
||||
mod shell_detect;
|
||||
pub mod skills;
|
||||
pub(crate) use skills::SkillInjections;
|
||||
pub(crate) use skills::SkillLoadOutcome;
|
||||
|
||||
@@ -10868,7 +10868,6 @@ async fn rejects_escalated_permissions_when_policy_not_on_request() {
|
||||
let turn_context_mut = Arc::get_mut(&mut turn_context).expect("unique thread settings Arc");
|
||||
turn_context_mut.permission_profile = PermissionProfile::Disabled;
|
||||
|
||||
let file_system_sandbox_policy = turn_context.file_system_sandbox_policy();
|
||||
let command = session.user_shell().derive_exec_args(
|
||||
command_script,
|
||||
turn_context.config.permissions.allow_login_shell,
|
||||
@@ -10880,7 +10879,6 @@ async fn rejects_escalated_permissions_when_policy_not_on_request() {
|
||||
command: &command,
|
||||
approval_policy: turn_context.approval_policy.value(),
|
||||
permission_profile: turn_context.permission_profile(),
|
||||
file_system_sandbox_policy: &file_system_sandbox_policy,
|
||||
#[allow(deprecated)]
|
||||
sandbox_cwd: turn_context.cwd.as_path(),
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
|
||||
+16
-312
@@ -1,19 +1,12 @@
|
||||
use crate::shell_detect::detect_shell_type;
|
||||
use crate::shell_snapshot::ShellSnapshot;
|
||||
use codex_shell_command::shell_detect::DetectedShell;
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
use std::path::PathBuf;
|
||||
use std::sync::Arc;
|
||||
use tokio::sync::watch;
|
||||
|
||||
#[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)]
|
||||
pub enum ShellType {
|
||||
Zsh,
|
||||
Bash,
|
||||
PowerShell,
|
||||
Sh,
|
||||
Cmd,
|
||||
}
|
||||
pub use codex_shell_command::shell_detect::ShellType;
|
||||
|
||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||
pub struct Shell {
|
||||
@@ -29,13 +22,7 @@ pub struct Shell {
|
||||
|
||||
impl Shell {
|
||||
pub fn name(&self) -> &'static str {
|
||||
match self.shell_type {
|
||||
ShellType::Zsh => "zsh",
|
||||
ShellType::Bash => "bash",
|
||||
ShellType::PowerShell => "powershell",
|
||||
ShellType::Sh => "sh",
|
||||
ShellType::Cmd => "cmd",
|
||||
}
|
||||
self.shell_type.name()
|
||||
}
|
||||
|
||||
/// Takes a string of shell and returns the full list of command args to
|
||||
@@ -88,319 +75,36 @@ impl PartialEq for Shell {
|
||||
|
||||
impl Eq for Shell {}
|
||||
|
||||
#[cfg(unix)]
|
||||
fn get_user_shell_path() -> Option<PathBuf> {
|
||||
let uid = unsafe { libc::getuid() };
|
||||
use std::ffi::CStr;
|
||||
use std::mem::MaybeUninit;
|
||||
use std::ptr;
|
||||
|
||||
let mut passwd = MaybeUninit::<libc::passwd>::uninit();
|
||||
|
||||
// We cannot use getpwuid here: it returns pointers into libc-managed
|
||||
// storage, which is not safe to read concurrently on all targets (the musl
|
||||
// static build used by the CLI can segfault when parallel callers race on
|
||||
// that buffer). getpwuid_r keeps the passwd data in caller-owned memory.
|
||||
let suggested_buffer_len = unsafe { libc::sysconf(libc::_SC_GETPW_R_SIZE_MAX) };
|
||||
let buffer_len = usize::try_from(suggested_buffer_len)
|
||||
.ok()
|
||||
.filter(|len| *len > 0)
|
||||
.unwrap_or(1024);
|
||||
let mut buffer = vec![0; buffer_len];
|
||||
|
||||
loop {
|
||||
let mut result = ptr::null_mut();
|
||||
let status = unsafe {
|
||||
libc::getpwuid_r(
|
||||
uid,
|
||||
passwd.as_mut_ptr(),
|
||||
buffer.as_mut_ptr().cast(),
|
||||
buffer.len(),
|
||||
&mut result,
|
||||
)
|
||||
};
|
||||
|
||||
if status == 0 {
|
||||
if result.is_null() {
|
||||
return None;
|
||||
}
|
||||
|
||||
let passwd = unsafe { passwd.assume_init_ref() };
|
||||
if passwd.pw_shell.is_null() {
|
||||
return None;
|
||||
}
|
||||
|
||||
let shell_path = unsafe { CStr::from_ptr(passwd.pw_shell) }
|
||||
.to_string_lossy()
|
||||
.into_owned();
|
||||
return Some(PathBuf::from(shell_path));
|
||||
}
|
||||
|
||||
if status != libc::ERANGE {
|
||||
return None;
|
||||
}
|
||||
|
||||
// Retry with a larger buffer until libc can materialize the passwd entry.
|
||||
let new_len = buffer.len().checked_mul(2)?;
|
||||
if new_len > 1024 * 1024 {
|
||||
return None;
|
||||
}
|
||||
buffer.resize(new_len, 0);
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(not(unix))]
|
||||
fn get_user_shell_path() -> Option<PathBuf> {
|
||||
None
|
||||
}
|
||||
|
||||
fn file_exists(path: &PathBuf) -> Option<PathBuf> {
|
||||
if std::fs::metadata(path).is_ok_and(|metadata| metadata.is_file()) {
|
||||
Some(PathBuf::from(path))
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
|
||||
fn get_shell_path(
|
||||
shell_type: ShellType,
|
||||
provided_path: Option<&PathBuf>,
|
||||
binary_name: &str,
|
||||
fallback_paths: &[&str],
|
||||
) -> Option<PathBuf> {
|
||||
// If exact provided path exists, use it
|
||||
if provided_path.and_then(file_exists).is_some() {
|
||||
return provided_path.cloned();
|
||||
}
|
||||
|
||||
// Check if the shell we are trying to load is user's default shell
|
||||
// if just use it
|
||||
let default_shell_path = get_user_shell_path();
|
||||
if let Some(default_shell_path) = default_shell_path
|
||||
&& detect_shell_type(&default_shell_path) == Some(shell_type)
|
||||
&& file_exists(&default_shell_path).is_some()
|
||||
{
|
||||
return Some(default_shell_path);
|
||||
}
|
||||
|
||||
if let Ok(path) = which::which(binary_name) {
|
||||
return Some(path);
|
||||
}
|
||||
|
||||
for path in fallback_paths {
|
||||
//check exists
|
||||
if let Some(path) = file_exists(&PathBuf::from(path)) {
|
||||
return Some(path);
|
||||
impl From<DetectedShell> for Shell {
|
||||
fn from(detected: DetectedShell) -> Self {
|
||||
Self {
|
||||
shell_type: detected.shell_type,
|
||||
shell_path: detected.shell_path,
|
||||
shell_snapshot: empty_shell_snapshot_receiver(),
|
||||
}
|
||||
}
|
||||
|
||||
None
|
||||
}
|
||||
|
||||
const ZSH_FALLBACK_PATHS: &[&str] = &["/bin/zsh"];
|
||||
|
||||
fn get_zsh_shell(path: Option<&PathBuf>) -> Option<Shell> {
|
||||
let shell_path = get_shell_path(ShellType::Zsh, path, "zsh", ZSH_FALLBACK_PATHS);
|
||||
|
||||
shell_path.map(|shell_path| Shell {
|
||||
shell_type: ShellType::Zsh,
|
||||
shell_path,
|
||||
shell_snapshot: empty_shell_snapshot_receiver(),
|
||||
})
|
||||
}
|
||||
|
||||
const BASH_FALLBACK_PATHS: &[&str] = &["/bin/bash"];
|
||||
|
||||
fn get_bash_shell(path: Option<&PathBuf>) -> Option<Shell> {
|
||||
let shell_path = get_shell_path(ShellType::Bash, path, "bash", BASH_FALLBACK_PATHS);
|
||||
|
||||
shell_path.map(|shell_path| Shell {
|
||||
shell_type: ShellType::Bash,
|
||||
shell_path,
|
||||
shell_snapshot: empty_shell_snapshot_receiver(),
|
||||
})
|
||||
}
|
||||
|
||||
const SH_FALLBACK_PATHS: &[&str] = &["/bin/sh"];
|
||||
|
||||
fn get_sh_shell(path: Option<&PathBuf>) -> Option<Shell> {
|
||||
let shell_path = get_shell_path(ShellType::Sh, path, "sh", SH_FALLBACK_PATHS);
|
||||
|
||||
shell_path.map(|shell_path| Shell {
|
||||
shell_type: ShellType::Sh,
|
||||
shell_path,
|
||||
shell_snapshot: empty_shell_snapshot_receiver(),
|
||||
})
|
||||
}
|
||||
|
||||
// Note the `pwsh` and `powershell` fallback paths are where the respective
|
||||
// shells are commonly installed on GitHub Actions Windows runners, but may not
|
||||
// be present on all Windows machines:
|
||||
// https://docs.github.com/en/actions/tutorials/build-and-test-code/powershell
|
||||
|
||||
#[cfg(windows)]
|
||||
const PWSH_FALLBACK_PATHS: &[&str] = &[r#"C:\Program Files\PowerShell\7\pwsh.exe"#];
|
||||
#[cfg(not(windows))]
|
||||
const PWSH_FALLBACK_PATHS: &[&str] = &["/usr/local/bin/pwsh"];
|
||||
|
||||
#[cfg(windows)]
|
||||
const POWERSHELL_FALLBACK_PATHS: &[&str] =
|
||||
&[r#"C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe"#];
|
||||
#[cfg(not(windows))]
|
||||
const POWERSHELL_FALLBACK_PATHS: &[&str] = &[];
|
||||
|
||||
fn get_powershell_shell(path: Option<&PathBuf>) -> Option<Shell> {
|
||||
let shell_path = get_shell_path(ShellType::PowerShell, path, "pwsh", PWSH_FALLBACK_PATHS)
|
||||
.or_else(|| {
|
||||
get_shell_path(
|
||||
ShellType::PowerShell,
|
||||
path,
|
||||
"powershell",
|
||||
POWERSHELL_FALLBACK_PATHS,
|
||||
)
|
||||
});
|
||||
|
||||
shell_path.map(|shell_path| Shell {
|
||||
shell_type: ShellType::PowerShell,
|
||||
shell_path,
|
||||
shell_snapshot: empty_shell_snapshot_receiver(),
|
||||
})
|
||||
}
|
||||
|
||||
fn get_cmd_shell(path: Option<&PathBuf>) -> Option<Shell> {
|
||||
let shell_path = get_shell_path(ShellType::Cmd, path, "cmd", &[]);
|
||||
|
||||
shell_path.map(|shell_path| Shell {
|
||||
shell_type: ShellType::Cmd,
|
||||
shell_path,
|
||||
shell_snapshot: empty_shell_snapshot_receiver(),
|
||||
})
|
||||
}
|
||||
|
||||
#[cfg(all(test, unix))]
|
||||
fn ultimate_fallback_shell() -> Shell {
|
||||
if cfg!(windows) {
|
||||
Shell {
|
||||
shell_type: ShellType::Cmd,
|
||||
shell_path: PathBuf::from("cmd.exe"),
|
||||
shell_snapshot: empty_shell_snapshot_receiver(),
|
||||
}
|
||||
} else {
|
||||
Shell {
|
||||
shell_type: ShellType::Sh,
|
||||
shell_path: PathBuf::from("/bin/sh"),
|
||||
shell_snapshot: empty_shell_snapshot_receiver(),
|
||||
}
|
||||
}
|
||||
codex_shell_command::shell_detect::ultimate_fallback_shell().into()
|
||||
}
|
||||
|
||||
pub fn get_shell_by_model_provided_path(shell_path: &PathBuf) -> Shell {
|
||||
detect_shell_type(shell_path)
|
||||
.and_then(|shell_type| get_shell(shell_type, Some(shell_path)))
|
||||
.unwrap_or(ultimate_fallback_shell())
|
||||
codex_shell_command::shell_detect::get_shell_by_model_provided_path(shell_path).into()
|
||||
}
|
||||
|
||||
pub fn get_shell(shell_type: ShellType, path: Option<&PathBuf>) -> Option<Shell> {
|
||||
match shell_type {
|
||||
ShellType::Zsh => get_zsh_shell(path),
|
||||
ShellType::Bash => get_bash_shell(path),
|
||||
ShellType::PowerShell => get_powershell_shell(path),
|
||||
ShellType::Sh => get_sh_shell(path),
|
||||
ShellType::Cmd => get_cmd_shell(path),
|
||||
}
|
||||
codex_shell_command::shell_detect::get_shell(shell_type, path).map(Into::into)
|
||||
}
|
||||
|
||||
pub fn default_user_shell() -> Shell {
|
||||
default_user_shell_from_path(get_user_shell_path())
|
||||
codex_shell_command::shell_detect::default_user_shell().into()
|
||||
}
|
||||
|
||||
#[cfg(all(test, target_os = "macos"))]
|
||||
fn default_user_shell_from_path(user_shell_path: Option<PathBuf>) -> Shell {
|
||||
if cfg!(windows) {
|
||||
get_shell(ShellType::PowerShell, /*path*/ None).unwrap_or(ultimate_fallback_shell())
|
||||
} else {
|
||||
let user_default_shell = user_shell_path
|
||||
.and_then(|shell| detect_shell_type(&shell))
|
||||
.and_then(|shell_type| get_shell(shell_type, /*path*/ None));
|
||||
|
||||
let shell_with_fallback = if cfg!(target_os = "macos") {
|
||||
user_default_shell
|
||||
.or_else(|| get_shell(ShellType::Zsh, /*path*/ None))
|
||||
.or_else(|| get_shell(ShellType::Bash, /*path*/ None))
|
||||
} else {
|
||||
user_default_shell
|
||||
.or_else(|| get_shell(ShellType::Bash, /*path*/ None))
|
||||
.or_else(|| get_shell(ShellType::Zsh, /*path*/ None))
|
||||
};
|
||||
|
||||
shell_with_fallback.unwrap_or(ultimate_fallback_shell())
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod detect_shell_type_tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn test_detect_shell_type() {
|
||||
assert_eq!(
|
||||
detect_shell_type(&PathBuf::from("zsh")),
|
||||
Some(ShellType::Zsh)
|
||||
);
|
||||
assert_eq!(
|
||||
detect_shell_type(&PathBuf::from("bash")),
|
||||
Some(ShellType::Bash)
|
||||
);
|
||||
assert_eq!(
|
||||
detect_shell_type(&PathBuf::from("pwsh")),
|
||||
Some(ShellType::PowerShell)
|
||||
);
|
||||
assert_eq!(
|
||||
detect_shell_type(&PathBuf::from("powershell")),
|
||||
Some(ShellType::PowerShell)
|
||||
);
|
||||
assert_eq!(detect_shell_type(&PathBuf::from("fish")), None);
|
||||
assert_eq!(detect_shell_type(&PathBuf::from("other")), None);
|
||||
assert_eq!(
|
||||
detect_shell_type(&PathBuf::from("/bin/zsh")),
|
||||
Some(ShellType::Zsh)
|
||||
);
|
||||
assert_eq!(
|
||||
detect_shell_type(&PathBuf::from("/bin/bash")),
|
||||
Some(ShellType::Bash)
|
||||
);
|
||||
assert_eq!(
|
||||
detect_shell_type(&PathBuf::from("powershell.exe")),
|
||||
Some(ShellType::PowerShell)
|
||||
);
|
||||
assert_eq!(
|
||||
detect_shell_type(&PathBuf::from(if cfg!(windows) {
|
||||
"C:\\windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe"
|
||||
} else {
|
||||
"/usr/local/bin/pwsh"
|
||||
})),
|
||||
Some(ShellType::PowerShell)
|
||||
);
|
||||
assert_eq!(
|
||||
detect_shell_type(&PathBuf::from("pwsh.exe")),
|
||||
Some(ShellType::PowerShell)
|
||||
);
|
||||
assert_eq!(
|
||||
detect_shell_type(&PathBuf::from("/usr/local/bin/pwsh")),
|
||||
Some(ShellType::PowerShell)
|
||||
);
|
||||
assert_eq!(
|
||||
detect_shell_type(&PathBuf::from("/bin/sh")),
|
||||
Some(ShellType::Sh)
|
||||
);
|
||||
assert_eq!(detect_shell_type(&PathBuf::from("sh")), Some(ShellType::Sh));
|
||||
assert_eq!(
|
||||
detect_shell_type(&PathBuf::from("cmd")),
|
||||
Some(ShellType::Cmd)
|
||||
);
|
||||
assert_eq!(
|
||||
detect_shell_type(&PathBuf::from("cmd.exe")),
|
||||
Some(ShellType::Cmd)
|
||||
);
|
||||
}
|
||||
codex_shell_command::shell_detect::default_user_shell_from_path(user_shell_path).into()
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
|
||||
@@ -1,24 +0,0 @@
|
||||
use crate::shell::ShellType;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
|
||||
pub(crate) fn detect_shell_type(shell_path: &PathBuf) -> Option<ShellType> {
|
||||
match shell_path.as_os_str().to_str() {
|
||||
Some("zsh") => Some(ShellType::Zsh),
|
||||
Some("sh") => Some(ShellType::Sh),
|
||||
Some("cmd") => Some(ShellType::Cmd),
|
||||
Some("bash") => Some(ShellType::Bash),
|
||||
Some("pwsh") => Some(ShellType::PowerShell),
|
||||
Some("powershell") => Some(ShellType::PowerShell),
|
||||
_ => {
|
||||
let shell_name = shell_path.file_stem();
|
||||
if let Some(shell_name) = shell_name {
|
||||
let shell_name_path = Path::new(shell_name);
|
||||
if shell_name_path != Path::new(shell_path) {
|
||||
return detect_shell_type(&shell_name_path.to_path_buf());
|
||||
}
|
||||
}
|
||||
None
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -151,9 +151,7 @@ impl ShellSnapshot {
|
||||
});
|
||||
|
||||
// Make the new snapshot.
|
||||
if let Err(err) =
|
||||
write_shell_snapshot(shell.shell_type.clone(), &temp_path, session_cwd).await
|
||||
{
|
||||
if let Err(err) = write_shell_snapshot(shell.shell_type, &temp_path, session_cwd).await {
|
||||
tracing::warn!(
|
||||
"Failed to create shell snapshot for {}: {err:?}",
|
||||
shell.name()
|
||||
@@ -203,7 +201,7 @@ async fn write_shell_snapshot(
|
||||
if shell_type == ShellType::PowerShell || shell_type == ShellType::Cmd {
|
||||
bail!("Shell snapshot not supported yet for {shell_type:?}");
|
||||
}
|
||||
let shell = get_shell(shell_type.clone(), /*path*/ None)
|
||||
let shell = get_shell(shell_type, /*path*/ None)
|
||||
.with_context(|| format!("No available shell for {shell_type:?}"))?;
|
||||
|
||||
let raw_snapshot = capture_snapshot(&shell, cwd).await?;
|
||||
@@ -225,7 +223,7 @@ async fn write_shell_snapshot(
|
||||
}
|
||||
|
||||
async fn capture_snapshot(shell: &Shell, cwd: &AbsolutePathBuf) -> Result<String> {
|
||||
let shell_type = shell.shell_type.clone();
|
||||
let shell_type = shell.shell_type;
|
||||
match shell_type {
|
||||
ShellType::Zsh => run_shell_script(shell, &zsh_snapshot_script(), cwd).await,
|
||||
ShellType::Bash => run_shell_script(shell, &bash_snapshot_script(), cwd).await,
|
||||
|
||||
@@ -160,7 +160,6 @@ async fn run_exec_like(args: RunExecLikeArgs) -> Result<FunctionToolOutput, Func
|
||||
);
|
||||
emitter.begin(event_ctx).await;
|
||||
|
||||
let file_system_sandbox_policy = turn.file_system_sandbox_policy();
|
||||
let exec_approval_requirement = session
|
||||
.services
|
||||
.exec_policy
|
||||
@@ -168,7 +167,6 @@ async fn run_exec_like(args: RunExecLikeArgs) -> Result<FunctionToolOutput, Func
|
||||
command: &exec_params.command,
|
||||
approval_policy: turn.approval_policy.value(),
|
||||
permission_profile: turn.permission_profile(),
|
||||
file_system_sandbox_policy: &file_system_sandbox_policy,
|
||||
#[allow(deprecated)]
|
||||
sandbox_cwd: turn.cwd.as_path(),
|
||||
sandbox_permissions: if effective_additional_permissions.permissions_preapproved {
|
||||
|
||||
@@ -182,7 +182,7 @@ impl ToolExecutor<ToolInvocation> for ShellCommandHandler {
|
||||
session.thread_id,
|
||||
turn.config.permissions.allow_login_shell,
|
||||
)?;
|
||||
let shell_type = Some(session.user_shell().shell_type.clone());
|
||||
let shell_type = Some(session.user_shell().shell_type);
|
||||
run_exec_like(RunExecLikeArgs {
|
||||
tool_name,
|
||||
exec_params,
|
||||
|
||||
@@ -122,7 +122,7 @@ pub(crate) fn get_command(
|
||||
let shell = model_shell.as_ref().unwrap_or(session_shell.as_ref());
|
||||
Ok(ResolvedCommand {
|
||||
command: shell.derive_exec_args(&args.cmd, use_login_shell),
|
||||
shell_type: shell.shell_type.clone(),
|
||||
shell_type: shell.shell_type,
|
||||
})
|
||||
}
|
||||
UnifiedExecShellMode::ZshFork(zsh_fork_config) => {
|
||||
|
||||
@@ -39,6 +39,7 @@ use codex_protocol::protocol::NetworkPolicyRuleAction;
|
||||
use codex_protocol::protocol::ReviewDecision;
|
||||
use codex_sandboxing::SandboxManager;
|
||||
use codex_sandboxing::SandboxType;
|
||||
use std::time::Instant;
|
||||
|
||||
pub(crate) struct ToolOrchestrator {
|
||||
sandbox: SandboxManager,
|
||||
@@ -256,6 +257,7 @@ impl ToolOrchestrator {
|
||||
network_denial_cancellation_token: None,
|
||||
};
|
||||
|
||||
let initial_attempt_start = Instant::now();
|
||||
let (first_result, first_deferred_network_approval) = Self::run_attempt(
|
||||
tool,
|
||||
req,
|
||||
@@ -264,6 +266,7 @@ impl ToolOrchestrator {
|
||||
managed_network_active,
|
||||
)
|
||||
.await;
|
||||
let initial_duration = initial_attempt_start.elapsed();
|
||||
match first_result {
|
||||
Ok(out) => {
|
||||
// We have a successful initial result
|
||||
@@ -284,12 +287,26 @@ impl ToolOrchestrator {
|
||||
None
|
||||
};
|
||||
if network_policy_decision.is_some() && network_approval_context.is_none() {
|
||||
otel.sandbox_outcome(
|
||||
&otel_tn,
|
||||
otel_ci,
|
||||
"denied",
|
||||
initial_duration,
|
||||
/*escalated_duration*/ None,
|
||||
);
|
||||
return Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied {
|
||||
output,
|
||||
network_policy_decision,
|
||||
})));
|
||||
}
|
||||
if !tool.escalate_on_failure() {
|
||||
otel.sandbox_outcome(
|
||||
&otel_tn,
|
||||
otel_ci,
|
||||
"denied",
|
||||
initial_duration,
|
||||
/*escalated_duration*/ None,
|
||||
);
|
||||
return Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied {
|
||||
output,
|
||||
network_policy_decision,
|
||||
@@ -312,6 +329,13 @@ impl ToolOrchestrator {
|
||||
ExecApprovalRequirement::NeedsApproval { .. }
|
||||
);
|
||||
if !allow_on_request_network_prompt {
|
||||
otel.sandbox_outcome(
|
||||
&otel_tn,
|
||||
otel_ci,
|
||||
"denied",
|
||||
initial_duration,
|
||||
/*escalated_duration*/ None,
|
||||
);
|
||||
return Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied {
|
||||
output,
|
||||
network_policy_decision,
|
||||
@@ -319,6 +343,13 @@ impl ToolOrchestrator {
|
||||
}
|
||||
}
|
||||
if !unsandboxed_allowed && network_approval_context.is_none() {
|
||||
otel.sandbox_outcome(
|
||||
&otel_tn,
|
||||
otel_ci,
|
||||
"denied",
|
||||
initial_duration,
|
||||
/*escalated_duration*/ None,
|
||||
);
|
||||
return Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied {
|
||||
output,
|
||||
network_policy_decision,
|
||||
@@ -400,15 +431,51 @@ impl ToolOrchestrator {
|
||||
};
|
||||
|
||||
// Second attempt.
|
||||
let escalated_attempt_start = Instant::now();
|
||||
let (retry_result, retry_deferred_network_approval) =
|
||||
Self::run_attempt(tool, req, tool_ctx, &retry_attempt, managed_network_active)
|
||||
.await;
|
||||
retry_result.map(|output| OrchestratorRunResult {
|
||||
output,
|
||||
deferred_network_approval: retry_deferred_network_approval,
|
||||
})
|
||||
let escalated_duration = escalated_attempt_start.elapsed();
|
||||
match retry_result {
|
||||
Ok(output) => {
|
||||
otel.sandbox_outcome(
|
||||
&otel_tn,
|
||||
otel_ci,
|
||||
"escalated",
|
||||
initial_duration,
|
||||
Some(escalated_duration),
|
||||
);
|
||||
Ok(OrchestratorRunResult {
|
||||
output,
|
||||
deferred_network_approval: retry_deferred_network_approval,
|
||||
})
|
||||
}
|
||||
Err(err) => {
|
||||
if let Some(outcome) = sandbox_outcome_from_tool_error(&err) {
|
||||
otel.sandbox_outcome(
|
||||
&otel_tn,
|
||||
otel_ci,
|
||||
outcome,
|
||||
initial_duration,
|
||||
Some(escalated_duration),
|
||||
);
|
||||
}
|
||||
Err(err)
|
||||
}
|
||||
}
|
||||
}
|
||||
Err(err) => {
|
||||
if let Some(outcome) = sandbox_outcome_from_tool_error(&err) {
|
||||
otel.sandbox_outcome(
|
||||
&otel_tn,
|
||||
otel_ci,
|
||||
outcome,
|
||||
initial_duration,
|
||||
/*escalated_duration*/ None,
|
||||
);
|
||||
}
|
||||
Err(err)
|
||||
}
|
||||
Err(err) => Err(err),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -509,6 +576,15 @@ impl ToolOrchestrator {
|
||||
}
|
||||
}
|
||||
|
||||
fn sandbox_outcome_from_tool_error(err: &ToolError) -> Option<&'static str> {
|
||||
match err {
|
||||
ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied { .. })) => Some("denied"),
|
||||
ToolError::Codex(CodexErr::Sandbox(SandboxErr::Timeout { .. })) => Some("timed_out"),
|
||||
ToolError::Codex(CodexErr::Sandbox(SandboxErr::Signal(_))) => Some("signal"),
|
||||
ToolError::Rejected(_) | ToolError::Codex(_) => None,
|
||||
}
|
||||
}
|
||||
|
||||
fn build_denial_reason_from_output(_output: &ExecToolCallOutput) -> String {
|
||||
// Keep approval reason terse and stable for UX/tests, but accept the
|
||||
// output so we can evolve heuristics later without touching call sites.
|
||||
|
||||
@@ -622,7 +622,6 @@ impl EscalationPolicy for CoreShellActionProvider {
|
||||
InterceptedExecPolicyContext {
|
||||
approval_policy: self.approval_policy,
|
||||
permission_profile: self.permission_profile.clone(),
|
||||
file_system_sandbox_policy: &self.file_system_sandbox_policy,
|
||||
sandbox_cwd: self.sandbox_policy_cwd.as_path(),
|
||||
sandbox_permissions: self.approval_sandbox_permissions,
|
||||
enable_shell_wrapper_parsing:
|
||||
@@ -677,7 +676,6 @@ fn evaluate_intercepted_exec_policy(
|
||||
let InterceptedExecPolicyContext {
|
||||
approval_policy,
|
||||
permission_profile,
|
||||
file_system_sandbox_policy,
|
||||
sandbox_cwd,
|
||||
sandbox_permissions,
|
||||
enable_shell_wrapper_parsing,
|
||||
@@ -705,7 +703,6 @@ fn evaluate_intercepted_exec_policy(
|
||||
crate::exec_policy::UnmatchedCommandContext {
|
||||
approval_policy,
|
||||
permission_profile: &permission_profile,
|
||||
file_system_sandbox_policy,
|
||||
sandbox_cwd,
|
||||
sandbox_permissions,
|
||||
used_complex_parsing,
|
||||
@@ -727,7 +724,6 @@ fn evaluate_intercepted_exec_policy(
|
||||
struct InterceptedExecPolicyContext<'a> {
|
||||
approval_policy: AskForApproval,
|
||||
permission_profile: PermissionProfile,
|
||||
file_system_sandbox_policy: &'a FileSystemSandboxPolicy,
|
||||
sandbox_cwd: &'a Path,
|
||||
sandbox_permissions: SandboxPermissions,
|
||||
enable_shell_wrapper_parsing: bool,
|
||||
|
||||
@@ -522,7 +522,6 @@ fn evaluate_intercepted_exec_policy_uses_wrapper_command_when_shell_wrapper_pars
|
||||
InterceptedExecPolicyContext {
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
file_system_sandbox_policy: &read_only_file_system_sandbox_policy(),
|
||||
sandbox_cwd: sandbox_cwd.as_path(),
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
enable_shell_wrapper_parsing: enable_intercepted_exec_policy_shell_wrapper_parsing,
|
||||
@@ -575,7 +574,6 @@ fn evaluate_intercepted_exec_policy_matches_inner_shell_commands_when_enabled()
|
||||
InterceptedExecPolicyContext {
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
file_system_sandbox_policy: &read_only_file_system_sandbox_policy(),
|
||||
sandbox_cwd: sandbox_cwd.as_path(),
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
enable_shell_wrapper_parsing: enable_intercepted_exec_policy_shell_wrapper_parsing,
|
||||
@@ -619,7 +617,6 @@ host_executable(name = "git", paths = ["{git_path_literal}"])
|
||||
InterceptedExecPolicyContext {
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
file_system_sandbox_policy: &read_only_file_system_sandbox_policy(),
|
||||
sandbox_cwd: sandbox_cwd.as_path(),
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
enable_shell_wrapper_parsing: false,
|
||||
@@ -747,7 +744,6 @@ fn intercepted_exec_policy_treats_preapproved_additional_permissions_as_default(
|
||||
let argv = ["printf".to_string(), "hello".to_string()];
|
||||
let approval_policy = AskForApproval::OnRequest;
|
||||
let permission_profile = PermissionProfile::workspace_write();
|
||||
let file_system_sandbox_policy = read_only_file_system_sandbox_policy();
|
||||
let sandbox_cwd = test_sandbox_cwd();
|
||||
|
||||
let preapproved = evaluate_intercepted_exec_policy(
|
||||
@@ -757,7 +753,6 @@ fn intercepted_exec_policy_treats_preapproved_additional_permissions_as_default(
|
||||
InterceptedExecPolicyContext {
|
||||
approval_policy,
|
||||
permission_profile: permission_profile.clone(),
|
||||
file_system_sandbox_policy: &file_system_sandbox_policy,
|
||||
sandbox_cwd: sandbox_cwd.as_path(),
|
||||
sandbox_permissions: super::approval_sandbox_permissions(
|
||||
SandboxPermissions::WithAdditionalPermissions,
|
||||
@@ -773,7 +768,6 @@ fn intercepted_exec_policy_treats_preapproved_additional_permissions_as_default(
|
||||
InterceptedExecPolicyContext {
|
||||
approval_policy,
|
||||
permission_profile,
|
||||
file_system_sandbox_policy: &file_system_sandbox_policy,
|
||||
sandbox_cwd: sandbox_cwd.as_path(),
|
||||
sandbox_permissions: SandboxPermissions::WithAdditionalPermissions,
|
||||
enable_shell_wrapper_parsing: false,
|
||||
@@ -808,7 +802,6 @@ host_executable(name = "git", paths = ["{allowed_git_literal}"])
|
||||
InterceptedExecPolicyContext {
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
file_system_sandbox_policy: &read_only_file_system_sandbox_policy(),
|
||||
sandbox_cwd: sandbox_cwd.as_path(),
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
enable_shell_wrapper_parsing: false,
|
||||
|
||||
@@ -1011,7 +1011,6 @@ impl UnifiedExecProcessManager {
|
||||
};
|
||||
let mut orchestrator = ToolOrchestrator::new();
|
||||
let mut runtime = UnifiedExecRuntime::new(self, request.shell_mode.clone());
|
||||
let file_system_sandbox_policy = context.turn.file_system_sandbox_policy();
|
||||
let exec_approval_requirement = context
|
||||
.session
|
||||
.services
|
||||
@@ -1020,7 +1019,6 @@ impl UnifiedExecProcessManager {
|
||||
command: &request.command,
|
||||
approval_policy: context.turn.approval_policy.value(),
|
||||
permission_profile: context.turn.permission_profile(),
|
||||
file_system_sandbox_policy: &file_system_sandbox_policy,
|
||||
// The process cwd may be model-controlled. Policy resolution
|
||||
// stays anchored to the selected turn environment cwd instead.
|
||||
sandbox_cwd: request.sandbox_cwd.as_path(),
|
||||
@@ -1034,7 +1032,7 @@ impl UnifiedExecProcessManager {
|
||||
.await;
|
||||
let req = UnifiedExecToolRequest {
|
||||
command: request.command.clone(),
|
||||
shell_type: request.shell_type.clone(),
|
||||
shell_type: request.shell_type,
|
||||
hook_command: request.hook_command.clone(),
|
||||
process_id: request.process_id,
|
||||
cwd,
|
||||
|
||||
@@ -231,6 +231,7 @@ fn remote_model_with_auto_review_override(slug: &str, review_model: &str) -> Mod
|
||||
input_modalities: default_input_modalities(),
|
||||
used_fallback_model_metadata: false,
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
auto_review_model_override: Some(review_model.to_string()),
|
||||
tool_mode: None,
|
||||
multi_agent_version: None,
|
||||
|
||||
@@ -1872,6 +1872,71 @@ async fn configured_reasoning_summary_is_sent() -> anyhow::Result<()> {
|
||||
.and_then(|value| value.as_str()),
|
||||
Some("concise")
|
||||
);
|
||||
pretty_assertions::assert_eq!(
|
||||
request_body
|
||||
.get("reasoning")
|
||||
.and_then(|reasoning| reasoning.get("context")),
|
||||
None
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn responses_lite_sets_all_turns_context_and_disables_parallel_tool_calls()
|
||||
-> anyhow::Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
let server = MockServer::start().await;
|
||||
|
||||
let resp_mock = mount_sse_once(
|
||||
&server,
|
||||
sse(vec![ev_response_created("resp1"), ev_completed("resp1")]),
|
||||
)
|
||||
.await;
|
||||
|
||||
let mut model_catalog = bundled_models_response()
|
||||
.unwrap_or_else(|err| panic!("bundled models.json should parse: {err}"));
|
||||
let model = model_catalog
|
||||
.models
|
||||
.iter_mut()
|
||||
.find(|model| model.slug == "gpt-5.4")
|
||||
.expect("gpt-5.4 exists in bundled models.json");
|
||||
model.use_responses_lite = true;
|
||||
model.supports_parallel_tool_calls = true;
|
||||
|
||||
let TestCodex { codex, .. } = test_codex()
|
||||
.with_model("gpt-5.4")
|
||||
.with_config(move |config| {
|
||||
config.model_catalog = Some(model_catalog);
|
||||
})
|
||||
.build(&server)
|
||||
.await?;
|
||||
|
||||
codex
|
||||
.submit(Op::UserInput {
|
||||
environments: None,
|
||||
items: vec![UserInput::Text {
|
||||
text: "hello".into(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
final_output_json_schema: None,
|
||||
responsesapi_client_metadata: None,
|
||||
additional_context: Default::default(),
|
||||
thread_settings: Default::default(),
|
||||
})
|
||||
.await?;
|
||||
|
||||
wait_for_event(&codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await;
|
||||
|
||||
let request_body = resp_mock.single_request().body_json();
|
||||
pretty_assertions::assert_eq!(
|
||||
request_body
|
||||
.get("reasoning")
|
||||
.and_then(|reasoning| reasoning.get("context"))
|
||||
.and_then(|value| value.as_str()),
|
||||
Some("all_turns")
|
||||
);
|
||||
pretty_assertions::assert_eq!(request_body.get("parallel_tool_calls"), Some(&json!(false)));
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -375,7 +375,11 @@ async fn code_mode_only_restricts_prompt_tools() -> Result<()> {
|
||||
let first_body = resp_mock.single_request().body_json();
|
||||
assert_eq!(
|
||||
tool_names(&first_body),
|
||||
vec!["exec".to_string(), "wait".to_string()]
|
||||
vec![
|
||||
"exec".to_string(),
|
||||
"wait".to_string(),
|
||||
"web_search".to_string()
|
||||
]
|
||||
);
|
||||
|
||||
Ok(())
|
||||
@@ -457,7 +461,12 @@ if (!tool) {
|
||||
let first_body = resp_mock.single_request().body_json();
|
||||
assert_eq!(
|
||||
tool_names(&first_body),
|
||||
vec!["exec".to_string(), "wait".to_string()]
|
||||
vec![
|
||||
"exec".to_string(),
|
||||
"wait".to_string(),
|
||||
"web_search".to_string(),
|
||||
"image_generation".to_string()
|
||||
]
|
||||
);
|
||||
|
||||
let exec_description = first_body
|
||||
@@ -2898,6 +2907,7 @@ text(JSON.stringify(Object.getOwnPropertyNames(globalThis).sort()));
|
||||
"escape",
|
||||
"exit",
|
||||
"eval",
|
||||
"generatedImage",
|
||||
"globalThis",
|
||||
"image",
|
||||
"isFinite",
|
||||
@@ -2953,7 +2963,7 @@ text(JSON.stringify(tool));
|
||||
parsed,
|
||||
serde_json::json!({
|
||||
"name": "view_image",
|
||||
"description": "View a local image file from the filesystem when visual inspection is needed. Use this for images already available on disk.\n\nexec tool declaration:\n```ts\ndeclare const tools: { view_image(args: {\n // Local filesystem path to an image file\n path: string;\n}): Promise<{\n // Image detail hint returned by view_image. Returns `high` for default resized behavior or `original` when original resolution is preserved.\n detail: \"high\" | \"original\";\n // Data URL for the loaded image.\n image_url: string;\n}>; };\n```",
|
||||
"description": "View a local image file from the filesystem when visual inspection is needed. Use this for images already available on disk.\n\nexec tool declaration:\n```ts\ndeclare const tools: { view_image(args: {\n // Local filesystem path to an image file.\n path: string;\n}): Promise<{\n // Image detail hint returned by view_image. Returns `high` for default resized behavior or `original` when original resolution is preserved.\n detail: \"high\" | \"original\";\n // Data URL for the loaded image.\n image_url: string;\n}>; };\n```",
|
||||
})
|
||||
);
|
||||
|
||||
|
||||
@@ -112,6 +112,7 @@ fn test_model_info(
|
||||
input_modalities,
|
||||
used_fallback_model_metadata: false,
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
auto_review_model_override: None,
|
||||
tool_mode: None,
|
||||
multi_agent_version: None,
|
||||
@@ -932,6 +933,7 @@ async fn model_switch_to_smaller_model_updates_token_context_window() -> Result<
|
||||
input_modalities: default_input_modalities(),
|
||||
used_fallback_model_metadata: false,
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
auto_review_model_override: None,
|
||||
tool_mode: None,
|
||||
multi_agent_version: None,
|
||||
|
||||
@@ -370,6 +370,7 @@ fn test_remote_model(slug: &str, priority: i32) -> ModelInfo {
|
||||
input_modalities: default_input_modalities(),
|
||||
used_fallback_model_metadata: false,
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
auto_review_model_override: None,
|
||||
tool_mode: None,
|
||||
multi_agent_version: None,
|
||||
|
||||
@@ -1,11 +1,15 @@
|
||||
use codex_core::config::Constrained;
|
||||
use codex_features::Feature;
|
||||
use codex_otel::SessionTelemetry;
|
||||
use codex_otel::TelemetryAuthMode;
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::openai_models::ReasoningEffort;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::Op;
|
||||
use codex_protocol::protocol::ReviewDecision;
|
||||
use codex_protocol::protocol::SessionSource;
|
||||
use codex_protocol::user_input::UserInput;
|
||||
use core_test_support::responses::ev_assistant_message;
|
||||
use core_test_support::responses::ev_completed;
|
||||
@@ -27,6 +31,7 @@ use core_test_support::test_codex::TestCodex;
|
||||
use core_test_support::test_codex::test_codex;
|
||||
use core_test_support::wait_for_event;
|
||||
use std::sync::Mutex;
|
||||
use std::time::Duration;
|
||||
use tracing::Level;
|
||||
use tracing_test::traced_test;
|
||||
|
||||
@@ -1145,6 +1150,70 @@ fn tool_decision_assertion<'a>(
|
||||
}
|
||||
}
|
||||
|
||||
fn sandbox_outcome_assertion<'a>(
|
||||
call_id: &'a str,
|
||||
expected_outcome: &'a str,
|
||||
) -> impl Fn(&[&str]) -> Result<(), String> + 'a {
|
||||
let call_id = call_id.to_string();
|
||||
let expected_outcome = expected_outcome.to_string();
|
||||
|
||||
move |lines: &[&str]| {
|
||||
let line = lines
|
||||
.iter()
|
||||
.find(|line| {
|
||||
line.contains("codex.sandbox_outcome")
|
||||
&& line.contains(&format!("call_id={call_id}"))
|
||||
})
|
||||
.ok_or_else(|| format!("missing codex.sandbox_outcome event for {call_id}"))?;
|
||||
|
||||
let lower = line.to_lowercase();
|
||||
if !lower.contains("tool_name=shell_command") {
|
||||
return Err("missing tool_name for shell_command".to_string());
|
||||
}
|
||||
if !lower.contains(&format!("outcome={expected_outcome}")) {
|
||||
return Err(format!("unexpected sandbox outcome for {call_id}"));
|
||||
}
|
||||
if !lower.contains("initial_duration_ms=12") {
|
||||
return Err("missing initial_duration_ms field".to_string());
|
||||
}
|
||||
if !lower.contains("escalated_duration_ms=34") {
|
||||
return Err("missing escalated_duration_ms field".to_string());
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
#[traced_test]
|
||||
fn sandbox_outcome_event_records_outcome() {
|
||||
let telemetry = SessionTelemetry::new(
|
||||
ThreadId::new(),
|
||||
"gpt-5.5",
|
||||
"gpt-5.5",
|
||||
/*account_id*/ None,
|
||||
/*account_email*/ None,
|
||||
Some(TelemetryAuthMode::ApiKey),
|
||||
"Codex_Desktop".to_string(),
|
||||
/*log_user_prompts*/ false,
|
||||
"tty".to_string(),
|
||||
SessionSource::Cli,
|
||||
);
|
||||
|
||||
telemetry.sandbox_outcome(
|
||||
"shell_command",
|
||||
"sandbox-outcome-call",
|
||||
"escalated",
|
||||
Duration::from_millis(/*millis*/ 12),
|
||||
Some(Duration::from_millis(/*millis*/ 34)),
|
||||
);
|
||||
|
||||
logs_assert(sandbox_outcome_assertion(
|
||||
"sandbox-outcome-call",
|
||||
"escalated",
|
||||
));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
#[traced_test]
|
||||
async fn handle_shell_command_autoapprove_from_config_records_tool_decision() {
|
||||
|
||||
@@ -592,6 +592,7 @@ async fn remote_model_friendly_personality_instructions_with_feature() -> anyhow
|
||||
input_modalities: default_input_modalities(),
|
||||
used_fallback_model_metadata: false,
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
auto_review_model_override: None,
|
||||
tool_mode: None,
|
||||
multi_agent_version: None,
|
||||
@@ -705,6 +706,7 @@ async fn user_turn_personality_remote_model_template_includes_update_message() -
|
||||
input_modalities: default_input_modalities(),
|
||||
used_fallback_model_metadata: false,
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
auto_review_model_override: None,
|
||||
tool_mode: None,
|
||||
multi_agent_version: None,
|
||||
|
||||
@@ -478,6 +478,7 @@ async fn remote_models_remote_model_uses_unified_exec() -> Result<()> {
|
||||
input_modalities: default_input_modalities(),
|
||||
used_fallback_model_metadata: false,
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
auto_review_model_override: None,
|
||||
tool_mode: None,
|
||||
multi_agent_version: None,
|
||||
@@ -730,6 +731,7 @@ async fn remote_models_apply_remote_base_instructions() -> Result<()> {
|
||||
input_modalities: default_input_modalities(),
|
||||
used_fallback_model_metadata: false,
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
auto_review_model_override: None,
|
||||
tool_mode: None,
|
||||
multi_agent_version: None,
|
||||
@@ -1216,6 +1218,7 @@ fn test_remote_model_with_policy(
|
||||
input_modalities: default_input_modalities(),
|
||||
used_fallback_model_metadata: false,
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
auto_review_model_override: None,
|
||||
tool_mode: None,
|
||||
multi_agent_version: None,
|
||||
|
||||
@@ -1349,6 +1349,7 @@ async fn stdio_image_responses_are_sanitized_for_text_only_model() -> anyhow::Re
|
||||
input_modalities: vec![InputModality::Text],
|
||||
used_fallback_model_metadata: false,
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
auto_review_model_override: None,
|
||||
tool_mode: None,
|
||||
multi_agent_version: None,
|
||||
|
||||
@@ -60,6 +60,7 @@ fn test_model_info(
|
||||
input_modalities: default_input_modalities(),
|
||||
used_fallback_model_metadata: false,
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
auto_review_model_override: None,
|
||||
tool_mode: None,
|
||||
multi_agent_version: None,
|
||||
|
||||
@@ -1355,6 +1355,7 @@ async fn view_image_tool_returns_unsupported_message_for_text_only_model() -> an
|
||||
input_modalities: vec![InputModality::Text],
|
||||
used_fallback_model_metadata: false,
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
auto_review_model_override: None,
|
||||
tool_mode: None,
|
||||
multi_agent_version: None,
|
||||
|
||||
@@ -81,7 +81,10 @@ GitHub Actions routes Bazel build and output-resolution commands through
|
||||
`.github/scripts/run-bazel-ci.sh` and `.github/scripts/rusty_v8_bazel.py`
|
||||
delegate remote configuration selection to that wrapper. The wrapper reads the
|
||||
GitHub Actions repository and event payload rather than relying on workflow
|
||||
files to duplicate tenant-selection logic.
|
||||
files to duplicate tenant-selection logic. It also normalizes GitHub Actions
|
||||
startup options so all Bazel launches in a job reuse the same server and
|
||||
in-memory analysis cache. Target-discovery and lockfile helpers delegate to the
|
||||
same wrapper so their callers do not need to select CI-specific startup options.
|
||||
|
||||
Loading-phase target-discovery `bazel query` commands run locally because they
|
||||
only enumerate labels and do not need remote caches or execution.
|
||||
|
||||
@@ -22,6 +22,7 @@ codex-client = { workspace = true }
|
||||
codex-file-system = { workspace = true }
|
||||
codex-protocol = { workspace = true }
|
||||
codex-sandboxing = { workspace = true }
|
||||
codex-shell-command = { workspace = true }
|
||||
codex-utils-absolute-path = { workspace = true }
|
||||
codex-utils-pty = { workspace = true }
|
||||
codex-utils-rustls-provider = { workspace = true }
|
||||
|
||||
@@ -29,6 +29,7 @@ use crate::connection::JsonRpcConnection;
|
||||
use crate::process::ExecProcessEvent;
|
||||
use crate::process::ExecProcessEventLog;
|
||||
use crate::process::ExecProcessEventReceiver;
|
||||
use crate::protocol::ENVIRONMENT_INFO_METHOD;
|
||||
use crate::protocol::EXEC_CLOSED_METHOD;
|
||||
use crate::protocol::EXEC_EXITED_METHOD;
|
||||
use crate::protocol::EXEC_METHOD;
|
||||
@@ -36,6 +37,7 @@ use crate::protocol::EXEC_OUTPUT_DELTA_METHOD;
|
||||
use crate::protocol::EXEC_READ_METHOD;
|
||||
use crate::protocol::EXEC_TERMINATE_METHOD;
|
||||
use crate::protocol::EXEC_WRITE_METHOD;
|
||||
use crate::protocol::EnvironmentInfo;
|
||||
use crate::protocol::ExecClosedNotification;
|
||||
use crate::protocol::ExecExitedNotification;
|
||||
use crate::protocol::ExecOutputDeltaNotification;
|
||||
@@ -279,6 +281,12 @@ impl HttpClient for LazyRemoteExecServerClient {
|
||||
}
|
||||
}
|
||||
|
||||
impl LazyRemoteExecServerClient {
|
||||
pub(crate) async fn environment_info(&self) -> Result<EnvironmentInfo, ExecServerError> {
|
||||
self.get().await?.environment_info().await
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, thiserror::Error)]
|
||||
pub enum ExecServerError {
|
||||
#[error("failed to spawn exec-server: {0}")]
|
||||
@@ -363,6 +371,10 @@ impl ExecServerClient {
|
||||
self.call(EXEC_METHOD, ¶ms).await
|
||||
}
|
||||
|
||||
pub async fn environment_info(&self) -> Result<EnvironmentInfo, ExecServerError> {
|
||||
self.call(ENVIRONMENT_INFO_METHOD, &()).await
|
||||
}
|
||||
|
||||
pub async fn read(&self, params: ReadParams) -> Result<ReadResponse, ExecServerError> {
|
||||
self.call(EXEC_READ_METHOD, ¶ms).await
|
||||
}
|
||||
|
||||
@@ -2,6 +2,9 @@ use std::collections::HashMap;
|
||||
use std::sync::Arc;
|
||||
use std::sync::RwLock;
|
||||
|
||||
use futures::FutureExt;
|
||||
use futures::future::BoxFuture;
|
||||
|
||||
use crate::ExecServerError;
|
||||
use crate::ExecServerRuntimePaths;
|
||||
use crate::ExecutorFileSystem;
|
||||
@@ -18,8 +21,11 @@ use crate::environment_toml::environment_provider_from_codex_home;
|
||||
use crate::local_file_system::LocalFileSystem;
|
||||
use crate::local_process::LocalProcess;
|
||||
use crate::process::ExecBackend;
|
||||
use crate::protocol::EnvironmentInfo;
|
||||
use crate::protocol::ShellInfo;
|
||||
use crate::remote_file_system::RemoteFileSystem;
|
||||
use crate::remote_process::RemoteProcess;
|
||||
use codex_shell_command::shell_detect::DetectedShell;
|
||||
|
||||
pub const CODEX_EXEC_SERVER_URL_ENV_VAR: &str = "CODEX_EXEC_SERVER_URL";
|
||||
|
||||
@@ -286,18 +292,49 @@ impl EnvironmentManager {
|
||||
pub struct Environment {
|
||||
exec_server_url: Option<String>,
|
||||
remote_transport: Option<ExecServerTransportParams>,
|
||||
info_provider: Arc<dyn EnvironmentInfoProvider>,
|
||||
exec_backend: Arc<dyn ExecBackend>,
|
||||
filesystem: Arc<dyn ExecutorFileSystem>,
|
||||
http_client: Arc<dyn HttpClient>,
|
||||
local_runtime_paths: Option<ExecServerRuntimePaths>,
|
||||
}
|
||||
|
||||
/// Provides environment metadata from either a local environment or a remote exec-server.
|
||||
trait EnvironmentInfoProvider: Send + Sync {
|
||||
fn info(&self) -> BoxFuture<'_, Result<EnvironmentInfo, ExecServerError>>;
|
||||
}
|
||||
|
||||
struct LocalEnvironmentInfoProvider;
|
||||
|
||||
impl EnvironmentInfoProvider for LocalEnvironmentInfoProvider {
|
||||
fn info(&self) -> BoxFuture<'_, Result<EnvironmentInfo, ExecServerError>> {
|
||||
std::future::ready(Ok(EnvironmentInfo::local())).boxed()
|
||||
}
|
||||
}
|
||||
|
||||
struct RemoteEnvironmentInfoProvider {
|
||||
client: LazyRemoteExecServerClient,
|
||||
}
|
||||
|
||||
impl RemoteEnvironmentInfoProvider {
|
||||
fn new(client: LazyRemoteExecServerClient) -> Self {
|
||||
Self { client }
|
||||
}
|
||||
}
|
||||
|
||||
impl EnvironmentInfoProvider for RemoteEnvironmentInfoProvider {
|
||||
fn info(&self) -> BoxFuture<'_, Result<EnvironmentInfo, ExecServerError>> {
|
||||
async move { self.client.environment_info().await }.boxed()
|
||||
}
|
||||
}
|
||||
|
||||
impl Environment {
|
||||
/// Builds a test-only local environment without configured sandbox helper paths.
|
||||
pub fn default_for_tests() -> Self {
|
||||
Self {
|
||||
exec_server_url: None,
|
||||
remote_transport: None,
|
||||
info_provider: Arc::new(LocalEnvironmentInfoProvider),
|
||||
exec_backend: Arc::new(LocalProcess::default()),
|
||||
filesystem: Arc::new(LocalFileSystem::unsandboxed()),
|
||||
http_client: Arc::new(ReqwestHttpClient),
|
||||
@@ -354,6 +391,7 @@ impl Environment {
|
||||
Self {
|
||||
exec_server_url: None,
|
||||
remote_transport: None,
|
||||
info_provider: Arc::new(LocalEnvironmentInfoProvider),
|
||||
exec_backend: Arc::new(LocalProcess::default()),
|
||||
filesystem: Arc::new(LocalFileSystem::with_runtime_paths(
|
||||
local_runtime_paths.clone(),
|
||||
@@ -392,6 +430,7 @@ impl Environment {
|
||||
Self {
|
||||
exec_server_url,
|
||||
remote_transport: Some(remote_transport),
|
||||
info_provider: Arc::new(RemoteEnvironmentInfoProvider::new(client.clone())),
|
||||
exec_backend,
|
||||
filesystem,
|
||||
http_client: Arc::new(client),
|
||||
@@ -412,6 +451,11 @@ impl Environment {
|
||||
self.local_runtime_paths.as_ref()
|
||||
}
|
||||
|
||||
/// Returns environment information from the selected execution/filesystem environment.
|
||||
pub async fn info(&self) -> Result<EnvironmentInfo, ExecServerError> {
|
||||
self.info_provider.info().await
|
||||
}
|
||||
|
||||
pub fn get_exec_backend(&self) -> Arc<dyn ExecBackend> {
|
||||
Arc::clone(&self.exec_backend)
|
||||
}
|
||||
@@ -425,6 +469,23 @@ impl Environment {
|
||||
}
|
||||
}
|
||||
|
||||
impl EnvironmentInfo {
|
||||
pub(crate) fn local() -> Self {
|
||||
Self {
|
||||
shell: codex_shell_command::shell_detect::default_user_shell().into(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl From<DetectedShell> for ShellInfo {
|
||||
fn from(shell: DetectedShell) -> Self {
|
||||
Self {
|
||||
name: shell.name().to_string(),
|
||||
path: shell.shell_path.to_string_lossy().into_owned(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use std::sync::Arc;
|
||||
@@ -458,6 +519,7 @@ mod tests {
|
||||
|
||||
assert_eq!(environment.exec_server_url(), None);
|
||||
assert!(!environment.is_remote());
|
||||
assert!(environment.info().await.is_ok());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
||||
@@ -57,6 +57,7 @@ pub use process::ExecProcessEvent;
|
||||
pub use process::ExecProcessEventReceiver;
|
||||
pub use process::StartedExecProcess;
|
||||
pub use process_id::ProcessId;
|
||||
pub use protocol::EnvironmentInfo;
|
||||
pub use protocol::ExecClosedNotification;
|
||||
pub use protocol::ExecEnvPolicy;
|
||||
pub use protocol::ExecExitedNotification;
|
||||
@@ -94,6 +95,7 @@ pub use protocol::InitializeResponse;
|
||||
pub use protocol::ProcessOutputChunk;
|
||||
pub use protocol::ReadParams;
|
||||
pub use protocol::ReadResponse;
|
||||
pub use protocol::ShellInfo;
|
||||
pub use protocol::TerminateParams;
|
||||
pub use protocol::TerminateResponse;
|
||||
pub use protocol::WriteParams;
|
||||
|
||||
@@ -19,6 +19,7 @@ pub const EXEC_TERMINATE_METHOD: &str = "process/terminate";
|
||||
pub const EXEC_OUTPUT_DELTA_METHOD: &str = "process/output";
|
||||
pub const EXEC_EXITED_METHOD: &str = "process/exited";
|
||||
pub const EXEC_CLOSED_METHOD: &str = "process/closed";
|
||||
pub const ENVIRONMENT_INFO_METHOD: &str = "environment/info";
|
||||
pub const FS_READ_FILE_METHOD: &str = "fs/readFile";
|
||||
pub const FS_WRITE_FILE_METHOD: &str = "fs/writeFile";
|
||||
pub const FS_CREATE_DIRECTORY_METHOD: &str = "fs/createDirectory";
|
||||
@@ -64,6 +65,23 @@ pub struct InitializeResponse {
|
||||
pub session_id: String,
|
||||
}
|
||||
|
||||
/// Information about an execution/filesystem environment.
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
pub struct EnvironmentInfo {
|
||||
pub shell: ShellInfo,
|
||||
}
|
||||
|
||||
/// Shell detected for an execution/filesystem environment.
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
pub struct ShellInfo {
|
||||
/// Stable shell name, for example `zsh`, `bash`, `powershell`, `sh`, or `cmd`.
|
||||
pub name: String,
|
||||
/// Path the exec server would use for that shell.
|
||||
pub path: String,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
pub struct ExecParams {
|
||||
|
||||
@@ -14,6 +14,7 @@ use tokio_util::task::TaskTracker;
|
||||
use crate::ExecServerRuntimePaths;
|
||||
use crate::client::http_client::PendingReqwestHttpBodyStream;
|
||||
use crate::client::http_client::ReqwestHttpRequestRunner;
|
||||
use crate::protocol::EnvironmentInfo;
|
||||
use crate::protocol::ExecParams;
|
||||
use crate::protocol::ExecResponse;
|
||||
use crate::protocol::FsCanonicalizeParams;
|
||||
@@ -147,6 +148,11 @@ impl ExecServerHandler {
|
||||
session.process().exec(params).await
|
||||
}
|
||||
|
||||
pub(crate) fn environment_info(&self) -> Result<EnvironmentInfo, JSONRPCErrorError> {
|
||||
self.require_initialized_for("environment info")?;
|
||||
Ok(EnvironmentInfo::local())
|
||||
}
|
||||
|
||||
pub(crate) async fn exec_read(
|
||||
&self,
|
||||
params: ReadParams,
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
use std::sync::Arc;
|
||||
|
||||
use crate::protocol::ENVIRONMENT_INFO_METHOD;
|
||||
use crate::protocol::EXEC_METHOD;
|
||||
use crate::protocol::EXEC_READ_METHOD;
|
||||
use crate::protocol::EXEC_TERMINATE_METHOD;
|
||||
@@ -60,6 +61,10 @@ pub(crate) fn build_router() -> RpcRouter<ExecServerHandler> {
|
||||
EXEC_METHOD,
|
||||
|handler: Arc<ExecServerHandler>, params: ExecParams| async move { handler.exec(params).await },
|
||||
);
|
||||
router.request(
|
||||
ENVIRONMENT_INFO_METHOD,
|
||||
|handler: Arc<ExecServerHandler>, _params: ()| async move { handler.environment_info() },
|
||||
);
|
||||
router.request(
|
||||
EXEC_READ_METHOD,
|
||||
|handler: Arc<ExecServerHandler>, params: ReadParams| async move {
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
|
||||
mod common;
|
||||
|
||||
use codex_exec_server::Environment;
|
||||
use common::exec_server::exec_server;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
@@ -19,3 +20,17 @@ async fn exec_server_serves_readyz_alongside_websocket_endpoint() -> anyhow::Res
|
||||
server.shutdown().await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn remote_environment_fetches_info_from_exec_server() -> anyhow::Result<()> {
|
||||
let mut server = exec_server().await?;
|
||||
let environment = Environment::create_for_tests(Some(server.websocket_url().to_string()))?;
|
||||
assert!(environment.is_remote());
|
||||
|
||||
let remote_info = environment.info().await?;
|
||||
let local_info = Environment::default_for_tests().info().await?;
|
||||
assert_eq!(remote_info, local_info);
|
||||
|
||||
server.shutdown().await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -99,6 +99,7 @@ pub fn model_info_from_slug(slug: &str) -> ModelInfo {
|
||||
input_modalities: default_input_modalities(),
|
||||
used_fallback_model_metadata: true, // this is the fallback model metadata
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
auto_review_model_override: None,
|
||||
tool_mode: None,
|
||||
multi_agent_version: None,
|
||||
|
||||
@@ -998,6 +998,37 @@ impl SessionTelemetry {
|
||||
);
|
||||
}
|
||||
|
||||
pub fn sandbox_outcome(
|
||||
&self,
|
||||
tool_name: &str,
|
||||
call_id: &str,
|
||||
outcome: &str,
|
||||
initial_duration: Duration,
|
||||
escalated_duration: Option<Duration>,
|
||||
) {
|
||||
let initial_duration_ms = initial_duration.as_millis().min(i64::MAX as u128) as i64;
|
||||
let escalated_duration_ms =
|
||||
escalated_duration.map(|duration| duration.as_millis().min(i64::MAX as u128) as i64);
|
||||
log_event!(
|
||||
self,
|
||||
event.name = "codex.sandbox_outcome",
|
||||
tool_name = %tool_name,
|
||||
call_id = %call_id,
|
||||
outcome = %outcome,
|
||||
initial_duration_ms = initial_duration_ms,
|
||||
escalated_duration_ms = escalated_duration_ms,
|
||||
);
|
||||
trace_event!(
|
||||
self,
|
||||
event.name = "codex.sandbox_outcome",
|
||||
tool_name = %tool_name,
|
||||
call_id = %call_id,
|
||||
outcome = %outcome,
|
||||
initial_duration_ms = initial_duration_ms,
|
||||
escalated_duration_ms = escalated_duration_ms,
|
||||
);
|
||||
}
|
||||
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
pub async fn log_tool_result_with_tags<F, Fut, E>(
|
||||
&self,
|
||||
|
||||
@@ -404,6 +404,8 @@ pub struct ModelInfo {
|
||||
pub used_fallback_model_metadata: bool,
|
||||
#[serde(default)]
|
||||
pub supports_search_tool: bool,
|
||||
#[serde(default)]
|
||||
pub use_responses_lite: bool,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub auto_review_model_override: Option<String>,
|
||||
#[serde(
|
||||
@@ -674,6 +676,7 @@ mod tests {
|
||||
input_modalities: default_input_modalities(),
|
||||
used_fallback_model_metadata: false,
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
auto_review_model_override: None,
|
||||
tool_mode: None,
|
||||
multi_agent_version: None,
|
||||
@@ -936,6 +939,7 @@ mod tests {
|
||||
assert!(!model.supports_image_detail_original);
|
||||
assert_eq!(model.web_search_tool_type, WebSearchToolType::Text);
|
||||
assert!(!model.supports_search_tool);
|
||||
assert!(!model.use_responses_lite);
|
||||
assert_eq!(model.auto_review_model_override, None);
|
||||
assert_eq!(model.tool_mode, None);
|
||||
}
|
||||
|
||||
@@ -70,6 +70,7 @@ codex-utils-cargo-bin = { workspace = true }
|
||||
pretty_assertions = { workspace = true }
|
||||
serial_test = { workspace = true }
|
||||
tempfile = { workspace = true }
|
||||
wiremock = { workspace = true }
|
||||
[target.'cfg(target_os = "linux")'.dependencies]
|
||||
keyring = { workspace = true, features = ["linux-native-async-persistent"] }
|
||||
|
||||
|
||||
@@ -113,7 +113,13 @@ fn refresh_expires_in_from_timestamp(tokens: &mut StoredOAuthTokens) {
|
||||
tokens.token_response.0.set_expires_in(Some(&duration));
|
||||
}
|
||||
None => {
|
||||
tokens.token_response.0.set_expires_in(None);
|
||||
// RMCP treats a missing expiry as unknown and uses the access token
|
||||
// as-is. Treat a known-expired timestamp as an explicit zero so
|
||||
// startup refreshes the token before the first request.
|
||||
tokens
|
||||
.token_response
|
||||
.0
|
||||
.set_expires_in(Some(&Duration::ZERO));
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -830,7 +836,7 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn refresh_expires_in_from_timestamp_clears_expired_tokens() {
|
||||
fn refresh_expires_in_from_timestamp_marks_expired_tokens() {
|
||||
let mut tokens = sample_tokens();
|
||||
let now = SystemTime::now()
|
||||
.duration_since(UNIX_EPOCH)
|
||||
@@ -843,7 +849,7 @@ mod tests {
|
||||
|
||||
super::refresh_expires_in_from_timestamp(&mut tokens);
|
||||
|
||||
assert!(tokens.token_response.0.expires_in().is_none());
|
||||
assert_eq!(tokens.token_response.0.expires_in(), Some(Duration::ZERO));
|
||||
}
|
||||
|
||||
fn assert_tokens_match_without_expiry(
|
||||
|
||||
@@ -0,0 +1,155 @@
|
||||
mod streamable_http_test_support;
|
||||
|
||||
use std::time::Duration;
|
||||
|
||||
use codex_config::types::OAuthCredentialsStoreMode;
|
||||
use codex_exec_server::Environment;
|
||||
use codex_rmcp_client::RmcpClient;
|
||||
use codex_rmcp_client::StoredOAuthTokens;
|
||||
use codex_rmcp_client::WrappedOAuthTokenResponse;
|
||||
use codex_rmcp_client::save_oauth_tokens;
|
||||
use oauth2::AccessToken;
|
||||
use oauth2::RefreshToken;
|
||||
use oauth2::basic::BasicTokenType;
|
||||
use rmcp::transport::auth::OAuthTokenResponse;
|
||||
use rmcp::transport::auth::VendorExtraTokenFields;
|
||||
use serde_json::Value;
|
||||
use serde_json::json;
|
||||
use tempfile::TempDir;
|
||||
use tokio::process::Command;
|
||||
use wiremock::Mock;
|
||||
use wiremock::MockServer;
|
||||
use wiremock::Request;
|
||||
use wiremock::ResponseTemplate;
|
||||
use wiremock::matchers::body_string_contains;
|
||||
use wiremock::matchers::header;
|
||||
use wiremock::matchers::method;
|
||||
use wiremock::matchers::path;
|
||||
|
||||
use streamable_http_test_support::initialize_client;
|
||||
|
||||
const SERVER_NAME: &str = "test-streamable-http-oauth-startup";
|
||||
const EXPIRED_ACCESS_TOKEN: &str = "expired-access-token";
|
||||
const REFRESH_TOKEN: &str = "valid-refresh-token";
|
||||
const REFRESHED_ACCESS_TOKEN: &str = "refreshed-access-token";
|
||||
const CHILD_SERVER_URL_ENV: &str = "MCP_TEST_OAUTH_STARTUP_SERVER_URL";
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
|
||||
async fn refreshes_expired_persisted_token_before_initialize() -> anyhow::Result<()> {
|
||||
let server = MockServer::start().await;
|
||||
Mock::given(method("GET"))
|
||||
.and(path("/.well-known/oauth-authorization-server/mcp"))
|
||||
.respond_with(ResponseTemplate::new(200).set_body_json(json!({
|
||||
"authorization_endpoint": format!("{}/oauth/authorize", server.uri()),
|
||||
"token_endpoint": format!("{}/oauth/token", server.uri()),
|
||||
"scopes_supported": [""],
|
||||
})))
|
||||
.expect(1)
|
||||
.mount(&server)
|
||||
.await;
|
||||
Mock::given(method("POST"))
|
||||
.and(path("/oauth/token"))
|
||||
.and(body_string_contains("grant_type=refresh_token"))
|
||||
.and(body_string_contains(format!(
|
||||
"refresh_token={REFRESH_TOKEN}"
|
||||
)))
|
||||
.respond_with(ResponseTemplate::new(200).set_body_json(json!({
|
||||
"access_token": REFRESHED_ACCESS_TOKEN,
|
||||
"token_type": "Bearer",
|
||||
"expires_in": 7200,
|
||||
"refresh_token": REFRESH_TOKEN,
|
||||
})))
|
||||
.expect(1)
|
||||
.mount(&server)
|
||||
.await;
|
||||
Mock::given(method("POST"))
|
||||
.and(path("/mcp"))
|
||||
.and(header(
|
||||
"authorization",
|
||||
format!("Bearer {REFRESHED_ACCESS_TOKEN}"),
|
||||
))
|
||||
.respond_with(|request: &Request| {
|
||||
let body: Value = request.body_json().expect("valid JSON-RPC request");
|
||||
match body.get("method").and_then(Value::as_str) {
|
||||
Some("initialize") => ResponseTemplate::new(200).set_body_json(json!({
|
||||
"jsonrpc": "2.0",
|
||||
"id": body.get("id").cloned().unwrap_or(Value::Null),
|
||||
"result": {
|
||||
"protocolVersion": body
|
||||
.pointer("/params/protocolVersion")
|
||||
.cloned()
|
||||
.unwrap_or_else(|| json!("2025-06-18")),
|
||||
"capabilities": {},
|
||||
"serverInfo": {
|
||||
"name": "oauth-startup-test",
|
||||
"version": "0.0.0-test",
|
||||
},
|
||||
},
|
||||
})),
|
||||
Some("notifications/initialized") => ResponseTemplate::new(202),
|
||||
method => ResponseTemplate::new(400)
|
||||
.set_body_string(format!("unexpected JSON-RPC method: {method:?}")),
|
||||
}
|
||||
})
|
||||
.expect(2)
|
||||
.mount(&server)
|
||||
.await;
|
||||
|
||||
let codex_home = TempDir::new()?;
|
||||
let server_url = format!("{}/mcp", server.uri());
|
||||
|
||||
// Credential storage resolves CODEX_HOME from the process environment.
|
||||
// Run the client half of the test in an ignored helper test so it can use
|
||||
// an isolated home without mutating the parent test runner's environment.
|
||||
let status = Command::new(std::env::current_exe()?)
|
||||
.args(["oauth_startup_child", "--exact", "--ignored", "--nocapture"])
|
||||
.env("CODEX_HOME", codex_home.path())
|
||||
.env(CHILD_SERVER_URL_ENV, server_url)
|
||||
.status()
|
||||
.await?;
|
||||
assert!(status.success(), "OAuth startup child failed: {status}");
|
||||
server.verify().await;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
|
||||
#[ignore = "spawned by refreshes_expired_persisted_token_before_initialize"]
|
||||
async fn oauth_startup_child() -> anyhow::Result<()> {
|
||||
let server_url = std::env::var(CHILD_SERVER_URL_ENV)?;
|
||||
|
||||
// Save an expired access token with a valid refresh token so startup must
|
||||
// refresh before sending the initialize request.
|
||||
let mut response = OAuthTokenResponse::new(
|
||||
AccessToken::new(EXPIRED_ACCESS_TOKEN.to_string()),
|
||||
BasicTokenType::Bearer,
|
||||
VendorExtraTokenFields::default(),
|
||||
);
|
||||
response.set_refresh_token(Some(RefreshToken::new(REFRESH_TOKEN.to_string())));
|
||||
response.set_expires_in(Some(&Duration::from_secs(7200)));
|
||||
let tokens = StoredOAuthTokens {
|
||||
server_name: SERVER_NAME.to_string(),
|
||||
url: server_url.clone(),
|
||||
client_id: "test-client-id".to_string(),
|
||||
token_response: WrappedOAuthTokenResponse(response),
|
||||
expires_at: Some(0),
|
||||
};
|
||||
save_oauth_tokens(SERVER_NAME, &tokens, OAuthCredentialsStoreMode::File)?;
|
||||
|
||||
// This mirrors create_client's transport and initialization setup, except
|
||||
// it omits the direct bearer token. Supplying that token would bypass the
|
||||
// persisted OAuth credentials and the startup refresh under test.
|
||||
let client = RmcpClient::new_streamable_http_client(
|
||||
SERVER_NAME,
|
||||
&server_url,
|
||||
/*bearer_token*/ None,
|
||||
/*http_headers*/ None,
|
||||
/*env_http_headers*/ None,
|
||||
OAuthCredentialsStoreMode::File,
|
||||
Environment::default_for_tests().get_http_client(),
|
||||
/*auth_provider*/ None,
|
||||
)
|
||||
.await?;
|
||||
|
||||
initialize_client(&client).await?;
|
||||
Ok(())
|
||||
}
|
||||
@@ -86,6 +86,12 @@ pub(crate) async fn create_client(base_url: &str) -> anyhow::Result<RmcpClient>
|
||||
)
|
||||
.await?;
|
||||
|
||||
initialize_client(&client).await?;
|
||||
|
||||
Ok(client)
|
||||
}
|
||||
|
||||
pub(crate) async fn initialize_client(client: &RmcpClient) -> anyhow::Result<()> {
|
||||
client
|
||||
.initialize(
|
||||
init_params(),
|
||||
@@ -102,8 +108,7 @@ pub(crate) async fn create_client(base_url: &str) -> anyhow::Result<RmcpClient>
|
||||
}),
|
||||
)
|
||||
.await?;
|
||||
|
||||
Ok(client)
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Creates a Streamable HTTP RMCP client that sends traffic through the remote
|
||||
|
||||
@@ -11,6 +11,7 @@ workspace = true
|
||||
base64 = { workspace = true }
|
||||
codex-protocol = { workspace = true }
|
||||
codex-utils-absolute-path = { workspace = true }
|
||||
libc = { workspace = true }
|
||||
once_cell = { workspace = true }
|
||||
regex = { workspace = true }
|
||||
serde = { workspace = true, features = ["derive"] }
|
||||
|
||||
@@ -100,7 +100,7 @@ pub fn extract_bash_command(command: &[String]) -> Option<(&str, &str)> {
|
||||
};
|
||||
if !matches!(flag.as_str(), "-lc" | "-c")
|
||||
|| !matches!(
|
||||
detect_shell_type(&PathBuf::from(shell)),
|
||||
detect_shell_type(PathBuf::from(shell)),
|
||||
Some(ShellType::Zsh) | Some(ShellType::Bash) | Some(ShellType::Sh)
|
||||
)
|
||||
{
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
//! Command parsing and safety utilities shared across Codex crates.
|
||||
|
||||
mod shell_detect;
|
||||
pub mod shell_detect;
|
||||
|
||||
pub mod bash;
|
||||
pub(crate) mod command_safety;
|
||||
|
||||
@@ -47,7 +47,7 @@ pub fn extract_powershell_command(command: &[String]) -> Option<(&str, &str)> {
|
||||
|
||||
let shell = &command[0];
|
||||
if !matches!(
|
||||
detect_shell_type(&PathBuf::from(shell)),
|
||||
detect_shell_type(PathBuf::from(shell)),
|
||||
Some(ShellType::PowerShell)
|
||||
) {
|
||||
return None;
|
||||
|
||||
@@ -1,8 +1,10 @@
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
|
||||
#[derive(Debug, PartialEq, Eq, Clone, Copy)]
|
||||
pub(crate) enum ShellType {
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
|
||||
#[derive(Debug, PartialEq, Eq, Clone, Copy, Serialize, Deserialize)]
|
||||
pub enum ShellType {
|
||||
Zsh,
|
||||
Bash,
|
||||
PowerShell,
|
||||
@@ -10,7 +12,32 @@ pub(crate) enum ShellType {
|
||||
Cmd,
|
||||
}
|
||||
|
||||
pub(crate) fn detect_shell_type(shell_path: &PathBuf) -> Option<ShellType> {
|
||||
impl ShellType {
|
||||
pub fn name(self) -> &'static str {
|
||||
match self {
|
||||
Self::Zsh => "zsh",
|
||||
Self::Bash => "bash",
|
||||
Self::PowerShell => "powershell",
|
||||
Self::Sh => "sh",
|
||||
Self::Cmd => "cmd",
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||
pub struct DetectedShell {
|
||||
pub shell_type: ShellType,
|
||||
pub shell_path: PathBuf,
|
||||
}
|
||||
|
||||
impl DetectedShell {
|
||||
pub fn name(&self) -> &'static str {
|
||||
self.shell_type.name()
|
||||
}
|
||||
}
|
||||
|
||||
pub fn detect_shell_type(shell_path: impl AsRef<std::path::Path>) -> Option<ShellType> {
|
||||
let shell_path = shell_path.as_ref();
|
||||
match shell_path.as_os_str().to_str() {
|
||||
Some("zsh") => Some(ShellType::Zsh),
|
||||
Some("sh") => Some(ShellType::Sh),
|
||||
@@ -21,12 +48,317 @@ pub(crate) fn detect_shell_type(shell_path: &PathBuf) -> Option<ShellType> {
|
||||
_ => {
|
||||
let shell_name = shell_path.file_stem();
|
||||
if let Some(shell_name) = shell_name {
|
||||
let shell_name_path = Path::new(shell_name);
|
||||
if shell_name_path != Path::new(shell_path) {
|
||||
return detect_shell_type(&shell_name_path.to_path_buf());
|
||||
let shell_name_path = std::path::Path::new(shell_name);
|
||||
if shell_name_path != shell_path {
|
||||
return detect_shell_type(shell_name_path);
|
||||
}
|
||||
}
|
||||
None
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
fn get_user_shell_path() -> Option<PathBuf> {
|
||||
let uid = unsafe { libc::getuid() };
|
||||
use std::ffi::CStr;
|
||||
use std::mem::MaybeUninit;
|
||||
use std::ptr;
|
||||
|
||||
let mut passwd = MaybeUninit::<libc::passwd>::uninit();
|
||||
|
||||
// We cannot use getpwuid here: it returns pointers into libc-managed
|
||||
// storage, which is not safe to read concurrently on all targets (the musl
|
||||
// static build used by the CLI can segfault when parallel callers race on
|
||||
// that buffer). getpwuid_r keeps the passwd data in caller-owned memory.
|
||||
let suggested_buffer_len = unsafe { libc::sysconf(libc::_SC_GETPW_R_SIZE_MAX) };
|
||||
let buffer_len = usize::try_from(suggested_buffer_len)
|
||||
.ok()
|
||||
.filter(|len| *len > 0)
|
||||
.unwrap_or(1024);
|
||||
let mut buffer = vec![0; buffer_len];
|
||||
|
||||
loop {
|
||||
let mut result = ptr::null_mut();
|
||||
let status = unsafe {
|
||||
libc::getpwuid_r(
|
||||
uid,
|
||||
passwd.as_mut_ptr(),
|
||||
buffer.as_mut_ptr().cast(),
|
||||
buffer.len(),
|
||||
&mut result,
|
||||
)
|
||||
};
|
||||
|
||||
if status == 0 {
|
||||
if result.is_null() {
|
||||
return None;
|
||||
}
|
||||
|
||||
let passwd = unsafe { passwd.assume_init_ref() };
|
||||
if passwd.pw_shell.is_null() {
|
||||
return None;
|
||||
}
|
||||
|
||||
let shell_path = unsafe { CStr::from_ptr(passwd.pw_shell) }
|
||||
.to_string_lossy()
|
||||
.into_owned();
|
||||
return Some(PathBuf::from(shell_path));
|
||||
}
|
||||
|
||||
if status != libc::ERANGE {
|
||||
return None;
|
||||
}
|
||||
|
||||
// Retry with a larger buffer until libc can materialize the passwd entry.
|
||||
let new_len = buffer.len().checked_mul(2)?;
|
||||
if new_len > 1024 * 1024 {
|
||||
return None;
|
||||
}
|
||||
buffer.resize(new_len, 0);
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(not(unix))]
|
||||
fn get_user_shell_path() -> Option<PathBuf> {
|
||||
None
|
||||
}
|
||||
|
||||
fn file_exists(path: &std::path::Path) -> Option<PathBuf> {
|
||||
if std::fs::metadata(path).is_ok_and(|metadata| metadata.is_file()) {
|
||||
Some(PathBuf::from(path))
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
|
||||
fn get_shell_path(
|
||||
shell_type: ShellType,
|
||||
provided_path: Option<&PathBuf>,
|
||||
binary_name: &str,
|
||||
fallback_paths: &[&str],
|
||||
) -> Option<PathBuf> {
|
||||
if let Some(path) = provided_path.and_then(|path| file_exists(path)) {
|
||||
return Some(path);
|
||||
}
|
||||
|
||||
let default_shell_path = get_user_shell_path();
|
||||
if let Some(default_shell_path) = default_shell_path
|
||||
&& detect_shell_type(&default_shell_path) == Some(shell_type)
|
||||
&& file_exists(&default_shell_path).is_some()
|
||||
{
|
||||
return Some(default_shell_path);
|
||||
}
|
||||
|
||||
if let Ok(path) = which::which(binary_name) {
|
||||
return Some(path);
|
||||
}
|
||||
|
||||
for path in fallback_paths {
|
||||
if let Some(path) = file_exists(std::path::Path::new(path)) {
|
||||
return Some(path);
|
||||
}
|
||||
}
|
||||
|
||||
None
|
||||
}
|
||||
|
||||
const ZSH_FALLBACK_PATHS: &[&str] = &["/bin/zsh"];
|
||||
|
||||
fn get_zsh_shell(path: Option<&PathBuf>) -> Option<DetectedShell> {
|
||||
let shell_path = get_shell_path(ShellType::Zsh, path, "zsh", ZSH_FALLBACK_PATHS);
|
||||
|
||||
shell_path.map(|shell_path| DetectedShell {
|
||||
shell_type: ShellType::Zsh,
|
||||
shell_path,
|
||||
})
|
||||
}
|
||||
|
||||
const BASH_FALLBACK_PATHS: &[&str] = &["/bin/bash"];
|
||||
|
||||
fn get_bash_shell(path: Option<&PathBuf>) -> Option<DetectedShell> {
|
||||
let shell_path = get_shell_path(ShellType::Bash, path, "bash", BASH_FALLBACK_PATHS);
|
||||
|
||||
shell_path.map(|shell_path| DetectedShell {
|
||||
shell_type: ShellType::Bash,
|
||||
shell_path,
|
||||
})
|
||||
}
|
||||
|
||||
const SH_FALLBACK_PATHS: &[&str] = &["/bin/sh"];
|
||||
|
||||
fn get_sh_shell(path: Option<&PathBuf>) -> Option<DetectedShell> {
|
||||
let shell_path = get_shell_path(ShellType::Sh, path, "sh", SH_FALLBACK_PATHS);
|
||||
|
||||
shell_path.map(|shell_path| DetectedShell {
|
||||
shell_type: ShellType::Sh,
|
||||
shell_path,
|
||||
})
|
||||
}
|
||||
|
||||
// Note the `pwsh` and `powershell` fallback paths are where the respective
|
||||
// shells are commonly installed on GitHub Actions Windows runners, but may not
|
||||
// be present on all Windows machines:
|
||||
// https://docs.github.com/en/actions/tutorials/build-and-test-code/powershell
|
||||
|
||||
#[cfg(windows)]
|
||||
const PWSH_FALLBACK_PATHS: &[&str] = &[r#"C:\Program Files\PowerShell\7\pwsh.exe"#];
|
||||
#[cfg(not(windows))]
|
||||
const PWSH_FALLBACK_PATHS: &[&str] = &["/usr/local/bin/pwsh"];
|
||||
|
||||
#[cfg(windows)]
|
||||
const POWERSHELL_FALLBACK_PATHS: &[&str] =
|
||||
&[r#"C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe"#];
|
||||
#[cfg(not(windows))]
|
||||
const POWERSHELL_FALLBACK_PATHS: &[&str] = &[];
|
||||
|
||||
fn get_powershell_shell(path: Option<&PathBuf>) -> Option<DetectedShell> {
|
||||
let shell_path = get_shell_path(ShellType::PowerShell, path, "pwsh", PWSH_FALLBACK_PATHS)
|
||||
.or_else(|| {
|
||||
get_shell_path(
|
||||
ShellType::PowerShell,
|
||||
path,
|
||||
"powershell",
|
||||
POWERSHELL_FALLBACK_PATHS,
|
||||
)
|
||||
});
|
||||
|
||||
shell_path.map(|shell_path| DetectedShell {
|
||||
shell_type: ShellType::PowerShell,
|
||||
shell_path,
|
||||
})
|
||||
}
|
||||
|
||||
fn get_cmd_shell(path: Option<&PathBuf>) -> Option<DetectedShell> {
|
||||
let shell_path = get_shell_path(ShellType::Cmd, path, "cmd", &[]);
|
||||
|
||||
shell_path.map(|shell_path| DetectedShell {
|
||||
shell_type: ShellType::Cmd,
|
||||
shell_path,
|
||||
})
|
||||
}
|
||||
|
||||
pub fn ultimate_fallback_shell() -> DetectedShell {
|
||||
if cfg!(windows) {
|
||||
DetectedShell {
|
||||
shell_type: ShellType::Cmd,
|
||||
shell_path: PathBuf::from("cmd.exe"),
|
||||
}
|
||||
} else {
|
||||
DetectedShell {
|
||||
shell_type: ShellType::Sh,
|
||||
shell_path: PathBuf::from("/bin/sh"),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pub fn get_shell_by_model_provided_path(shell_path: &PathBuf) -> DetectedShell {
|
||||
detect_shell_type(shell_path)
|
||||
.and_then(|shell_type| get_shell(shell_type, Some(shell_path)))
|
||||
.unwrap_or_else(ultimate_fallback_shell)
|
||||
}
|
||||
|
||||
pub fn get_shell(shell_type: ShellType, path: Option<&PathBuf>) -> Option<DetectedShell> {
|
||||
match shell_type {
|
||||
ShellType::Zsh => get_zsh_shell(path),
|
||||
ShellType::Bash => get_bash_shell(path),
|
||||
ShellType::PowerShell => get_powershell_shell(path),
|
||||
ShellType::Sh => get_sh_shell(path),
|
||||
ShellType::Cmd => get_cmd_shell(path),
|
||||
}
|
||||
}
|
||||
|
||||
pub fn default_user_shell() -> DetectedShell {
|
||||
default_user_shell_from_path(get_user_shell_path())
|
||||
}
|
||||
|
||||
pub fn default_user_shell_from_path(user_shell_path: Option<PathBuf>) -> DetectedShell {
|
||||
if cfg!(windows) {
|
||||
get_shell(ShellType::PowerShell, /*path*/ None).unwrap_or_else(ultimate_fallback_shell)
|
||||
} else {
|
||||
let user_default_shell = user_shell_path
|
||||
.and_then(|shell| detect_shell_type(&shell))
|
||||
.and_then(|shell_type| get_shell(shell_type, /*path*/ None));
|
||||
|
||||
let shell_with_fallback = if cfg!(target_os = "macos") {
|
||||
user_default_shell
|
||||
.or_else(|| get_shell(ShellType::Zsh, /*path*/ None))
|
||||
.or_else(|| get_shell(ShellType::Bash, /*path*/ None))
|
||||
} else {
|
||||
user_default_shell
|
||||
.or_else(|| get_shell(ShellType::Bash, /*path*/ None))
|
||||
.or_else(|| get_shell(ShellType::Zsh, /*path*/ None))
|
||||
};
|
||||
|
||||
shell_with_fallback.unwrap_or_else(ultimate_fallback_shell)
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[test]
|
||||
fn test_detect_shell_type() {
|
||||
assert_eq!(
|
||||
detect_shell_type(PathBuf::from("zsh")),
|
||||
Some(ShellType::Zsh)
|
||||
);
|
||||
assert_eq!(
|
||||
detect_shell_type(PathBuf::from("bash")),
|
||||
Some(ShellType::Bash)
|
||||
);
|
||||
assert_eq!(
|
||||
detect_shell_type(PathBuf::from("pwsh")),
|
||||
Some(ShellType::PowerShell)
|
||||
);
|
||||
assert_eq!(
|
||||
detect_shell_type(PathBuf::from("powershell")),
|
||||
Some(ShellType::PowerShell)
|
||||
);
|
||||
assert_eq!(detect_shell_type(PathBuf::from("fish")), None);
|
||||
assert_eq!(detect_shell_type(PathBuf::from("other")), None);
|
||||
assert_eq!(
|
||||
detect_shell_type(PathBuf::from("/bin/zsh")),
|
||||
Some(ShellType::Zsh)
|
||||
);
|
||||
assert_eq!(
|
||||
detect_shell_type(PathBuf::from("/bin/bash")),
|
||||
Some(ShellType::Bash)
|
||||
);
|
||||
assert_eq!(
|
||||
detect_shell_type(PathBuf::from("powershell.exe")),
|
||||
Some(ShellType::PowerShell)
|
||||
);
|
||||
assert_eq!(
|
||||
detect_shell_type(PathBuf::from(if cfg!(windows) {
|
||||
"C:\\windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe"
|
||||
} else {
|
||||
"/usr/local/bin/pwsh"
|
||||
})),
|
||||
Some(ShellType::PowerShell)
|
||||
);
|
||||
assert_eq!(
|
||||
detect_shell_type(PathBuf::from("pwsh.exe")),
|
||||
Some(ShellType::PowerShell)
|
||||
);
|
||||
assert_eq!(
|
||||
detect_shell_type(PathBuf::from("/usr/local/bin/pwsh")),
|
||||
Some(ShellType::PowerShell)
|
||||
);
|
||||
assert_eq!(
|
||||
detect_shell_type(PathBuf::from("/bin/sh")),
|
||||
Some(ShellType::Sh)
|
||||
);
|
||||
assert_eq!(detect_shell_type(PathBuf::from("sh")), Some(ShellType::Sh));
|
||||
assert_eq!(
|
||||
detect_shell_type(PathBuf::from("cmd")),
|
||||
Some(ShellType::Cmd)
|
||||
);
|
||||
assert_eq!(
|
||||
detect_shell_type(PathBuf::from("cmd.exe")),
|
||||
Some(ShellType::Cmd)
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -44,6 +44,7 @@ fn model_with_shell_type(shell_type: ConfigShellToolType) -> ModelInfo {
|
||||
input_modalities: codex_protocol::openai_models::default_input_modalities(),
|
||||
used_fallback_model_metadata: false,
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
auto_review_model_override: None,
|
||||
tool_mode: None,
|
||||
multi_agent_version: None,
|
||||
|
||||
@@ -15,6 +15,7 @@ use crate::allow::compute_allow_paths_for_permissions;
|
||||
use crate::helper_materialization::bundled_executable_path_for_exe;
|
||||
use crate::helper_materialization::helper_bin_dir;
|
||||
use crate::identity::sandbox_setup_is_complete;
|
||||
use crate::logging::current_log_file_path;
|
||||
use crate::logging::log_note;
|
||||
use crate::path_normalization::canonical_path_key;
|
||||
use crate::path_normalization::canonicalize_path;
|
||||
@@ -25,7 +26,6 @@ use crate::setup_error::clear_setup_error_report;
|
||||
use crate::setup_error::failure;
|
||||
use crate::setup_error::read_setup_error_report;
|
||||
use crate::ssh_config_dependencies::ssh_config_dependency_paths;
|
||||
use anyhow::Context;
|
||||
use anyhow::Result;
|
||||
use anyhow::anyhow;
|
||||
use base64::Engine;
|
||||
@@ -210,6 +210,18 @@ fn run_setup_refresh_inner(
|
||||
let json = serde_json::to_vec(&payload)?;
|
||||
let b64 = BASE64_STANDARD.encode(json);
|
||||
let exe = find_setup_exe();
|
||||
let sbx_dir = sandbox_dir(request.codex_home);
|
||||
let log_path = current_log_file_path(&sbx_dir);
|
||||
let cleared_report = match clear_setup_error_report(request.codex_home) {
|
||||
Ok(()) => true,
|
||||
Err(err) => {
|
||||
log_note(
|
||||
&format!("setup refresh: failed to clear setup_error.json before launch: {err}"),
|
||||
Some(&sbx_dir),
|
||||
);
|
||||
false
|
||||
}
|
||||
};
|
||||
// Refresh should never request elevation; ensure verb isn't set and we don't trigger UAC.
|
||||
let mut cmd = Command::new(&exe);
|
||||
cmd.arg(&b64).stdout(Stdio::null()).stderr(Stdio::null());
|
||||
@@ -221,24 +233,34 @@ fn run_setup_refresh_inner(
|
||||
cwd.display(),
|
||||
b64.len()
|
||||
),
|
||||
Some(&sandbox_dir(request.codex_home)),
|
||||
Some(&sbx_dir),
|
||||
);
|
||||
let status = cmd
|
||||
.status()
|
||||
.map_err(|e| {
|
||||
log_note(
|
||||
&format!("setup refresh: failed to spawn {}: {e}", exe.display()),
|
||||
Some(&sandbox_dir(request.codex_home)),
|
||||
);
|
||||
e
|
||||
})
|
||||
.context("spawn setup refresh")?;
|
||||
let status = cmd.status().map_err(|err| {
|
||||
let message = format!(
|
||||
"setup refresh failed to launch helper: helper={}, cwd={}, log={}, error={err}",
|
||||
exe.display(),
|
||||
cwd.display(),
|
||||
log_path.display()
|
||||
);
|
||||
log_note(&format!("setup refresh: {message}"), Some(&sbx_dir));
|
||||
failure(SetupErrorCode::OrchestratorHelperLaunchFailed, message)
|
||||
})?;
|
||||
if !status.success() {
|
||||
log_note(
|
||||
&format!("setup refresh: exited with status {status:?}"),
|
||||
Some(&sandbox_dir(request.codex_home)),
|
||||
Some(&sbx_dir),
|
||||
);
|
||||
return Err(report_helper_failure(
|
||||
request.codex_home,
|
||||
cleared_report,
|
||||
status.code(),
|
||||
));
|
||||
}
|
||||
if let Err(err) = clear_setup_error_report(request.codex_home) {
|
||||
log_note(
|
||||
&format!("setup refresh: failed to clear setup_error.json after success: {err}"),
|
||||
Some(&sbx_dir),
|
||||
);
|
||||
return Err(anyhow!("setup refresh failed with status {status}"));
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
@@ -1087,7 +1109,9 @@ mod tests {
|
||||
use crate::helper_materialization::helper_bin_dir;
|
||||
use crate::resolved_permissions::ResolvedWindowsSandboxPermissions;
|
||||
use crate::setup_error::SetupErrorCode;
|
||||
use crate::setup_error::SetupErrorReport;
|
||||
use crate::setup_error::extract_failure;
|
||||
use crate::setup_error::write_setup_error_report;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::permissions::NetworkSandboxPolicy;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
@@ -1146,6 +1170,64 @@ mod tests {
|
||||
)
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn report_helper_failure_uses_setup_error_report_when_clear_succeeded() {
|
||||
let tmp = TempDir::new().expect("tempdir");
|
||||
let codex_home = tmp.path().join("codex-home");
|
||||
write_setup_error_report(
|
||||
codex_home.as_path(),
|
||||
&SetupErrorReport {
|
||||
code: super::SetupErrorCode::HelperFirewallPolicyAccessFailed,
|
||||
message: "firewall policy unavailable".to_string(),
|
||||
},
|
||||
)
|
||||
.expect("write setup error report");
|
||||
|
||||
let err = super::report_helper_failure(
|
||||
codex_home.as_path(),
|
||||
/*cleared_report*/ true,
|
||||
/*exit_code*/ Some(1),
|
||||
);
|
||||
|
||||
let failure = extract_failure(&err).expect("structured setup failure");
|
||||
assert_eq!(
|
||||
&super::SetupFailure::new(
|
||||
super::SetupErrorCode::HelperFirewallPolicyAccessFailed,
|
||||
"firewall policy unavailable",
|
||||
),
|
||||
failure
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn report_helper_failure_ignores_setup_error_report_when_clear_failed() {
|
||||
let tmp = TempDir::new().expect("tempdir");
|
||||
let codex_home = tmp.path().join("codex-home");
|
||||
write_setup_error_report(
|
||||
codex_home.as_path(),
|
||||
&SetupErrorReport {
|
||||
code: super::SetupErrorCode::HelperFirewallPolicyAccessFailed,
|
||||
message: "stale report".to_string(),
|
||||
},
|
||||
)
|
||||
.expect("write setup error report");
|
||||
|
||||
let err = super::report_helper_failure(
|
||||
codex_home.as_path(),
|
||||
/*cleared_report*/ false,
|
||||
/*exit_code*/ Some(1),
|
||||
);
|
||||
|
||||
let failure = extract_failure(&err).expect("structured setup failure");
|
||||
assert_eq!(
|
||||
&super::SetupFailure::new(
|
||||
super::SetupErrorCode::OrchestratorHelperExitNonzero,
|
||||
"setup helper exited with status Some(1)",
|
||||
),
|
||||
failure
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn setup_refresh_skips_profiles_without_managed_filesystem_permissions() {
|
||||
let tmp = TempDir::new().expect("tempdir");
|
||||
|
||||
@@ -117,7 +117,7 @@ pub struct SetupErrorReport {
|
||||
pub message: String,
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
#[derive(Debug, PartialEq, Eq)]
|
||||
pub struct SetupFailure {
|
||||
pub code: SetupErrorCode,
|
||||
pub message: String,
|
||||
|
||||
@@ -106,6 +106,55 @@ index 9648e4a..88efd41 100644
|
||||
}) + select({
|
||||
":should_add_rdynamic": ["-rdynamic"],
|
||||
"//conditions:default": [],
|
||||
@@ -459,6 +488,11 @@
|
||||
def _mksnapshot(ctx):
|
||||
prefix = ctx.attr.prefix
|
||||
suffix = ctx.attr.suffix
|
||||
+ # Windows cross-builds use Linux for the exec configuration, but the
|
||||
+ # snapshot generator must match the target ABI and run on the Windows
|
||||
+ # runner. Action strategies only choose where an action runs; they cannot
|
||||
+ # change this executable from the exec to the target configuration.
|
||||
+ tool = ctx.executable.target_tool if ctx.attr.target_os == "win" else ctx.executable.tool
|
||||
outs = [
|
||||
ctx.actions.declare_file(prefix + "/snapshot" + suffix + ".cc"),
|
||||
ctx.actions.declare_file(prefix + "/embedded" + suffix + ".S"),
|
||||
@@ -476,7 +510,7 @@
|
||||
outs[1].path,
|
||||
] + ctx.attr.args,
|
||||
- executable = ctx.executable.tool,
|
||||
+ executable = tool,
|
||||
progress_message = "Running mksnapshot",
|
||||
)
|
||||
return [DefaultInfo(files = depset(outs))]
|
||||
@@ -491,6 +525,12 @@
|
||||
executable = True,
|
||||
cfg = "exec",
|
||||
),
|
||||
+ "target_tool": attr.label(
|
||||
+ mandatory = True,
|
||||
+ allow_files = True,
|
||||
+ executable = True,
|
||||
+ cfg = "target",
|
||||
+ ),
|
||||
"target_os": attr.string(mandatory = True),
|
||||
"prefix": attr.string(mandatory = True),
|
||||
"suffix": attr.string(mandatory = True),
|
||||
@@ -504,6 +544,7 @@
|
||||
args = args,
|
||||
prefix = "noicu",
|
||||
tool = ":noicu/mksnapshot" + suffix,
|
||||
+ target_tool = ":noicu/mksnapshot" + suffix,
|
||||
suffix = suffix,
|
||||
target_os = select({
|
||||
"@v8//bazel/config:is_macos": "mac",
|
||||
@@ -516,6 +557,7 @@
|
||||
args = args,
|
||||
prefix = "icu",
|
||||
tool = ":icu/mksnapshot" + suffix,
|
||||
+ target_tool = ":icu/mksnapshot" + suffix,
|
||||
suffix = suffix,
|
||||
target_os = select({
|
||||
"@v8//bazel/config:is_macos": "mac",
|
||||
diff --git a/orig/v8-14.6.202.11/BUILD.bazel b/mod/v8-14.6.202.11/BUILD.bazel
|
||||
index 421ebcd..52283ea 100644
|
||||
--- a/orig/v8-14.6.202.11/BUILD.bazel
|
||||
@@ -309,7 +358,7 @@ index 421ebcd..52283ea 100644
|
||||
],
|
||||
)
|
||||
|
||||
@@ -4772,9 +4784,15 @@ v8_binary(
|
||||
@@ -4772,9 +4784,20 @@ v8_binary(
|
||||
":icu/generated_torque_initializers",
|
||||
":icu/v8_initializers_files",
|
||||
],
|
||||
@@ -317,11 +366,16 @@ index 421ebcd..52283ea 100644
|
||||
+ # external-reference helpers together while producing the snapshot, the
|
||||
+ # final embedder binary may not fold the same pair and startup
|
||||
+ # deserialization will reject the snapshot.
|
||||
+ # GN also increases the x64 Windows stack reserve because constructing the
|
||||
+ # snapshot overflows the linker's 1 MiB default.
|
||||
linkopts = select({
|
||||
"@v8//bazel/config:is_android": ["-llog"],
|
||||
- "//conditions:default": [],
|
||||
+ "@v8//bazel/config:is_macos": ["-Wl,-no_deduplicate"],
|
||||
+ "@v8//bazel/config:is_windows": ["/OPT:NOICF"],
|
||||
+ "@v8//bazel/config:is_windows": [
|
||||
+ "-Wl,/OPT:NOICF",
|
||||
+ "-Wl,/STACK:2097152",
|
||||
+ ],
|
||||
+ "//conditions:default": ["-Wl,--icf=none"],
|
||||
}),
|
||||
noicu_deps = [":v8_libshared_noicu"],
|
||||
|
||||
@@ -1,7 +1,8 @@
|
||||
#!/usr/bin/env bash
|
||||
set -euo pipefail
|
||||
|
||||
if ! bazel mod deps --lockfile_mode=error; then
|
||||
repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
|
||||
if ! "${repo_root}/.github/scripts/run_bazel_with_buildbuddy.py" mod deps --lockfile_mode=error; then
|
||||
echo "MODULE.bazel.lock is out of date."
|
||||
echo "Run 'just bazel-lock-update' and commit the updated lockfile."
|
||||
exit 1
|
||||
|
||||
Reference in New Issue
Block a user