mirror of
https://github.com/openai/codex.git
synced 2026-11-11 21:10:39 +00:00
Merge 448f71e7d0 into sapling-pr-archive-bolinfest
This commit is contained in:
@@ -93,6 +93,8 @@ pub struct ConfigRequirements {
|
||||
pub network: Option<Sourced<NetworkConstraints>>,
|
||||
/// Managed filesystem constraints derived from requirements.
|
||||
pub filesystem: Option<Sourced<FilesystemConstraints>>,
|
||||
/// Source for the managed guardian policy config, when one is configured.
|
||||
pub guardian_policy_config_source: Option<RequirementSource>,
|
||||
}
|
||||
|
||||
impl Default for ConfigRequirements {
|
||||
@@ -123,6 +125,7 @@ impl Default for ConfigRequirements {
|
||||
),
|
||||
network: None,
|
||||
filesystem: None,
|
||||
guardian_policy_config_source: None,
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -887,7 +890,7 @@ impl TryFrom<ConfigRequirementsWithSources> for ConfigRequirements {
|
||||
enforce_residency,
|
||||
network,
|
||||
permissions,
|
||||
guardian_policy_config: _guardian_policy_config,
|
||||
guardian_policy_config,
|
||||
} = toml;
|
||||
|
||||
let approval_policy = match allowed_approval_policies {
|
||||
@@ -1096,6 +1099,7 @@ impl TryFrom<ConfigRequirementsWithSources> for ConfigRequirements {
|
||||
let Sourced { value, source } = sourced_permissions;
|
||||
Sourced::new(FilesystemConstraints::from(value), source)
|
||||
});
|
||||
let guardian_policy_config_source = guardian_policy_config.map(|sourced| sourced.source);
|
||||
Ok(ConfigRequirements {
|
||||
approval_policy,
|
||||
approvals_reviewer,
|
||||
@@ -1107,6 +1111,7 @@ impl TryFrom<ConfigRequirementsWithSources> for ConfigRequirements {
|
||||
enforce_residency,
|
||||
network,
|
||||
filesystem,
|
||||
guardian_policy_config_source,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1578,6 +1578,7 @@ impl Config {
|
||||
enforce_residency,
|
||||
network: network_requirements,
|
||||
filesystem: filesystem_requirements,
|
||||
guardian_policy_config_source: _,
|
||||
} = config_layer_stack.requirements().clone();
|
||||
|
||||
let user_instructions = AgentsMdManager::load_global_instructions(Some(&codex_home))
|
||||
|
||||
@@ -323,8 +323,10 @@ impl LocalProcess {
|
||||
)
|
||||
};
|
||||
|
||||
let has_new_lifecycle_event = response.next_seq > after_seq.saturating_add(1);
|
||||
if !response.chunks.is_empty()
|
||||
|| response.exited
|
||||
|| has_new_lifecycle_event
|
||||
|| response.closed
|
||||
|| tokio::time::Instant::now() >= deadline
|
||||
{
|
||||
let _total_bytes: usize = response
|
||||
@@ -625,16 +627,7 @@ async fn watch_exit(
|
||||
.await;
|
||||
}
|
||||
|
||||
maybe_emit_closed(process_id.clone(), Arc::clone(&inner)).await;
|
||||
|
||||
tokio::time::sleep(EXITED_PROCESS_RETENTION).await;
|
||||
let mut processes = inner.processes.lock().await;
|
||||
if matches!(
|
||||
processes.get(&process_id),
|
||||
Some(ProcessEntry::Running(process)) if process.exit_code == Some(exit_code)
|
||||
) {
|
||||
processes.remove(&process_id);
|
||||
}
|
||||
maybe_emit_closed(process_id, Arc::clone(&inner)).await;
|
||||
}
|
||||
|
||||
async fn finish_output_stream(process_id: ProcessId, inner: Arc<Inner>) {
|
||||
@@ -653,7 +646,7 @@ async fn finish_output_stream(process_id: ProcessId, inner: Arc<Inner>) {
|
||||
}
|
||||
|
||||
async fn maybe_emit_closed(process_id: ProcessId, inner: Arc<Inner>) {
|
||||
let notification = {
|
||||
let closed = {
|
||||
let mut processes = inner.processes.lock().await;
|
||||
let Some(ProcessEntry::Running(process)) = processes.get_mut(&process_id) else {
|
||||
return;
|
||||
@@ -668,16 +661,33 @@ async fn maybe_emit_closed(process_id: ProcessId, inner: Arc<Inner>) {
|
||||
process.next_seq += 1;
|
||||
let _ = process.wake_tx.send(seq);
|
||||
process.events.publish(ExecProcessEvent::Closed { seq });
|
||||
Some(ExecClosedNotification {
|
||||
process_id: process_id.clone(),
|
||||
seq,
|
||||
})
|
||||
Some((
|
||||
ExecClosedNotification {
|
||||
process_id: process_id.clone(),
|
||||
seq,
|
||||
},
|
||||
Arc::clone(&process.output_notify),
|
||||
))
|
||||
};
|
||||
|
||||
let Some(notification) = notification else {
|
||||
let Some((notification, output_notify)) = closed else {
|
||||
return;
|
||||
};
|
||||
|
||||
output_notify.notify_waiters();
|
||||
let cleanup_process_id = process_id.clone();
|
||||
let cleanup_inner = Arc::clone(&inner);
|
||||
tokio::spawn(async move {
|
||||
tokio::time::sleep(EXITED_PROCESS_RETENTION).await;
|
||||
let mut processes = cleanup_inner.processes.lock().await;
|
||||
if matches!(
|
||||
processes.get(&cleanup_process_id),
|
||||
Some(ProcessEntry::Running(process)) if process.closed
|
||||
) {
|
||||
processes.remove(&cleanup_process_id);
|
||||
}
|
||||
});
|
||||
|
||||
if let Some(notifications) = notification_sender(&inner) {
|
||||
let _ = notifications
|
||||
.notify(EXEC_CLOSED_METHOD, ¬ification)
|
||||
@@ -697,6 +707,7 @@ fn notification_sender(inner: &Inner) -> Option<RpcNotificationSender> {
|
||||
mod tests {
|
||||
use super::*;
|
||||
use codex_config::types::ShellEnvironmentPolicyInherit;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
fn test_exec_params(env: HashMap<String, String>) -> ExecParams {
|
||||
ExecParams {
|
||||
@@ -745,4 +756,70 @@ mod tests {
|
||||
|
||||
assert_eq!(child_env(¶ms), expected);
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[tokio::test]
|
||||
async fn exited_process_is_retained_until_output_streams_close() {
|
||||
let process_id = ProcessId::from("retained-until-stream-close");
|
||||
let spawned = codex_utils_pty::spawn_pipe_process_no_stdin(
|
||||
"/bin/sleep",
|
||||
&["5".to_string()],
|
||||
std::env::current_dir().expect("cwd").as_path(),
|
||||
&HashMap::new(),
|
||||
&None::<String>,
|
||||
)
|
||||
.await
|
||||
.expect("spawn sleep");
|
||||
let (outgoing_tx, _outgoing_rx) = mpsc::channel(16);
|
||||
let (wake_tx, _wake_rx) = watch::channel(0);
|
||||
let output_notify = Arc::new(Notify::new());
|
||||
let inner = Arc::new(Inner {
|
||||
notifications: std::sync::RwLock::new(Some(RpcNotificationSender::new(outgoing_tx))),
|
||||
processes: Mutex::new(HashMap::from([(
|
||||
process_id.clone(),
|
||||
ProcessEntry::Running(Box::new(RunningProcess {
|
||||
session: spawned.session,
|
||||
tty: false,
|
||||
pipe_stdin: false,
|
||||
output: VecDeque::new(),
|
||||
retained_bytes: 0,
|
||||
next_seq: 1,
|
||||
exit_code: None,
|
||||
wake_tx,
|
||||
events: ExecProcessEventLog::new(
|
||||
PROCESS_EVENT_CHANNEL_CAPACITY,
|
||||
RETAINED_OUTPUT_BYTES_PER_PROCESS,
|
||||
),
|
||||
output_notify: Arc::clone(&output_notify),
|
||||
open_streams: 1,
|
||||
closed: false,
|
||||
})),
|
||||
)])),
|
||||
});
|
||||
let (exit_tx, exit_rx) = tokio::sync::oneshot::channel();
|
||||
let watch_exit_task = tokio::spawn(watch_exit(
|
||||
process_id.clone(),
|
||||
exit_rx,
|
||||
Arc::clone(&inner),
|
||||
output_notify,
|
||||
));
|
||||
|
||||
exit_tx.send(0).expect("send exit");
|
||||
tokio::time::sleep(EXITED_PROCESS_RETENTION + Duration::from_millis(20)).await;
|
||||
{
|
||||
let processes = inner.processes.lock().await;
|
||||
let Some(ProcessEntry::Running(process)) = processes.get(&process_id) else {
|
||||
panic!("exited process should stay retained while output streams are open");
|
||||
};
|
||||
assert_eq!(process.exit_code, Some(0));
|
||||
assert!(!process.closed);
|
||||
}
|
||||
|
||||
finish_output_stream(process_id.clone(), Arc::clone(&inner)).await;
|
||||
watch_exit_task.await.expect("watch exit task should join");
|
||||
tokio::time::sleep(EXITED_PROCESS_RETENTION + Duration::from_millis(20)).await;
|
||||
|
||||
let processes = inner.processes.lock().await;
|
||||
assert!(!processes.contains_key(&process_id));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1794,7 +1794,7 @@ impl ChatWidget {
|
||||
fn update_task_running_state(&mut self) {
|
||||
self.bottom_pane
|
||||
.set_task_running(self.agent_turn_running || self.mcp_startup_status.is_some());
|
||||
self.refresh_terminal_title();
|
||||
self.refresh_status_surfaces();
|
||||
}
|
||||
|
||||
fn restore_reasoning_status_header(&mut self) {
|
||||
@@ -1913,7 +1913,7 @@ impl ChatWidget {
|
||||
|| (title_uses_spinner
|
||||
&& self.terminal_title_status_kind == TerminalTitleStatusKind::Undoing)
|
||||
{
|
||||
self.refresh_terminal_title();
|
||||
self.refresh_status_surfaces();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2297,7 +2297,6 @@ impl ChatWidget {
|
||||
self.add_boxed_history(Box::new(cell));
|
||||
}
|
||||
self.thread_name = event.thread_name;
|
||||
self.refresh_terminal_title();
|
||||
self.refresh_status_surfaces();
|
||||
self.request_redraw();
|
||||
self.maybe_send_next_queued_input();
|
||||
@@ -2501,7 +2500,6 @@ impl ChatWidget {
|
||||
self.saw_plan_update_this_turn = false;
|
||||
self.saw_plan_item_this_turn = false;
|
||||
self.latest_proposed_plan_markdown = None;
|
||||
self.last_plan_progress = None;
|
||||
self.plan_delta_buffer.clear();
|
||||
self.plan_item_active = false;
|
||||
self.adaptive_chunking.reset();
|
||||
@@ -3573,7 +3571,7 @@ impl ChatWidget {
|
||||
self.update_task_running_state();
|
||||
if restored_task_running && !self.bottom_pane.is_task_running() {
|
||||
self.bottom_pane.set_task_running(/*running*/ true);
|
||||
self.refresh_terminal_title();
|
||||
self.refresh_status_surfaces();
|
||||
}
|
||||
self.refresh_pending_input_preview();
|
||||
self.request_redraw();
|
||||
@@ -3595,7 +3593,7 @@ impl ChatWidget {
|
||||
})
|
||||
.count();
|
||||
self.last_plan_progress = (total > 0).then_some((completed, total));
|
||||
self.refresh_terminal_title();
|
||||
self.refresh_status_surfaces();
|
||||
self.add_to_history(history_cell::new_plan_update(update));
|
||||
}
|
||||
|
||||
|
||||
@@ -144,6 +144,14 @@ fn render_debug_config_lines(stack: &ConfigLayerStack) -> Vec<Line<'static>> {
|
||||
));
|
||||
}
|
||||
|
||||
if requirements_toml.guardian_policy_config.is_some() {
|
||||
requirement_lines.push(requirement_line(
|
||||
"guardian_policy_config",
|
||||
"configured".to_string(),
|
||||
requirements.guardian_policy_config_source.as_ref(),
|
||||
));
|
||||
}
|
||||
|
||||
if let Some(feature_requirements) = requirements.feature_requirements.as_ref() {
|
||||
let value = join_or_empty(
|
||||
feature_requirements
|
||||
@@ -633,6 +641,7 @@ mod tests {
|
||||
file: requirements_file.clone(),
|
||||
},
|
||||
)),
|
||||
guardian_policy_config_source: Some(RequirementSource::CloudRequirements),
|
||||
..ConfigRequirements::default()
|
||||
};
|
||||
|
||||
@@ -642,7 +651,7 @@ mod tests {
|
||||
allowed_sandbox_modes: Some(vec![SandboxModeRequirement::ReadOnly]),
|
||||
remote_sandbox_config: None,
|
||||
allowed_web_search_modes: Some(vec![WebSearchModeRequirement::Cached]),
|
||||
guardian_policy_config: None,
|
||||
guardian_policy_config: Some("Use the managed guardian policy.".to_string()),
|
||||
feature_requirements: Some(FeatureRequirementsToml {
|
||||
entries: BTreeMap::from([("guardian_approval".to_string(), true)]),
|
||||
}),
|
||||
@@ -697,6 +706,9 @@ mod tests {
|
||||
"allowed_web_search_modes: cached, disabled (source: cloud requirements)"
|
||||
)
|
||||
);
|
||||
assert!(
|
||||
rendered.contains("guardian_policy_config: configured (source: cloud requirements)")
|
||||
);
|
||||
assert!(rendered.contains("features: guardian_approval=true (source: cloud requirements)"));
|
||||
assert!(rendered.contains("mcp_servers: docs (source: MDM managed_config.toml (legacy))"));
|
||||
assert!(rendered.contains("enforce_residency: us (source: cloud requirements)"));
|
||||
|
||||
Reference in New Issue
Block a user