mirror of
https://github.com/openai/codex.git
synced 2026-09-04 15:08:45 +00:00
Preserve resume permissions from rollout
This commit is contained in:
@@ -160,6 +160,111 @@ fn merge_persisted_resume_metadata(
|
||||
}
|
||||
}
|
||||
|
||||
struct PersistedResumeThreadSettings {
|
||||
approval_policy: codex_protocol::protocol::AskForApproval,
|
||||
approvals_reviewer: Option<codex_protocol::config_types::ApprovalsReviewer>,
|
||||
permission_profile: codex_protocol::models::PermissionProfile,
|
||||
}
|
||||
|
||||
fn merge_persisted_resume_thread_settings(
|
||||
request_overrides: Option<&HashMap<String, serde_json::Value>>,
|
||||
typesafe_overrides: &mut ConfigOverrides,
|
||||
settings: PersistedResumeThreadSettings,
|
||||
) {
|
||||
if !has_config_override(request_overrides, "approval_policy")
|
||||
&& typesafe_overrides.approval_policy.is_none()
|
||||
{
|
||||
typesafe_overrides.approval_policy = Some(settings.approval_policy);
|
||||
}
|
||||
|
||||
if let Some(approvals_reviewer) = settings.approvals_reviewer
|
||||
&& !has_config_override(request_overrides, "approvals_reviewer")
|
||||
&& typesafe_overrides.approvals_reviewer.is_none()
|
||||
{
|
||||
typesafe_overrides.approvals_reviewer = Some(approvals_reviewer);
|
||||
}
|
||||
|
||||
if !has_permission_resume_override(request_overrides, typesafe_overrides) {
|
||||
typesafe_overrides.permission_profile = Some(settings.permission_profile);
|
||||
}
|
||||
}
|
||||
|
||||
fn has_config_override(
|
||||
request_overrides: Option<&HashMap<String, serde_json::Value>>,
|
||||
key: &str,
|
||||
) -> bool {
|
||||
request_overrides.is_some_and(|overrides| overrides.contains_key(key))
|
||||
}
|
||||
|
||||
fn has_permission_resume_override(
|
||||
request_overrides: Option<&HashMap<String, serde_json::Value>>,
|
||||
typesafe_overrides: &ConfigOverrides,
|
||||
) -> bool {
|
||||
typesafe_overrides.permission_profile.is_some()
|
||||
|| typesafe_overrides.default_permissions.is_some()
|
||||
|| typesafe_overrides.sandbox_mode.is_some()
|
||||
|| has_config_override(request_overrides, "permission_profile")
|
||||
|| has_config_override(request_overrides, "default_permissions")
|
||||
|| has_config_override(request_overrides, "sandbox_mode")
|
||||
}
|
||||
|
||||
async fn latest_persisted_resume_thread_settings(
|
||||
resumed_history: &ResumedHistory,
|
||||
) -> Option<PersistedResumeThreadSettings> {
|
||||
if let Some(rollout_path) = resumed_history.rollout_path.as_ref()
|
||||
&& let Ok((items, _, _)) =
|
||||
codex_rollout::RolloutRecorder::load_rollout_items(rollout_path).await
|
||||
&& let Some(settings) = latest_persisted_resume_thread_settings_from_items(&items)
|
||||
{
|
||||
return Some(settings);
|
||||
}
|
||||
|
||||
latest_persisted_resume_thread_settings_from_items(&resumed_history.history)
|
||||
}
|
||||
|
||||
fn latest_persisted_resume_thread_settings_from_items(
|
||||
rollout_items: &[RolloutItem],
|
||||
) -> Option<PersistedResumeThreadSettings> {
|
||||
let mut settings = None;
|
||||
|
||||
for item in rollout_items {
|
||||
match item {
|
||||
RolloutItem::EventMsg(EventMsg::SessionConfigured(event)) => {
|
||||
settings = Some(PersistedResumeThreadSettings {
|
||||
approval_policy: event.approval_policy,
|
||||
approvals_reviewer: Some(event.approvals_reviewer),
|
||||
permission_profile: event.permission_profile.clone(),
|
||||
});
|
||||
}
|
||||
RolloutItem::EventMsg(EventMsg::ThreadSettingsApplied(event)) => {
|
||||
let snapshot = &event.thread_settings;
|
||||
settings = Some(PersistedResumeThreadSettings {
|
||||
approval_policy: snapshot.approval_policy,
|
||||
approvals_reviewer: Some(snapshot.approvals_reviewer),
|
||||
permission_profile: snapshot.permission_profile.clone(),
|
||||
});
|
||||
}
|
||||
RolloutItem::TurnContext(turn_context) => {
|
||||
let approvals_reviewer = settings
|
||||
.as_ref()
|
||||
.and_then(|settings| settings.approvals_reviewer);
|
||||
settings = Some(PersistedResumeThreadSettings {
|
||||
approval_policy: turn_context.approval_policy,
|
||||
approvals_reviewer,
|
||||
permission_profile: turn_context.permission_profile(),
|
||||
});
|
||||
}
|
||||
RolloutItem::SessionMeta(_)
|
||||
| RolloutItem::EventMsg(_)
|
||||
| RolloutItem::ResponseItem(_)
|
||||
| RolloutItem::InterAgentCommunication(_)
|
||||
| RolloutItem::Compacted(_) => {}
|
||||
}
|
||||
}
|
||||
|
||||
settings
|
||||
}
|
||||
|
||||
fn normalize_thread_list_cwd_filters(
|
||||
cwd: Option<ThreadListCwdFilter>,
|
||||
) -> Result<Option<Vec<PathBuf>>, JSONRPCErrorError> {
|
||||
@@ -2840,6 +2945,13 @@ impl ThreadRequestProcessor {
|
||||
let InitialHistory::Resumed(resumed_history) = thread_history else {
|
||||
return None;
|
||||
};
|
||||
if let Some(settings) = latest_persisted_resume_thread_settings(resumed_history).await {
|
||||
merge_persisted_resume_thread_settings(
|
||||
request_overrides.as_ref(),
|
||||
typesafe_overrides,
|
||||
settings,
|
||||
);
|
||||
}
|
||||
let state_db_ctx = self.state_db.clone()?;
|
||||
let persisted_metadata = state_db_ctx
|
||||
.get_thread(resumed_history.conversation_id)
|
||||
|
||||
@@ -14,6 +14,7 @@ use app_test_support::test_absolute_path;
|
||||
use app_test_support::to_response;
|
||||
use app_test_support::write_chatgpt_auth;
|
||||
use chrono::Utc;
|
||||
use codex_app_server_protocol::ApprovalsReviewer;
|
||||
use codex_app_server_protocol::AskForApproval;
|
||||
use codex_app_server_protocol::ClientInfo;
|
||||
use codex_app_server_protocol::CommandExecutionApprovalDecision;
|
||||
@@ -27,6 +28,7 @@ use codex_app_server_protocol::McpToolCallAppContext;
|
||||
use codex_app_server_protocol::PatchApplyStatus;
|
||||
use codex_app_server_protocol::PatchChangeKind;
|
||||
use codex_app_server_protocol::RequestId;
|
||||
use codex_app_server_protocol::SandboxPolicy;
|
||||
use codex_app_server_protocol::ServerNotification;
|
||||
use codex_app_server_protocol::ServerRequest;
|
||||
use codex_app_server_protocol::SessionSource;
|
||||
@@ -58,6 +60,8 @@ use codex_login::REFRESH_TOKEN_URL_OVERRIDE_ENV_VAR;
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::config_types::Personality;
|
||||
use codex_protocol::mcp::CallToolResult;
|
||||
use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_DANGER_FULL_ACCESS;
|
||||
use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_WORKSPACE;
|
||||
use codex_protocol::models::ContentItem;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use codex_protocol::protocol::AgentMessageEvent;
|
||||
@@ -112,6 +116,7 @@ use super::analytics::wait_for_goal_event;
|
||||
const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(25);
|
||||
#[cfg(not(windows))]
|
||||
const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10);
|
||||
const PERMISSIONS_RESUME_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(30);
|
||||
const CODEX_5_2_INSTRUCTIONS_TEMPLATE_DEFAULT: &str = "You are Codex, a coding agent based on GPT-5. You and the user share the same workspace and collaborate to achieve the user's goals.";
|
||||
|
||||
fn normalized_existing_path(path: impl AsRef<Path>) -> Result<PathBuf> {
|
||||
@@ -148,6 +153,50 @@ async fn wait_for_responses_request_count(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn start_materialized_workspace_auto_review_thread(
|
||||
mcp: &mut TestAppServer,
|
||||
) -> Result<String> {
|
||||
let start_id = mcp
|
||||
.send_thread_start_request(ThreadStartParams {
|
||||
model: Some("gpt-5.4".to_string()),
|
||||
approval_policy: Some(AskForApproval::OnRequest),
|
||||
approvals_reviewer: Some(ApprovalsReviewer::AutoReview),
|
||||
permissions: Some(BUILT_IN_PERMISSION_PROFILE_WORKSPACE.to_string()),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let start_resp: JSONRPCResponse = timeout(
|
||||
PERMISSIONS_RESUME_READ_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(start_id)),
|
||||
)
|
||||
.await??;
|
||||
let ThreadStartResponse { thread, .. } = to_response::<ThreadStartResponse>(start_resp)?;
|
||||
|
||||
let turn_id = mcp
|
||||
.send_turn_start_request(TurnStartParams {
|
||||
thread_id: thread.id.clone(),
|
||||
client_user_message_id: None,
|
||||
input: vec![UserInput::Text {
|
||||
text: "seed history".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
timeout(
|
||||
PERMISSIONS_RESUME_READ_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(turn_id)),
|
||||
)
|
||||
.await??;
|
||||
timeout(
|
||||
PERMISSIONS_RESUME_READ_TIMEOUT,
|
||||
mcp.read_stream_until_notification_message("turn/completed"),
|
||||
)
|
||||
.await??;
|
||||
|
||||
Ok(thread.id)
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn thread_resume_rejects_unmaterialized_thread() -> Result<()> {
|
||||
let server = create_mock_responses_server_repeating_assistant("Done").await;
|
||||
@@ -195,6 +244,90 @@ async fn thread_resume_rejects_unmaterialized_thread() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn thread_resume_preserves_persisted_permissions_without_overrides() -> Result<()> {
|
||||
let server = create_mock_responses_server_repeating_assistant("Done").await;
|
||||
let codex_home = TempDir::new()?;
|
||||
create_config_toml(codex_home.path(), &server.uri())?;
|
||||
|
||||
let mut primary = TestAppServer::new(codex_home.path()).await?;
|
||||
timeout(PERMISSIONS_RESUME_READ_TIMEOUT, primary.initialize()).await??;
|
||||
let thread_id = start_materialized_workspace_auto_review_thread(&mut primary).await?;
|
||||
drop(primary);
|
||||
|
||||
let mut secondary = TestAppServer::new(codex_home.path()).await?;
|
||||
timeout(PERMISSIONS_RESUME_READ_TIMEOUT, secondary.initialize()).await??;
|
||||
|
||||
let resume_id = secondary
|
||||
.send_thread_resume_request(ThreadResumeParams {
|
||||
thread_id,
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let resume_resp: JSONRPCResponse = timeout(
|
||||
PERMISSIONS_RESUME_READ_TIMEOUT,
|
||||
secondary.read_stream_until_response_message(RequestId::Integer(resume_id)),
|
||||
)
|
||||
.await??;
|
||||
let ThreadResumeResponse {
|
||||
approval_policy,
|
||||
approvals_reviewer,
|
||||
sandbox,
|
||||
..
|
||||
} = to_response::<ThreadResumeResponse>(resume_resp)?;
|
||||
|
||||
assert_eq!(approval_policy, AskForApproval::OnRequest);
|
||||
assert_eq!(approvals_reviewer, ApprovalsReviewer::AutoReview);
|
||||
assert!(
|
||||
matches!(sandbox, SandboxPolicy::WorkspaceWrite { .. }),
|
||||
"expected persisted workspace-write permissions, got {sandbox:?}"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn thread_resume_permission_overrides_win_over_persisted_permissions() -> Result<()> {
|
||||
let server = create_mock_responses_server_repeating_assistant("Done").await;
|
||||
let codex_home = TempDir::new()?;
|
||||
create_config_toml(codex_home.path(), &server.uri())?;
|
||||
|
||||
let mut primary = TestAppServer::new(codex_home.path()).await?;
|
||||
timeout(PERMISSIONS_RESUME_READ_TIMEOUT, primary.initialize()).await??;
|
||||
let thread_id = start_materialized_workspace_auto_review_thread(&mut primary).await?;
|
||||
drop(primary);
|
||||
|
||||
let mut secondary = TestAppServer::new(codex_home.path()).await?;
|
||||
timeout(PERMISSIONS_RESUME_READ_TIMEOUT, secondary.initialize()).await??;
|
||||
|
||||
let resume_id = secondary
|
||||
.send_thread_resume_request(ThreadResumeParams {
|
||||
thread_id,
|
||||
approval_policy: Some(AskForApproval::Never),
|
||||
approvals_reviewer: Some(ApprovalsReviewer::User),
|
||||
permissions: Some(BUILT_IN_PERMISSION_PROFILE_DANGER_FULL_ACCESS.to_string()),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let resume_resp: JSONRPCResponse = timeout(
|
||||
PERMISSIONS_RESUME_READ_TIMEOUT,
|
||||
secondary.read_stream_until_response_message(RequestId::Integer(resume_id)),
|
||||
)
|
||||
.await??;
|
||||
let ThreadResumeResponse {
|
||||
approval_policy,
|
||||
approvals_reviewer,
|
||||
sandbox,
|
||||
..
|
||||
} = to_response::<ThreadResumeResponse>(resume_resp)?;
|
||||
|
||||
assert_eq!(approval_policy, AskForApproval::Never);
|
||||
assert_eq!(approvals_reviewer, ApprovalsReviewer::User);
|
||||
assert_eq!(sandbox, SandboxPolicy::DangerFullAccess);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn thread_resume_with_empty_path_uses_running_thread_id() -> Result<()> {
|
||||
let server = create_mock_responses_server_repeating_assistant("Done").await;
|
||||
|
||||
@@ -85,6 +85,8 @@ pub fn should_persist_event_msg(ev: &EventMsg) -> bool {
|
||||
| EventMsg::PatchApplyEnd(_)
|
||||
| EventMsg::TokenCount(_)
|
||||
| EventMsg::ThreadGoalUpdated(_)
|
||||
| EventMsg::SessionConfigured(_)
|
||||
| EventMsg::ThreadSettingsApplied(_)
|
||||
| EventMsg::ContextCompacted(_)
|
||||
| EventMsg::EnteredReviewMode(_)
|
||||
| EventMsg::ExitedReviewMode(_)
|
||||
@@ -129,8 +131,6 @@ pub fn should_persist_event_msg(ev: &EventMsg) -> bool {
|
||||
| EventMsg::TurnModerationMetadata(_)
|
||||
| EventMsg::AgentReasoningSectionBreak(_)
|
||||
| EventMsg::RawResponseItem(_)
|
||||
| EventMsg::SessionConfigured(_)
|
||||
| EventMsg::ThreadSettingsApplied(_)
|
||||
| EventMsg::McpToolCallBegin(_)
|
||||
| EventMsg::ExecCommandBegin(_)
|
||||
| EventMsg::TerminalInteraction(_)
|
||||
|
||||
Reference in New Issue
Block a user