mirror of
https://github.com/openai/codex.git
synced 2026-04-24 14:45:27 +00:00
feat: gate non-pty detach behind experimental flag
This commit is contained in:
@@ -1215,6 +1215,7 @@ impl CodexMessageProcessor {
|
||||
let timeout_ms = params
|
||||
.timeout_ms
|
||||
.and_then(|timeout_ms| u64::try_from(timeout_ms).ok());
|
||||
let detach_from_tty = self.config.features.enabled(Feature::DetachNonTty);
|
||||
let exec_params = ExecParams {
|
||||
command: params.command,
|
||||
cwd,
|
||||
@@ -1223,6 +1224,7 @@ impl CodexMessageProcessor {
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
justification: None,
|
||||
arg0: None,
|
||||
detach_from_tty,
|
||||
};
|
||||
|
||||
let requested_policy = params.sandbox_policy.map(|policy| policy.to_core());
|
||||
|
||||
@@ -75,9 +75,15 @@
|
||||
"apply_patch_freeform": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"child_agents_md": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"collab": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"detach_non_tty": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"elevated_windows_sandbox": {
|
||||
"type": "boolean"
|
||||
},
|
||||
@@ -99,9 +105,6 @@
|
||||
"experimental_windows_sandbox": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"child_agents_md": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"include_apply_patch_tool": {
|
||||
"type": "boolean"
|
||||
},
|
||||
@@ -543,9 +546,15 @@
|
||||
"apply_patch_freeform": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"child_agents_md": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"collab": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"detach_non_tty": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"elevated_windows_sandbox": {
|
||||
"type": "boolean"
|
||||
},
|
||||
@@ -567,9 +576,6 @@
|
||||
"experimental_windows_sandbox": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"child_agents_md": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"include_apply_patch_tool": {
|
||||
"type": "boolean"
|
||||
},
|
||||
|
||||
@@ -4286,6 +4286,7 @@ mod tests {
|
||||
sandbox_permissions,
|
||||
justification: Some("test".to_string()),
|
||||
arg0: None,
|
||||
detach_from_tty: false,
|
||||
};
|
||||
|
||||
let params2 = ExecParams {
|
||||
@@ -4296,6 +4297,7 @@ mod tests {
|
||||
env: HashMap::new(),
|
||||
justification: params.justification.clone(),
|
||||
arg0: None,
|
||||
detach_from_tty: false,
|
||||
};
|
||||
|
||||
let turn_diff_tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::new()));
|
||||
|
||||
@@ -60,6 +60,7 @@ pub struct ExecParams {
|
||||
pub sandbox_permissions: SandboxPermissions,
|
||||
pub justification: Option<String>,
|
||||
pub arg0: Option<String>,
|
||||
pub detach_from_tty: bool,
|
||||
}
|
||||
|
||||
/// Mechanism to terminate an exec invocation before it finishes naturally.
|
||||
@@ -151,6 +152,7 @@ pub async fn process_exec_tool_call(
|
||||
sandbox_permissions,
|
||||
justification,
|
||||
arg0: _,
|
||||
detach_from_tty,
|
||||
} = params;
|
||||
|
||||
let (program, args) = command.split_first().ok_or_else(|| {
|
||||
@@ -168,6 +170,7 @@ pub async fn process_exec_tool_call(
|
||||
expiration,
|
||||
sandbox_permissions,
|
||||
justification,
|
||||
detach_from_tty,
|
||||
};
|
||||
|
||||
let manager = SandboxManager::new();
|
||||
@@ -199,6 +202,7 @@ pub(crate) async fn execute_exec_env(
|
||||
sandbox_permissions,
|
||||
justification,
|
||||
arg0,
|
||||
detach_from_tty,
|
||||
} = env;
|
||||
|
||||
let params = ExecParams {
|
||||
@@ -209,6 +213,7 @@ pub(crate) async fn execute_exec_env(
|
||||
sandbox_permissions,
|
||||
justification,
|
||||
arg0,
|
||||
detach_from_tty,
|
||||
};
|
||||
|
||||
let start = Instant::now();
|
||||
@@ -539,6 +544,7 @@ async fn exec(
|
||||
env,
|
||||
arg0,
|
||||
expiration,
|
||||
detach_from_tty,
|
||||
..
|
||||
} = params;
|
||||
|
||||
@@ -549,7 +555,7 @@ async fn exec(
|
||||
))
|
||||
})?;
|
||||
let arg0_ref = arg0.as_deref();
|
||||
let detach_from_tty = matches!(sandbox, SandboxType::None);
|
||||
let detach_from_tty = detach_from_tty && matches!(sandbox, SandboxType::None);
|
||||
let child = spawn_child_async(
|
||||
PathBuf::from(program),
|
||||
args.into(),
|
||||
@@ -852,6 +858,7 @@ mod tests {
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
justification: None,
|
||||
arg0: None,
|
||||
detach_from_tty: false,
|
||||
};
|
||||
|
||||
let output = exec(params, SandboxType::None, &SandboxPolicy::ReadOnly, None).await?;
|
||||
@@ -897,6 +904,7 @@ mod tests {
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
justification: None,
|
||||
arg0: None,
|
||||
detach_from_tty: false,
|
||||
};
|
||||
tokio::spawn(async move {
|
||||
tokio::time::sleep(Duration::from_millis(1_000)).await;
|
||||
|
||||
@@ -88,6 +88,8 @@ pub enum Feature {
|
||||
RemoteModels,
|
||||
/// Experimental shell snapshotting.
|
||||
ShellSnapshot,
|
||||
/// Detach non-interactive child processes from the controlling TTY.
|
||||
DetachNonTty,
|
||||
/// Append additional AGENTS.md guidance to user instructions.
|
||||
ChildAgentsMd,
|
||||
/// Experimental TUI v2 (viewport) implementation.
|
||||
@@ -358,6 +360,16 @@ pub const FEATURES: &[FeatureSpec] = &[
|
||||
},
|
||||
default_enabled: false,
|
||||
},
|
||||
FeatureSpec {
|
||||
id: Feature::DetachNonTty,
|
||||
key: "detach_non_tty",
|
||||
stage: Stage::Beta {
|
||||
name: "Detach non-PTY commands",
|
||||
menu_description: "Run non-interactive commands without inheriting the controlling TTY.",
|
||||
announcement: "NEW! Try detaching non-PTY commands to avoid TTY issues. Enable in /experimental!",
|
||||
},
|
||||
default_enabled: false,
|
||||
},
|
||||
FeatureSpec {
|
||||
id: Feature::ChildAgentsMd,
|
||||
key: "child_agents_md",
|
||||
|
||||
@@ -35,6 +35,7 @@ pub struct CommandSpec {
|
||||
pub expiration: ExecExpiration,
|
||||
pub sandbox_permissions: SandboxPermissions,
|
||||
pub justification: Option<String>,
|
||||
pub detach_from_tty: bool,
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
@@ -47,6 +48,7 @@ pub struct ExecEnv {
|
||||
pub sandbox_permissions: SandboxPermissions,
|
||||
pub justification: Option<String>,
|
||||
pub arg0: Option<String>,
|
||||
pub detach_from_tty: bool,
|
||||
}
|
||||
|
||||
pub enum SandboxPreference {
|
||||
@@ -163,6 +165,7 @@ impl SandboxManager {
|
||||
sandbox_permissions: spec.sandbox_permissions,
|
||||
justification: spec.justification,
|
||||
arg0: arg0_override,
|
||||
detach_from_tty: spec.detach_from_tty,
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -16,6 +16,7 @@ use crate::exec::StdoutStream;
|
||||
use crate::exec::StreamOutput;
|
||||
use crate::exec::execute_exec_env;
|
||||
use crate::exec_env::create_env;
|
||||
use crate::features::Feature;
|
||||
use crate::parse_command::parse_command;
|
||||
use crate::protocol::EventMsg;
|
||||
use crate::protocol::ExecCommandBeginEvent;
|
||||
@@ -111,6 +112,7 @@ impl SessionTask for UserShellCommandTask {
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
justification: None,
|
||||
arg0: None,
|
||||
detach_from_tty: session.features().enabled(Feature::DetachNonTty),
|
||||
};
|
||||
|
||||
let stdout_stream = Some(StdoutStream {
|
||||
|
||||
@@ -6,6 +6,7 @@ use std::sync::Arc;
|
||||
use crate::codex::TurnContext;
|
||||
use crate::exec::ExecParams;
|
||||
use crate::exec_env::create_env;
|
||||
use crate::features::Feature;
|
||||
use crate::function_tool::FunctionCallError;
|
||||
use crate::is_safe_command::is_known_safe_command;
|
||||
use crate::protocol::ExecCommandSource;
|
||||
@@ -29,7 +30,11 @@ pub struct ShellHandler;
|
||||
pub struct ShellCommandHandler;
|
||||
|
||||
impl ShellHandler {
|
||||
fn to_exec_params(params: ShellToolCallParams, turn_context: &TurnContext) -> ExecParams {
|
||||
fn to_exec_params(
|
||||
params: ShellToolCallParams,
|
||||
session: &crate::codex::Session,
|
||||
turn_context: &TurnContext,
|
||||
) -> ExecParams {
|
||||
ExecParams {
|
||||
command: params.command,
|
||||
cwd: turn_context.resolve_path(params.workdir.clone()),
|
||||
@@ -38,6 +43,7 @@ impl ShellHandler {
|
||||
sandbox_permissions: params.sandbox_permissions.unwrap_or_default(),
|
||||
justification: params.justification,
|
||||
arg0: None,
|
||||
detach_from_tty: session.features().enabled(Feature::DetachNonTty),
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -64,6 +70,7 @@ impl ShellCommandHandler {
|
||||
sandbox_permissions: params.sandbox_permissions.unwrap_or_default(),
|
||||
justification: params.justification,
|
||||
arg0: None,
|
||||
detach_from_tty: session.features().enabled(Feature::DetachNonTty),
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -106,7 +113,7 @@ impl ToolHandler for ShellHandler {
|
||||
match payload {
|
||||
ToolPayload::Function { arguments } => {
|
||||
let params: ShellToolCallParams = parse_arguments(&arguments)?;
|
||||
let exec_params = Self::to_exec_params(params, turn.as_ref());
|
||||
let exec_params = Self::to_exec_params(params, session.as_ref(), turn.as_ref());
|
||||
Self::run_exec_like(
|
||||
tool_name.as_str(),
|
||||
exec_params,
|
||||
@@ -119,7 +126,7 @@ impl ToolHandler for ShellHandler {
|
||||
.await
|
||||
}
|
||||
ToolPayload::LocalShell { params } => {
|
||||
let exec_params = Self::to_exec_params(params, turn.as_ref());
|
||||
let exec_params = Self::to_exec_params(params, session.as_ref(), turn.as_ref());
|
||||
Self::run_exec_like(
|
||||
tool_name.as_str(),
|
||||
exec_params,
|
||||
@@ -297,6 +304,7 @@ mod tests {
|
||||
|
||||
use crate::codex::make_session_and_context;
|
||||
use crate::exec_env::create_env;
|
||||
use crate::features::Feature;
|
||||
use crate::is_safe_command::is_known_safe_command;
|
||||
use crate::powershell::try_find_powershell_executable_blocking;
|
||||
use crate::powershell::try_find_pwsh_executable_blocking;
|
||||
@@ -387,6 +395,10 @@ mod tests {
|
||||
assert_eq!(exec_params.sandbox_permissions, sandbox_permissions);
|
||||
assert_eq!(exec_params.justification, justification);
|
||||
assert_eq!(exec_params.arg0, None);
|
||||
assert_eq!(
|
||||
exec_params.detach_from_tty,
|
||||
session.features().enabled(Feature::DetachNonTty)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -64,6 +64,7 @@ impl ApplyPatchRuntime {
|
||||
env: HashMap::new(),
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
justification: None,
|
||||
detach_from_tty: false,
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -25,6 +25,7 @@ pub(crate) fn build_command_spec(
|
||||
expiration: ExecExpiration,
|
||||
sandbox_permissions: SandboxPermissions,
|
||||
justification: Option<String>,
|
||||
detach_from_tty: bool,
|
||||
) -> Result<CommandSpec, ToolError> {
|
||||
let (program, args) = command
|
||||
.split_first()
|
||||
@@ -37,6 +38,7 @@ pub(crate) fn build_command_spec(
|
||||
expiration,
|
||||
sandbox_permissions,
|
||||
justification,
|
||||
detach_from_tty,
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -162,6 +162,7 @@ impl ToolRuntime<ShellRequest, ExecToolCallOutput> for ShellRuntime {
|
||||
req.timeout_ms.into(),
|
||||
req.sandbox_permissions,
|
||||
req.justification.clone(),
|
||||
ctx.session.features().enabled(Feature::DetachNonTty),
|
||||
)?;
|
||||
let env = attempt
|
||||
.env_for(spec)
|
||||
|
||||
@@ -181,6 +181,7 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
|
||||
command
|
||||
};
|
||||
|
||||
let detach_from_tty = ctx.session.features().enabled(Feature::DetachNonTty) && !req.tty;
|
||||
let spec = build_command_spec(
|
||||
&command,
|
||||
&req.cwd,
|
||||
@@ -188,6 +189,7 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
|
||||
ExecExpiration::DefaultTimeout,
|
||||
req.sandbox_permissions,
|
||||
req.justification.clone(),
|
||||
detach_from_tty,
|
||||
)
|
||||
.map_err(|_| ToolError::Rejected("missing command line for PTY".to_string()))?;
|
||||
let exec_env = attempt
|
||||
|
||||
@@ -471,7 +471,7 @@ impl UnifiedExecProcessManager {
|
||||
)
|
||||
.await
|
||||
} else {
|
||||
let detach_from_tty = matches!(env.sandbox, SandboxType::None);
|
||||
let detach_from_tty = env.detach_from_tty && matches!(env.sandbox, SandboxType::None);
|
||||
if detach_from_tty {
|
||||
codex_utils_pty::pipe::spawn_process_no_stdin_detached(
|
||||
program,
|
||||
|
||||
@@ -38,6 +38,7 @@ async fn run_test_cmd(tmp: TempDir, cmd: Vec<&str>) -> Result<ExecToolCallOutput
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
justification: None,
|
||||
arg0: None,
|
||||
detach_from_tty: false,
|
||||
};
|
||||
|
||||
let policy = SandboxPolicy::new_read_only_policy();
|
||||
|
||||
@@ -89,6 +89,7 @@ impl EscalateServer {
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
justification: None,
|
||||
arg0: None,
|
||||
detach_from_tty: false,
|
||||
},
|
||||
&sandbox_state.sandbox_policy,
|
||||
&sandbox_state.sandbox_cwd,
|
||||
|
||||
@@ -62,6 +62,7 @@ async fn run_cmd_output(
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
justification: None,
|
||||
arg0: None,
|
||||
detach_from_tty: false,
|
||||
};
|
||||
|
||||
let sandbox_policy = SandboxPolicy::WorkspaceWrite {
|
||||
@@ -179,6 +180,7 @@ async fn assert_network_blocked(cmd: &[&str]) {
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
justification: None,
|
||||
arg0: None,
|
||||
detach_from_tty: false,
|
||||
};
|
||||
|
||||
let sandbox_policy = SandboxPolicy::new_read_only_policy();
|
||||
|
||||
Reference in New Issue
Block a user