feat: POSIX unification and snapshot sessions (#3179)
## Session snapshot For POSIX shell, the goal is to take a snapshot of the interactive shell environment, store it in a session file located in `.codex/` and only source this file for every command that is run. As a result, if a snapshot files exist, `bash -lc <CALL>` get replaced by `bash -c <CALL>`. This also fixes the issue that `bash -lc` does not source `.bashrc`, resulting in missing env variables and aliases in the codex session. ## POSIX unification Unify `bash` and `zsh` shell into a POSIX shell. The rational is that the tool will not use any `zsh` specific capabilities. --------- Co-authored-by: Michael Bolin <mbolin@openai.com>
This commit is contained in:
@@ -405,7 +405,7 @@ impl Session {
|
||||
let rollout_fut = RolloutRecorder::new(&config, rollout_params);
|
||||
|
||||
let mcp_fut = McpConnectionManager::new(config.mcp_servers.clone());
|
||||
let default_shell_fut = shell::default_user_shell();
|
||||
let default_shell_fut = shell::default_user_shell(conversation_id.0, &config.codex_home);
|
||||
let history_meta_fut = crate::message_history::history_metadata(&config);
|
||||
|
||||
// Join all independent futures.
|
||||
@@ -477,6 +477,7 @@ impl Session {
|
||||
shell_environment_policy: config.shell_environment_policy.clone(),
|
||||
cwd,
|
||||
};
|
||||
|
||||
let sess = Arc::new(Session {
|
||||
conversation_id,
|
||||
tx_event: tx_event.clone(),
|
||||
@@ -2323,13 +2324,25 @@ pub struct ExecInvokeArgs<'a> {
|
||||
pub stdout_stream: Option<StdoutStream>,
|
||||
}
|
||||
|
||||
fn should_translate_shell_command(
|
||||
shell: &crate::shell::Shell,
|
||||
shell_policy: &ShellEnvironmentPolicy,
|
||||
) -> bool {
|
||||
matches!(shell, crate::shell::Shell::PowerShell(_))
|
||||
|| shell_policy.use_profile
|
||||
|| matches!(
|
||||
shell,
|
||||
crate::shell::Shell::Posix(shell) if shell.shell_snapshot.is_some()
|
||||
)
|
||||
}
|
||||
|
||||
fn maybe_translate_shell_command(
|
||||
params: ExecParams,
|
||||
sess: &Session,
|
||||
turn_context: &TurnContext,
|
||||
) -> ExecParams {
|
||||
let should_translate = matches!(sess.user_shell, crate::shell::Shell::PowerShell(_))
|
||||
|| turn_context.shell_environment_policy.use_profile;
|
||||
let should_translate =
|
||||
should_translate_shell_command(&sess.user_shell, &turn_context.shell_environment_policy);
|
||||
|
||||
if should_translate
|
||||
&& let Some(command) = sess
|
||||
@@ -2946,10 +2959,15 @@ fn convert_call_tool_result_to_function_call_output_payload(
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::config_types::ShellEnvironmentPolicyInherit;
|
||||
use mcp_types::ContentBlock;
|
||||
use mcp_types::TextContent;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
use shell::ShellSnapshot;
|
||||
use std::collections::HashMap;
|
||||
use std::path::PathBuf;
|
||||
use std::sync::Arc;
|
||||
use std::time::Duration as StdDuration;
|
||||
|
||||
fn text_block(s: &str) -> ContentBlock {
|
||||
@@ -2960,6 +2978,48 @@ mod tests {
|
||||
})
|
||||
}
|
||||
|
||||
fn shell_policy_with_profile(use_profile: bool) -> ShellEnvironmentPolicy {
|
||||
ShellEnvironmentPolicy {
|
||||
inherit: ShellEnvironmentPolicyInherit::All,
|
||||
ignore_default_excludes: false,
|
||||
exclude: Vec::new(),
|
||||
r#set: HashMap::new(),
|
||||
include_only: Vec::new(),
|
||||
use_profile,
|
||||
}
|
||||
}
|
||||
|
||||
fn zsh_shell(shell_snapshot: Option<Arc<ShellSnapshot>>) -> shell::Shell {
|
||||
shell::Shell::Posix(shell::PosixShell {
|
||||
shell_path: "/bin/zsh".to_string(),
|
||||
rc_path: "/Users/example/.zshrc".to_string(),
|
||||
shell_snapshot,
|
||||
})
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn translates_commands_when_shell_policy_requests_profile() {
|
||||
let policy = shell_policy_with_profile(true);
|
||||
let shell = zsh_shell(None);
|
||||
assert!(should_translate_shell_command(&shell, &policy));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn translates_commands_for_zsh_with_snapshot() {
|
||||
let policy = shell_policy_with_profile(false);
|
||||
let shell = zsh_shell(Some(Arc::new(ShellSnapshot::new(PathBuf::from(
|
||||
"/tmp/snapshot",
|
||||
)))));
|
||||
assert!(should_translate_shell_command(&shell, &policy));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn bypasses_translation_for_zsh_without_snapshot_or_profile() {
|
||||
let policy = shell_policy_with_profile(false);
|
||||
let shell = zsh_shell(None);
|
||||
assert!(!should_translate_shell_command(&shell, &policy));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn prefers_structured_content_when_present() {
|
||||
let ctr = CallToolResult {
|
||||
|
||||
Reference in New Issue
Block a user