mirror of
https://github.com/openai/codex.git
synced 2026-10-21 05:22:21 +00:00
Preserve saved permissions when resuming or forking remote tasks (#43330)
## Why Local permission settings could overwrite a remote task's saved server settings when resuming or forking, including its named permission profile. ## What changed - Omit permission overrides from remote resume requests and remote fork requests that inherit saved settings. - Reject explicit permission overrides when resuming or forking remote tasks, including overrides from session flags and selected configuration profiles. - Refresh restored runtime permission and approval overrides after a remote resume so settings from the previous task do not persist. - Carry the selected fork permission mode through retries after unarchiving. ## Testing Add regression coverage for restoring saved server permission profiles, approval policies, and approval reviewers across resume and fork; rejecting explicit remote resume overrides; and clearing stale restored runtime overrides. GitOrigin-RevId: 465c0444df380553e50ddbeeceff9dc64e5bf9c1
This commit is contained in:
@@ -398,6 +398,9 @@ impl App {
|
||||
}
|
||||
}
|
||||
};
|
||||
if unloaded && self.reject_remote_resume_permission_override(&resume_config) {
|
||||
return Ok(AppRunControl::Continue);
|
||||
}
|
||||
let baseline_approval = resume_config.permissions.approval_policy.value();
|
||||
let baseline_permissions =
|
||||
RuntimePermissionProfileOverride::from_config(&resume_config);
|
||||
|
||||
@@ -550,7 +550,7 @@ impl App {
|
||||
},
|
||||
})
|
||||
.await;
|
||||
let Ok(ThreadReadResponse { thread }) = thread else {
|
||||
let Ok(ThreadReadResponse { thread, .. }) = thread else {
|
||||
return;
|
||||
};
|
||||
let SessionSource::SubAgent(SubAgentSource::ThreadSpawn {
|
||||
|
||||
@@ -52,6 +52,39 @@ pub(super) fn resume_model_settings_for_overrides(
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) fn has_explicit_resume_permission_override(
|
||||
config: &Config,
|
||||
overrides: &ConfigOverrides,
|
||||
) -> bool {
|
||||
overrides.approval_policy.is_some()
|
||||
|| overrides.approvals_reviewer.is_some()
|
||||
|| overrides.sandbox_mode.is_some()
|
||||
|| overrides.permission_profile.is_some()
|
||||
|| overrides.default_permissions.is_some()
|
||||
|| !overrides.additional_writable_roots.is_empty()
|
||||
|| overrides.workspace_roots.is_some()
|
||||
|| config.config_layer_stack.layers_high_to_low().any(|layer| {
|
||||
matches!(
|
||||
&layer.name,
|
||||
ConfigLayerSource::SessionFlags
|
||||
| ConfigLayerSource::User {
|
||||
profile: Some(_),
|
||||
..
|
||||
}
|
||||
) && [
|
||||
"approval_policy",
|
||||
"approvals_reviewer",
|
||||
"sandbox_mode",
|
||||
"default_permissions",
|
||||
"permissions",
|
||||
"network",
|
||||
"sandbox_workspace_write",
|
||||
]
|
||||
.iter()
|
||||
.any(|key| layer.config.get(*key).is_some())
|
||||
})
|
||||
}
|
||||
|
||||
impl App {
|
||||
pub(super) async fn rebuild_config_for_cwd(&self, cwd: PathBuf) -> Result<Config> {
|
||||
let mut overrides = self.harness_overrides.clone();
|
||||
@@ -881,6 +914,33 @@ impl App {
|
||||
resume_model_settings_for_overrides(&self.config, &self.harness_overrides)
|
||||
}
|
||||
|
||||
pub(super) fn reject_remote_resume_permission_override(&mut self, config: &Config) -> bool {
|
||||
if self.app_server_target.thread_params_mode()
|
||||
!= crate::app_server_session::ThreadParamsMode::Remote
|
||||
{
|
||||
return false;
|
||||
}
|
||||
let explicitly_selected =
|
||||
has_explicit_resume_permission_override(config, &self.harness_overrides)
|
||||
|| matches!(
|
||||
self.runtime_approval_policy_override,
|
||||
Some(RuntimeApprovalPolicyOverride::Explicit(_))
|
||||
)
|
||||
|| self
|
||||
.runtime_permission_profile_override
|
||||
.as_ref()
|
||||
.is_some_and(|profile| {
|
||||
profile.turn_override == RuntimePermissionProfileTurnOverride::LegacySandbox
|
||||
});
|
||||
if explicitly_selected {
|
||||
self.chat_widget.add_error_message(
|
||||
"Permission overrides are not supported when resuming a remote task.".into(),
|
||||
);
|
||||
return true;
|
||||
}
|
||||
false
|
||||
}
|
||||
|
||||
pub(super) fn on_update_personality(&mut self, personality: Personality) {
|
||||
self.config.personality = Some(personality);
|
||||
self.chat_widget.set_personality(personality);
|
||||
|
||||
@@ -1146,6 +1146,11 @@ impl App {
|
||||
Ok(config) => config,
|
||||
Err(control) => return Ok(control),
|
||||
};
|
||||
if self.reject_remote_resume_permission_override(&resume_config) {
|
||||
return Ok(AppRunControl::Continue);
|
||||
}
|
||||
let baseline_approval = resume_config.permissions.approval_policy.value();
|
||||
let baseline_permissions = RuntimePermissionProfileOverride::from_config(&resume_config);
|
||||
self.apply_runtime_policy_overrides(&mut resume_config, RuntimePolicyOverrideScope::All);
|
||||
|
||||
let summary = session_summary(
|
||||
@@ -1229,6 +1234,15 @@ impl App {
|
||||
.mark_external_writer();
|
||||
self.chat_widget.show_external_writer_thread();
|
||||
}
|
||||
if self.app_server_target.uses_remote_workspace() {
|
||||
let config = self.chat_widget.config_ref();
|
||||
let approval = config.permissions.approval_policy.value();
|
||||
self.runtime_approval_policy_override = (approval != baseline_approval)
|
||||
.then_some(RuntimeApprovalPolicyOverride::Restored(approval.into()));
|
||||
self.runtime_permission_profile_override = (!baseline_permissions
|
||||
.matches_config(config))
|
||||
.then(|| RuntimePermissionProfileOverride::from_restored_config(config));
|
||||
}
|
||||
self.backfill_loaded_subagent_threads(app_server).await;
|
||||
if !read_only {
|
||||
self.replay_agents_overview_requests(app_server, resumed_thread_id)
|
||||
|
||||
@@ -432,6 +432,21 @@ impl App {
|
||||
(chat_widget, None)
|
||||
}
|
||||
SessionSelection::Resume(target_session) => {
|
||||
if app_server_target.thread_params_mode()
|
||||
== crate::app_server_session::ThreadParamsMode::Remote
|
||||
&& config_persistence::has_explicit_resume_permission_override(
|
||||
&config,
|
||||
&harness_overrides,
|
||||
)
|
||||
{
|
||||
return shutdown_on_startup_error(
|
||||
app_server,
|
||||
color_eyre::eyre::eyre!(
|
||||
"Permission overrides are not supported when resuming a remote task."
|
||||
),
|
||||
)
|
||||
.await;
|
||||
}
|
||||
if let Some(history_mode) = target_session.history_mode {
|
||||
app_server.remember_thread_history_mode(target_session.thread_id, history_mode);
|
||||
}
|
||||
@@ -528,6 +543,28 @@ impl App {
|
||||
(ChatWidget::new_with_app_event(init), Some(resumed))
|
||||
}
|
||||
SessionSelection::Fork(target_session) => {
|
||||
let explicit_permission_override =
|
||||
config_persistence::has_explicit_resume_permission_override(
|
||||
&config,
|
||||
&harness_overrides,
|
||||
);
|
||||
if explicit_permission_override
|
||||
&& app_server_target.thread_params_mode()
|
||||
== crate::app_server_session::ThreadParamsMode::Remote
|
||||
{
|
||||
return shutdown_on_startup_error(
|
||||
app_server,
|
||||
color_eyre::eyre::eyre!(
|
||||
"Permission overrides are not supported when forking a remote task."
|
||||
),
|
||||
)
|
||||
.await;
|
||||
}
|
||||
let permission_mode = if explicit_permission_override {
|
||||
crate::app_server_session::ForkPermissionMode::OverrideFromCurrentConfig
|
||||
} else {
|
||||
crate::app_server_session::ForkPermissionMode::InheritSaved
|
||||
};
|
||||
session_telemetry.counter(
|
||||
"codex.thread.fork",
|
||||
/*inc*/ 1,
|
||||
@@ -536,10 +573,11 @@ impl App {
|
||||
let forked = match startup_draft
|
||||
.run_until(
|
||||
tui,
|
||||
app_server.fork_thread(
|
||||
app_server.fork_thread_with_permission_mode(
|
||||
&local_settings,
|
||||
config.clone(),
|
||||
target_session.thread_id,
|
||||
permission_mode,
|
||||
),
|
||||
)
|
||||
.await
|
||||
@@ -547,7 +585,7 @@ impl App {
|
||||
Ok(forked) => forked,
|
||||
Err(err) => return shutdown_on_startup_error(app_server, err).await,
|
||||
};
|
||||
let action = SessionStartAction::Fork;
|
||||
let action = SessionStartAction::Fork(permission_mode);
|
||||
let Some(forked) = complete_session_start(
|
||||
&mut app_server,
|
||||
&config,
|
||||
|
||||
@@ -2896,6 +2896,91 @@ async fn reset_memories_clears_local_memory_directories() -> Result<()> {
|
||||
.await
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn resume_rejects_selected_config_profile_permission_definitions() -> Result<()> {
|
||||
let home = tempdir()?;
|
||||
std::fs::write(
|
||||
home.path().join("config.toml"),
|
||||
"default_permissions = \":workspace\"\n",
|
||||
)?;
|
||||
let selected = home.path().join("work.config.toml");
|
||||
std::fs::write(
|
||||
&selected,
|
||||
"default_permissions = \":workspace\"\n[permissions.unused]\nextends = \":read-only\"\n",
|
||||
)?;
|
||||
let mut loader = codex_config::LoaderOverrides::without_managed_config_for_tests();
|
||||
loader.user_config_path = Some(selected.abs());
|
||||
loader.user_config_profile = Some("work".parse()?);
|
||||
let config = ConfigBuilder::default()
|
||||
.codex_home(home.path().to_path_buf())
|
||||
.loader_overrides(loader)
|
||||
.build()
|
||||
.await?;
|
||||
assert!(config_persistence::has_explicit_resume_permission_override(
|
||||
&config,
|
||||
&ConfigOverrides::default()
|
||||
));
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn remote_resume_rejects_explicit_permission_override() -> Result<()> {
|
||||
let (mut app, mut events, _ops) = make_test_app_with_channels().await;
|
||||
let endpoint = crate::resolve_remote_addr("ws://127.0.0.1:8765")?;
|
||||
app.app_server_target = crate::AppServerTarget::Remote { endpoint };
|
||||
let home = tempdir()?;
|
||||
std::fs::write(
|
||||
home.path().join("config.toml"),
|
||||
"default_permissions = \":workspace\"\n",
|
||||
)?;
|
||||
app.config.codex_home = home.path().to_path_buf().abs();
|
||||
let mut server = crate::start_embedded_app_server_for_picker(&app.config).await?;
|
||||
let selected = home.path().join("work.config.toml");
|
||||
std::fs::write(
|
||||
&selected,
|
||||
"[permissions.unused]\nextends = \":read-only\"\n",
|
||||
)?;
|
||||
app.loader_overrides.user_config_path = Some(selected.abs());
|
||||
app.loader_overrides.user_config_profile = Some("work".parse()?);
|
||||
assert!(
|
||||
!config_persistence::has_explicit_resume_permission_override(
|
||||
&app.config,
|
||||
&app.harness_overrides
|
||||
)
|
||||
);
|
||||
let refreshed = app
|
||||
.rebuild_config_for_cwd(app.config.cwd.to_path_buf())
|
||||
.await?;
|
||||
assert!(config_persistence::has_explicit_resume_permission_override(
|
||||
&refreshed,
|
||||
&app.harness_overrides
|
||||
));
|
||||
let mut tui = crate::tui::test_support::make_test_tui()?;
|
||||
app.resume_target_session(
|
||||
&mut tui,
|
||||
&mut server,
|
||||
SessionTarget {
|
||||
thread_id: ThreadId::new(),
|
||||
path: None,
|
||||
history_mode: None,
|
||||
},
|
||||
)
|
||||
.await?;
|
||||
assert_eq!(app.active_thread_id, None);
|
||||
let cell = std::iter::from_fn(|| events.try_recv().ok())
|
||||
.find_map(|event| match event {
|
||||
AppEvent::InsertHistoryCell(cell) => Some(cell),
|
||||
_ => None,
|
||||
})
|
||||
.expect("permission override error");
|
||||
insta::assert_snapshot!(
|
||||
lines_to_single_string(&cell.display_lines(/*width*/ 80)),
|
||||
@"■ Permission overrides are not supported when resuming a remote task."
|
||||
);
|
||||
server.shutdown().await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn apply_permission_profile_selection_preserves_loader_overrides() -> Result<()> {
|
||||
let (mut app, mut app_event_rx, _op_rx) = make_test_app_with_channels().await;
|
||||
@@ -7169,7 +7254,25 @@ async fn remote_resume_keeps_server_only_cwd_out_of_local_config() -> Result<()>
|
||||
let mut app_server = Box::pin(crate::start_embedded_app_server_for_picker(&app.config))
|
||||
.await?
|
||||
.with_remote_cwd_override(Some(remote_cwd.clone()));
|
||||
app_server
|
||||
.resume_thread(
|
||||
&crate::local_settings::LocalSettings::from(&app.config),
|
||||
app.config.clone(),
|
||||
ThreadId::from_string(&thread_id)?,
|
||||
crate::app_server_session::ResumeModelSettings::RestoreFromThread,
|
||||
)
|
||||
.await?;
|
||||
let mut tui = crate::tui::test_support::make_test_tui()?;
|
||||
app.runtime_approval_policy_override = Some(RuntimeApprovalPolicyOverride::Restored(
|
||||
AskForApproval::Never,
|
||||
));
|
||||
let mut prior_config = app.config.clone();
|
||||
prior_config
|
||||
.permissions
|
||||
.set_permission_profile(PermissionProfile::read_only())?;
|
||||
app.runtime_permission_profile_override = Some(
|
||||
RuntimePermissionProfileOverride::from_restored_config(&prior_config),
|
||||
);
|
||||
|
||||
let control = app
|
||||
.resume_target_session(
|
||||
@@ -7190,6 +7293,8 @@ async fn remote_resume_keeps_server_only_cwd_out_of_local_config() -> Result<()>
|
||||
&local_cwd,
|
||||
));
|
||||
assert_eq!(app.config.workspace_roots, local_workspace_roots);
|
||||
assert_eq!(app.runtime_approval_policy_override, None);
|
||||
assert!(app.runtime_permission_profile_override.is_none());
|
||||
app_server.shutdown().await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -169,6 +169,12 @@ pub(crate) enum ForkGoalContinuation {
|
||||
DeferUntilNextTurn,
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
|
||||
pub(crate) enum ForkPermissionMode {
|
||||
InheritSaved,
|
||||
OverrideFromCurrentConfig,
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
|
||||
enum ForkPresentation {
|
||||
Regular,
|
||||
@@ -830,13 +836,31 @@ impl AppServerSession {
|
||||
config: Config,
|
||||
thread_id: ThreadId,
|
||||
) -> Result<AppServerStartedThread> {
|
||||
self.fork_thread_at(
|
||||
self.fork_thread_with_permission_mode(
|
||||
local_settings,
|
||||
config,
|
||||
thread_id,
|
||||
ForkPermissionMode::InheritSaved,
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
pub(crate) async fn fork_thread_with_permission_mode(
|
||||
&mut self,
|
||||
local_settings: &LocalSettings,
|
||||
config: Config,
|
||||
thread_id: ThreadId,
|
||||
permission_mode: ForkPermissionMode,
|
||||
) -> Result<AppServerStartedThread> {
|
||||
self.fork_thread_at_with_presentation(
|
||||
local_settings,
|
||||
config,
|
||||
thread_id,
|
||||
/*last_turn_id*/ None,
|
||||
/*before_turn_id*/ None,
|
||||
ForkGoalContinuation::StartIfIdle,
|
||||
ForkPresentation::Regular,
|
||||
permission_mode,
|
||||
)
|
||||
.await
|
||||
}
|
||||
@@ -858,6 +882,7 @@ impl AppServerSession {
|
||||
before_turn_id,
|
||||
goal_continuation,
|
||||
ForkPresentation::Regular,
|
||||
ForkPermissionMode::InheritSaved,
|
||||
)
|
||||
.await
|
||||
}
|
||||
@@ -876,6 +901,7 @@ impl AppServerSession {
|
||||
/*before_turn_id*/ None,
|
||||
ForkGoalContinuation::StartIfIdle,
|
||||
ForkPresentation::SideConversation,
|
||||
ForkPermissionMode::InheritSaved,
|
||||
)
|
||||
.await
|
||||
}
|
||||
@@ -893,6 +919,7 @@ impl AppServerSession {
|
||||
before_turn_id: Option<String>,
|
||||
goal_continuation: ForkGoalContinuation,
|
||||
presentation: ForkPresentation,
|
||||
permission_mode: ForkPermissionMode,
|
||||
) -> Result<AppServerStartedThread> {
|
||||
let fork_parent = match presentation {
|
||||
ForkPresentation::Regular => self
|
||||
@@ -920,6 +947,15 @@ impl AppServerSession {
|
||||
self.remote_cwd_override.as_deref(),
|
||||
)
|
||||
};
|
||||
if self.thread_params_mode() == ThreadParamsMode::Remote
|
||||
&& permission_mode == ForkPermissionMode::InheritSaved
|
||||
{
|
||||
params.approval_policy = None;
|
||||
params.approvals_reviewer = None;
|
||||
params.sandbox = None;
|
||||
params.permissions = None;
|
||||
remove_permission_config_overrides(&mut params.config);
|
||||
}
|
||||
self.thread_tool_transport()
|
||||
.configure_mcp(&mut params.config);
|
||||
let response: ThreadForkResponse = match self
|
||||
@@ -1816,6 +1852,22 @@ fn config_request_overrides_from_config(
|
||||
Some(overrides)
|
||||
}
|
||||
|
||||
fn remove_permission_config_overrides(config: &mut Option<HashMap<String, serde_json::Value>>) {
|
||||
if let Some(overrides) = config.as_mut() {
|
||||
for key in [
|
||||
"default_permissions",
|
||||
"permissions",
|
||||
"network",
|
||||
"sandbox_workspace_write",
|
||||
] {
|
||||
overrides.remove(key);
|
||||
}
|
||||
}
|
||||
if config.as_ref().is_some_and(HashMap::is_empty) {
|
||||
*config = None;
|
||||
}
|
||||
}
|
||||
|
||||
fn service_tier_override_from_config(config: &Config) -> Option<Option<String>> {
|
||||
let local_settings = LocalSettings::from(config);
|
||||
config.service_tier.clone().map(Some).or_else(|| {
|
||||
@@ -2001,7 +2053,7 @@ fn thread_resume_params_from_config(
|
||||
(None, None)
|
||||
}
|
||||
};
|
||||
ThreadResumeParams {
|
||||
let mut params = ThreadResumeParams {
|
||||
thread_id: thread_id.to_string(),
|
||||
model,
|
||||
model_provider,
|
||||
@@ -2017,7 +2069,16 @@ fn thread_resume_params_from_config(
|
||||
&config, /*control_instructions*/ None,
|
||||
),
|
||||
..ThreadResumeParams::default()
|
||||
};
|
||||
if thread_params_mode == ThreadParamsMode::Remote {
|
||||
// Resuming restores the server's saved permission settings, including named profiles.
|
||||
params.approval_policy = None;
|
||||
params.approvals_reviewer = None;
|
||||
params.sandbox = None;
|
||||
params.permissions = None;
|
||||
remove_permission_config_overrides(&mut params.config);
|
||||
}
|
||||
params
|
||||
}
|
||||
|
||||
fn thread_fork_params_from_config(
|
||||
@@ -3052,7 +3113,9 @@ mod tests {
|
||||
assert_eq!(resume.model_provider, None);
|
||||
assert_eq!(fork.model_provider, None);
|
||||
assert_eq!(start.sandbox, expected_sandbox);
|
||||
assert_eq!(resume.sandbox, expected_sandbox);
|
||||
assert_eq!(resume.sandbox, None);
|
||||
assert_eq!(resume.approval_policy, None);
|
||||
assert_eq!(resume.approvals_reviewer, None);
|
||||
assert_eq!(fork.sandbox, expected_sandbox);
|
||||
assert_eq!(start.permissions, None);
|
||||
assert_eq!(resume.permissions, None);
|
||||
@@ -3062,7 +3125,7 @@ mod tests {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn remote_resume_params_keep_local_roots_with_cross_platform_cwd_override() {
|
||||
async fn remote_resume_params_keep_cwd_without_overriding_saved_permissions() {
|
||||
let temp_dir = tempfile::tempdir().expect("tempdir");
|
||||
let config = build_config(&temp_dir).await;
|
||||
let expected_workspace_roots = config.workspace_roots.clone();
|
||||
@@ -3192,7 +3255,7 @@ mod tests {
|
||||
assert_eq!(resume.model_provider, None);
|
||||
assert_eq!(fork.model_provider, None);
|
||||
assert_eq!(start.sandbox, expected_sandbox);
|
||||
assert_eq!(resume.sandbox, expected_sandbox);
|
||||
assert_eq!(resume.sandbox, None);
|
||||
assert_eq!(fork.sandbox, expected_sandbox);
|
||||
assert_eq!(start.permissions, None);
|
||||
assert_eq!(resume.permissions, None);
|
||||
|
||||
@@ -1,4 +1,6 @@
|
||||
use super::super::ForkPermissionMode;
|
||||
use super::super::ResumeModelSettings;
|
||||
use super::super::ThreadParamsMode;
|
||||
use crate::legacy_core::config::Config;
|
||||
use crate::legacy_core::config::ConfigBuilder;
|
||||
use app_test_support::create_fake_paginated_rollout;
|
||||
@@ -19,6 +21,159 @@ async fn build_config(temp_dir: &TempDir) -> Config {
|
||||
.expect("config should build")
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn remote_resume_restores_saved_server_profile_without_permission_overrides() -> Result<()> {
|
||||
let home = tempfile::tempdir()?;
|
||||
std::fs::write(
|
||||
home.path().join("config.toml"),
|
||||
"default_permissions = \":workspace\"\n[permissions.server-only]\nextends = \":read-only\"\n",
|
||||
)?;
|
||||
let config = build_config(&home).await;
|
||||
let client_config = ConfigBuilder::default()
|
||||
.codex_home(home.path().to_path_buf())
|
||||
.cli_overrides(vec![(
|
||||
"default_permissions".to_string(),
|
||||
":workspace".into(),
|
||||
)])
|
||||
.harness_overrides(crate::legacy_core::config::ConfigOverrides {
|
||||
default_permissions: Some(":read-only".into()),
|
||||
..Default::default()
|
||||
})
|
||||
.build()
|
||||
.await?;
|
||||
let thread_id = ThreadId::from_string(
|
||||
&create_fake_rollout(
|
||||
home.path(),
|
||||
"2025-01-05T12-00-00",
|
||||
"2025-01-05T12:00:00Z",
|
||||
"Saved user message",
|
||||
Some(config.model_provider_id.as_str()),
|
||||
/*git_info*/ None,
|
||||
)
|
||||
.expect("create source rollout"),
|
||||
)?;
|
||||
let mut server = crate::start_embedded_app_server_for_picker(&config).await?;
|
||||
server.thread_params_mode = ThreadParamsMode::Remote;
|
||||
let local_settings = crate::local_settings::LocalSettings::from(&client_config);
|
||||
let initial = server
|
||||
.resume_thread(
|
||||
&local_settings,
|
||||
client_config.clone(),
|
||||
thread_id,
|
||||
ResumeModelSettings::RestoreFromThread,
|
||||
)
|
||||
.await?;
|
||||
assert_eq!(
|
||||
initial
|
||||
.session
|
||||
.active_permission_profile
|
||||
.map(|profile| profile.id),
|
||||
Some(":workspace".to_string())
|
||||
);
|
||||
server
|
||||
.thread_settings_update(codex_app_server_protocol::ThreadSettingsUpdateParams {
|
||||
thread_id: thread_id.to_string(),
|
||||
permissions: Some("server-only".into()),
|
||||
approval_policy: Some(codex_app_server_protocol::AskForApproval::Never),
|
||||
approvals_reviewer: Some(codex_app_server_protocol::ApprovalsReviewer::AutoReview),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
tokio::time::timeout(std::time::Duration::from_secs(5), async {
|
||||
loop {
|
||||
if let Some(codex_app_server_client::AppServerEvent::ServerNotification(notification)) =
|
||||
server.next_event().await
|
||||
&& let codex_app_server_protocol::ServerNotification::ThreadSettingsUpdated(
|
||||
settings,
|
||||
) = *notification
|
||||
&& settings.thread_id == thread_id.to_string()
|
||||
&& settings
|
||||
.thread_settings
|
||||
.active_permission_profile
|
||||
.as_ref()
|
||||
.is_some_and(|profile| profile.id == "server-only")
|
||||
{
|
||||
break;
|
||||
}
|
||||
}
|
||||
})
|
||||
.await?;
|
||||
server.shutdown().await?;
|
||||
|
||||
let mut server = crate::start_embedded_app_server_for_picker(&config).await?;
|
||||
server.thread_params_mode = ThreadParamsMode::Remote;
|
||||
let resumed = server
|
||||
.resume_thread(
|
||||
&local_settings,
|
||||
client_config.clone(),
|
||||
thread_id,
|
||||
ResumeModelSettings::RestoreFromThread,
|
||||
)
|
||||
.await?;
|
||||
assert_eq!(
|
||||
resumed
|
||||
.session
|
||||
.active_permission_profile
|
||||
.as_ref()
|
||||
.map(|profile| profile.id.as_str()),
|
||||
Some("server-only")
|
||||
);
|
||||
assert_eq!(
|
||||
resumed.session.approval_policy,
|
||||
codex_app_server_protocol::AskForApproval::Never
|
||||
);
|
||||
assert_eq!(
|
||||
resumed.session.approvals_reviewer,
|
||||
codex_protocol::config_types::ApprovalsReviewer::AutoReview
|
||||
);
|
||||
let forked = server
|
||||
.fork_thread(&local_settings, client_config, thread_id)
|
||||
.await?;
|
||||
assert_eq!(
|
||||
forked
|
||||
.session
|
||||
.active_permission_profile
|
||||
.as_ref()
|
||||
.map(|profile| profile.id.as_str()),
|
||||
Some("server-only")
|
||||
);
|
||||
assert_eq!(
|
||||
forked.session.approval_policy,
|
||||
codex_app_server_protocol::AskForApproval::Never
|
||||
);
|
||||
assert_eq!(
|
||||
forked.session.approvals_reviewer,
|
||||
codex_protocol::config_types::ApprovalsReviewer::AutoReview
|
||||
);
|
||||
let explicit_config = ConfigBuilder::default()
|
||||
.codex_home(home.path().to_path_buf())
|
||||
.harness_overrides(crate::legacy_core::config::ConfigOverrides {
|
||||
sandbox_mode: Some(codex_protocol::config_types::SandboxMode::ReadOnly),
|
||||
approval_policy: Some(codex_protocol::protocol::AskForApproval::OnRequest),
|
||||
..Default::default()
|
||||
})
|
||||
.build()
|
||||
.await?;
|
||||
let explicit_fork = server
|
||||
.fork_thread_with_permission_mode(
|
||||
&local_settings,
|
||||
explicit_config,
|
||||
thread_id,
|
||||
ForkPermissionMode::OverrideFromCurrentConfig,
|
||||
)
|
||||
.await?;
|
||||
assert_eq!(
|
||||
explicit_fork.session.approval_policy,
|
||||
codex_app_server_protocol::AskForApproval::OnRequest
|
||||
);
|
||||
assert_eq!(
|
||||
explicit_fork.session.permission_profile,
|
||||
codex_protocol::models::PermissionProfile::read_only()
|
||||
);
|
||||
server.shutdown().await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn viewing_thread_reads_history_without_resuming_it() -> Result<()> {
|
||||
let codex_home = tempfile::tempdir()?;
|
||||
|
||||
@@ -4,6 +4,7 @@ use crate::app::AppExitInfo;
|
||||
use crate::app::ExitReason;
|
||||
use crate::app_server_session::AppServerSession;
|
||||
use crate::app_server_session::AppServerStartedThread;
|
||||
use crate::app_server_session::ForkPermissionMode;
|
||||
use crate::app_server_session::ResumeModelSettings;
|
||||
use crate::legacy_core::config::Config;
|
||||
use crate::resume_picker::SessionTarget;
|
||||
@@ -14,14 +15,14 @@ use color_eyre::eyre::WrapErr;
|
||||
#[derive(Clone, Copy)]
|
||||
pub(crate) enum SessionStartAction {
|
||||
Resume(ResumeModelSettings),
|
||||
Fork,
|
||||
Fork(ForkPermissionMode),
|
||||
}
|
||||
|
||||
impl SessionStartAction {
|
||||
pub(crate) fn verb(self) -> &'static str {
|
||||
match self {
|
||||
Self::Resume(_) => "resume",
|
||||
Self::Fork => "fork",
|
||||
Self::Fork(_) => "fork",
|
||||
}
|
||||
}
|
||||
|
||||
@@ -38,9 +39,14 @@ impl SessionStartAction {
|
||||
.resume_thread(&local_settings, config.clone(), target.thread_id, settings)
|
||||
.await
|
||||
}
|
||||
Self::Fork => {
|
||||
Self::Fork(permission_mode) => {
|
||||
app_server
|
||||
.fork_thread(&local_settings, config.clone(), target.thread_id)
|
||||
.fork_thread_with_permission_mode(
|
||||
&local_settings,
|
||||
config.clone(),
|
||||
target.thread_id,
|
||||
permission_mode,
|
||||
)
|
||||
.await
|
||||
}
|
||||
}
|
||||
|
||||
@@ -23,7 +23,7 @@ async fn start_session(
|
||||
async fn archived_session_requires_confirmation_before_resume_or_fork() -> Result<()> {
|
||||
for action in [
|
||||
SessionStartAction::Resume(ResumeModelSettings::RestoreFromThread),
|
||||
SessionStartAction::Fork,
|
||||
SessionStartAction::Fork(crate::app_server_session::ForkPermissionMode::InheritSaved),
|
||||
] {
|
||||
let codex_home = TempDir::new()?;
|
||||
let config = ConfigBuilder::default()
|
||||
@@ -79,7 +79,7 @@ async fn archived_session_requires_confirmation_before_resume_or_fork() -> Resul
|
||||
SessionStartAction::Resume(_) => {
|
||||
assert_eq!(started.session.thread_id, target.thread_id)
|
||||
}
|
||||
SessionStartAction::Fork => {
|
||||
SessionStartAction::Fork(_) => {
|
||||
assert_ne!(started.session.thread_id, target.thread_id);
|
||||
assert_eq!(
|
||||
app_server
|
||||
|
||||
@@ -46,7 +46,9 @@ fn resume_prompt_snapshot() {
|
||||
|
||||
#[test]
|
||||
fn fork_prompt_cancel_snapshot() {
|
||||
let mut screen = prompt(SessionStartAction::Fork);
|
||||
let mut screen = prompt(SessionStartAction::Fork(
|
||||
crate::app_server_session::ForkPermissionMode::InheritSaved,
|
||||
));
|
||||
assert_eq!(
|
||||
screen.handle_key(KeyEvent::new(KeyCode::Down, KeyModifiers::NONE)),
|
||||
None
|
||||
@@ -68,7 +70,9 @@ fn fork_prompt_cancel_snapshot() {
|
||||
|
||||
#[test]
|
||||
fn confirmation_requires_an_accept_key() {
|
||||
let mut screen = prompt(SessionStartAction::Fork);
|
||||
let mut screen = prompt(SessionStartAction::Fork(
|
||||
crate::app_server_session::ForkPermissionMode::InheritSaved,
|
||||
));
|
||||
for key in [
|
||||
KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE),
|
||||
KeyEvent::new(KeyCode::Char('n'), KeyModifiers::NONE),
|
||||
@@ -92,7 +96,9 @@ fn confirmation_requires_an_accept_key() {
|
||||
}
|
||||
|
||||
fn render_inline(width: u16) -> String {
|
||||
let screen = prompt(SessionStartAction::Fork);
|
||||
let screen = prompt(SessionStartAction::Fork(
|
||||
crate::app_server_session::ForkPermissionMode::InheritSaved,
|
||||
));
|
||||
let height = screen.content().desired_height(width);
|
||||
let mut terminal = Terminal::new(VT100Backend::new(width, height + 2)).unwrap();
|
||||
terminal
|
||||
@@ -133,7 +139,9 @@ async fn inline_prompt_only_claims_content_height() -> Result<()> {
|
||||
/*x*/ 0, /*y*/ 3, /*width*/ 80, /*height*/ 0,
|
||||
);
|
||||
tui.terminal.set_viewport_area(original);
|
||||
let screen = prompt(SessionStartAction::Fork);
|
||||
let screen = prompt(SessionStartAction::Fork(
|
||||
crate::app_server_session::ForkPermissionMode::InheritSaved,
|
||||
));
|
||||
{
|
||||
let mut guard = PromptScreenGuard::enter(&mut tui)?;
|
||||
guard.draw(&screen)?;
|
||||
@@ -159,7 +167,9 @@ async fn alternate_prompt_restores_original_viewport() -> Result<()> {
|
||||
tui.terminal.set_viewport_area(original);
|
||||
{
|
||||
let mut guard = PromptScreenGuard::enter(&mut tui)?;
|
||||
guard.draw(&prompt(SessionStartAction::Fork))?;
|
||||
guard.draw(&prompt(SessionStartAction::Fork(
|
||||
crate::app_server_session::ForkPermissionMode::InheritSaved,
|
||||
)))?;
|
||||
assert!(guard.tui.is_alt_screen_active());
|
||||
assert_eq!(
|
||||
guard.tui.terminal.viewport_area,
|
||||
|
||||
Reference in New Issue
Block a user