fix: warn when --add-dir would be ignored (#5351)
Add shared helper to format warnings when add-dir is incompatible with the sandbox. Surface the warning in the TUI entrypoint and document the limitation for add-dir.
This commit is contained in:
committed by
GitHub
parent
846960ae3d
commit
8044b55335
71
codex-rs/tui/src/additional_dirs.rs
Normal file
71
codex-rs/tui/src/additional_dirs.rs
Normal file
@@ -0,0 +1,71 @@
|
|||||||
|
use codex_core::protocol::SandboxPolicy;
|
||||||
|
use std::path::PathBuf;
|
||||||
|
|
||||||
|
/// Returns a warning describing why `--add-dir` entries will be ignored for the
|
||||||
|
/// resolved sandbox policy. The caller is responsible for presenting the
|
||||||
|
/// warning to the user (for example, printing to stderr).
|
||||||
|
pub fn add_dir_warning_message(
|
||||||
|
additional_dirs: &[PathBuf],
|
||||||
|
sandbox_policy: &SandboxPolicy,
|
||||||
|
) -> Option<String> {
|
||||||
|
if additional_dirs.is_empty() {
|
||||||
|
return None;
|
||||||
|
}
|
||||||
|
|
||||||
|
match sandbox_policy {
|
||||||
|
SandboxPolicy::WorkspaceWrite { .. } | SandboxPolicy::DangerFullAccess => None,
|
||||||
|
SandboxPolicy::ReadOnly => Some(format_warning(additional_dirs)),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fn format_warning(additional_dirs: &[PathBuf]) -> String {
|
||||||
|
let joined_paths = additional_dirs
|
||||||
|
.iter()
|
||||||
|
.map(|path| path.to_string_lossy())
|
||||||
|
.collect::<Vec<_>>()
|
||||||
|
.join(", ");
|
||||||
|
format!(
|
||||||
|
"Ignoring --add-dir ({joined_paths}) because the effective sandbox mode is read-only. Switch to workspace-write or danger-full-access to allow additional writable roots."
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use super::add_dir_warning_message;
|
||||||
|
use codex_core::protocol::SandboxPolicy;
|
||||||
|
use pretty_assertions::assert_eq;
|
||||||
|
use std::path::PathBuf;
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn returns_none_for_workspace_write() {
|
||||||
|
let sandbox = SandboxPolicy::new_workspace_write_policy();
|
||||||
|
let dirs = vec![PathBuf::from("/tmp/example")];
|
||||||
|
assert_eq!(add_dir_warning_message(&dirs, &sandbox), None);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn returns_none_for_danger_full_access() {
|
||||||
|
let sandbox = SandboxPolicy::DangerFullAccess;
|
||||||
|
let dirs = vec![PathBuf::from("/tmp/example")];
|
||||||
|
assert_eq!(add_dir_warning_message(&dirs, &sandbox), None);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn warns_for_read_only() {
|
||||||
|
let sandbox = SandboxPolicy::ReadOnly;
|
||||||
|
let dirs = vec![PathBuf::from("relative"), PathBuf::from("/abs")];
|
||||||
|
let message = add_dir_warning_message(&dirs, &sandbox)
|
||||||
|
.expect("expected warning for read-only sandbox");
|
||||||
|
assert_eq!(
|
||||||
|
message,
|
||||||
|
"Ignoring --add-dir (relative, /abs) because the effective sandbox mode is read-only. Switch to workspace-write or danger-full-access to allow additional writable roots."
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn returns_none_when_no_additional_dirs() {
|
||||||
|
let sandbox = SandboxPolicy::ReadOnly;
|
||||||
|
let dirs: Vec<PathBuf> = Vec::new();
|
||||||
|
assert_eq!(add_dir_warning_message(&dirs, &sandbox), None);
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -3,6 +3,7 @@
|
|||||||
// alternate‑screen mode starts; that file opts‑out locally via `allow`.
|
// alternate‑screen mode starts; that file opts‑out locally via `allow`.
|
||||||
#![deny(clippy::print_stdout, clippy::print_stderr)]
|
#![deny(clippy::print_stdout, clippy::print_stderr)]
|
||||||
#![deny(clippy::disallowed_methods)]
|
#![deny(clippy::disallowed_methods)]
|
||||||
|
use additional_dirs::add_dir_warning_message;
|
||||||
use app::App;
|
use app::App;
|
||||||
pub use app::AppExitInfo;
|
pub use app::AppExitInfo;
|
||||||
use codex_app_server_protocol::AuthMode;
|
use codex_app_server_protocol::AuthMode;
|
||||||
@@ -27,6 +28,7 @@ use tracing_subscriber::EnvFilter;
|
|||||||
use tracing_subscriber::filter::Targets;
|
use tracing_subscriber::filter::Targets;
|
||||||
use tracing_subscriber::prelude::*;
|
use tracing_subscriber::prelude::*;
|
||||||
|
|
||||||
|
mod additional_dirs;
|
||||||
mod app;
|
mod app;
|
||||||
mod app_backtrack;
|
mod app_backtrack;
|
||||||
mod app_event;
|
mod app_event;
|
||||||
@@ -194,6 +196,14 @@ pub async fn run_main(
|
|||||||
|
|
||||||
let config = load_config_or_exit(cli_kv_overrides.clone(), overrides.clone()).await;
|
let config = load_config_or_exit(cli_kv_overrides.clone(), overrides.clone()).await;
|
||||||
|
|
||||||
|
if let Some(warning) = add_dir_warning_message(&cli.add_dir, &config.sandbox_policy) {
|
||||||
|
#[allow(clippy::print_stderr)]
|
||||||
|
{
|
||||||
|
eprintln!("Error adding directories: {warning}");
|
||||||
|
std::process::exit(1);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
#[allow(clippy::print_stderr)]
|
#[allow(clippy::print_stderr)]
|
||||||
if let Err(err) = enforce_login_restrictions(&config).await {
|
if let Err(err) = enforce_login_restrictions(&config).await {
|
||||||
eprintln!("{err}");
|
eprintln!("{err}");
|
||||||
|
|||||||
Reference in New Issue
Block a user