mirror of
https://github.com/openai/codex.git
synced 2026-10-18 17:18:52 +00:00
Restore permission profiles when resuming threads (#39153)
## Why Cold resumes and forks could lose the thread's active permission profile and fall back to the current configured default. ## What changed - Restore the latest persisted approval policy, approvals reviewer, and active permission-profile ID when resuming or forking a thread. - Re-resolve persisted profile IDs through current configuration and requirements. Missing or invalid profiles fall back to the configured default, while threads without a persisted profile continue to use current configuration. - Keep explicit permission and approval overrides ahead of persisted settings. ## Testing Added coverage for legacy and paginated histories, settings updates, explicit overrides, removed or changed profiles, requirement fallbacks, and forks. GitOrigin-RevId: 2289af2f3206bf131a6ffe0e8447e8d402e76c47
This commit is contained in:
@@ -358,7 +358,7 @@ Start a fresh thread when you need a new Codex conversation.
|
||||
|
||||
Valid `personality` values are `"friendly"`, `"pragmatic"`, and `"none"`. When `"none"` is selected, the personality placeholder is replaced with an empty string.
|
||||
|
||||
To continue a stored session, call `thread/resume` with the `thread.id` you previously recorded. The response shape matches `thread/start`. When the stored session includes persisted token usage, the server emits `thread/tokenUsage/updated` immediately after the response so clients can render restored usage before the next turn starts. You can also pass the same configuration overrides supported by `thread/start`, including `approvalsReviewer`. On cold resume, approval policy uses the first allowed value in this order: request override, latest persisted thread setting, current configured default.
|
||||
To continue a stored session, call `thread/resume` with the `thread.id` you previously recorded. The response shape matches `thread/start`. When the stored session includes persisted token usage, the server emits `thread/tokenUsage/updated` immediately after the response so clients can render restored usage before the next turn starts. You can also pass the same configuration overrides supported by `thread/start`, including `approvalsReviewer`. On cold resume, approval policy and the active permission-profile ID select a source in this order: request override, latest persisted thread setting, current configured default. The persisted profile ID is resolved through the same config and requirements path as a `permissions` override. Threads without an active profile ID use current config instead of restoring their concrete historical permissions.
|
||||
|
||||
By default, `thread/resume` includes the reconstructed turn history in `thread.turns`. Experimental clients can pass `excludeTurns: true` to return only thread metadata and live resume state, then call `thread/turns/list` separately if they want to page the turn history over the network. A cold paginated resume can still replay persisted `thread/tokenUsage/updated` when it can identify the corresponding stored turn; resuming an already-loaded thread waits for the next live update.
|
||||
|
||||
|
||||
@@ -1,10 +1,14 @@
|
||||
use codex_protocol::config_types::ApprovalsReviewer;
|
||||
use codex_protocol::models::ActivePermissionProfile;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_rollout::RolloutItem;
|
||||
|
||||
#[derive(Debug, PartialEq, Eq)]
|
||||
pub(super) struct PersistedResumeSettings {
|
||||
pub(super) approval_policy: AskForApproval,
|
||||
pub(super) approvals_reviewer: Option<ApprovalsReviewer>,
|
||||
pub(super) active_permission_profile: Option<ActivePermissionProfile>,
|
||||
}
|
||||
|
||||
pub(super) fn latest_persisted_resume_settings(
|
||||
@@ -16,6 +20,7 @@ pub(super) fn latest_persisted_resume_settings(
|
||||
.rev()
|
||||
.find_map(|(index, item)| match item {
|
||||
RolloutItem::TurnContext(turn_context) => Some(PersistedResumeSettings {
|
||||
approval_policy: turn_context.approval_policy,
|
||||
approvals_reviewer: turn_context.approvals_reviewer.or_else(|| {
|
||||
history[..index].iter().rev().find_map(|item| match item {
|
||||
RolloutItem::TurnContext(turn_context) => turn_context.approvals_reviewer,
|
||||
@@ -25,10 +30,16 @@ pub(super) fn latest_persisted_resume_settings(
|
||||
_ => None,
|
||||
})
|
||||
}),
|
||||
active_permission_profile: turn_context.active_permission_profile.clone(),
|
||||
}),
|
||||
RolloutItem::EventMsg(EventMsg::ThreadSettingsApplied(event)) => {
|
||||
Some(PersistedResumeSettings {
|
||||
approval_policy: event.thread_settings.approval_policy,
|
||||
approvals_reviewer: Some(event.thread_settings.approvals_reviewer),
|
||||
active_permission_profile: event
|
||||
.thread_settings
|
||||
.active_permission_profile
|
||||
.clone(),
|
||||
})
|
||||
}
|
||||
_ => None,
|
||||
|
||||
@@ -4,6 +4,7 @@ use codex_protocol::config_types::ApprovalsReviewer;
|
||||
use codex_protocol::config_types::CollaborationMode;
|
||||
use codex_protocol::config_types::ModeKind;
|
||||
use codex_protocol::config_types::Settings;
|
||||
use codex_protocol::models::ActivePermissionProfile;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
@@ -20,17 +21,21 @@ fn cwd() -> AbsolutePathBuf {
|
||||
.expect("absolute current directory")
|
||||
}
|
||||
|
||||
fn settings_item(approvals_reviewer: ApprovalsReviewer) -> RolloutItem {
|
||||
fn settings_item(
|
||||
approval_policy: AskForApproval,
|
||||
approvals_reviewer: ApprovalsReviewer,
|
||||
active_permission_profile: Option<ActivePermissionProfile>,
|
||||
) -> RolloutItem {
|
||||
RolloutItem::EventMsg(EventMsg::ThreadSettingsApplied(
|
||||
ThreadSettingsAppliedEvent {
|
||||
thread_settings: ThreadSettingsSnapshot {
|
||||
model: "gpt-5".to_string(),
|
||||
model_provider_id: "openai".to_string(),
|
||||
service_tier: None,
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
approval_policy,
|
||||
approvals_reviewer,
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
active_permission_profile: None,
|
||||
active_permission_profile,
|
||||
cwd: cwd(),
|
||||
reasoning_effort: None,
|
||||
reasoning_summary: None,
|
||||
@@ -48,18 +53,23 @@ fn settings_item(approvals_reviewer: ApprovalsReviewer) -> RolloutItem {
|
||||
))
|
||||
}
|
||||
|
||||
fn turn_context_item(turn_id: &str, approvals_reviewer: Option<ApprovalsReviewer>) -> RolloutItem {
|
||||
fn turn_context_item(
|
||||
turn_id: &str,
|
||||
approval_policy: AskForApproval,
|
||||
approvals_reviewer: Option<ApprovalsReviewer>,
|
||||
active_permission_profile: Option<ActivePermissionProfile>,
|
||||
) -> RolloutItem {
|
||||
RolloutItem::TurnContext(TurnContextItem {
|
||||
turn_id: Some(turn_id.to_string()),
|
||||
cwd: cwd(),
|
||||
workspace_roots: None,
|
||||
workspace_roots: Some(vec![cwd()]),
|
||||
current_date: None,
|
||||
timezone: None,
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
approval_policy,
|
||||
approvals_reviewer,
|
||||
sandbox_policy: SandboxPolicy::new_read_only_policy(),
|
||||
permission_profile: None,
|
||||
active_permission_profile: None,
|
||||
permission_profile: Some(PermissionProfile::read_only()),
|
||||
active_permission_profile,
|
||||
network: None,
|
||||
file_system_sandbox_policy: None,
|
||||
model: "gpt-5".to_string(),
|
||||
@@ -76,45 +86,74 @@ fn turn_context_item(turn_id: &str, approvals_reviewer: Option<ApprovalsReviewer
|
||||
|
||||
#[test]
|
||||
fn latest_settings_snapshot_wins() {
|
||||
let expected = PersistedResumeSettings {
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
approvals_reviewer: Some(ApprovalsReviewer::AutoReview),
|
||||
active_permission_profile: Some(ActivePermissionProfile::new("dev")),
|
||||
};
|
||||
let history = vec![
|
||||
settings_item(ApprovalsReviewer::User),
|
||||
settings_item(ApprovalsReviewer::AutoReview),
|
||||
settings_item(
|
||||
AskForApproval::Never,
|
||||
ApprovalsReviewer::User,
|
||||
/*active_permission_profile*/ None,
|
||||
),
|
||||
settings_item(
|
||||
AskForApproval::OnRequest,
|
||||
ApprovalsReviewer::AutoReview,
|
||||
Some(ActivePermissionProfile::new("dev")),
|
||||
),
|
||||
];
|
||||
|
||||
assert_eq!(
|
||||
latest_persisted_resume_settings(&history),
|
||||
Some(PersistedResumeSettings {
|
||||
approvals_reviewer: Some(ApprovalsReviewer::AutoReview),
|
||||
})
|
||||
);
|
||||
assert_eq!(latest_persisted_resume_settings(&history), Some(expected));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn latest_turn_context_wins_over_earlier_settings_update() {
|
||||
let expected = PersistedResumeSettings {
|
||||
approval_policy: AskForApproval::UnlessTrusted,
|
||||
approvals_reviewer: Some(ApprovalsReviewer::User),
|
||||
active_permission_profile: Some(ActivePermissionProfile::read_only()),
|
||||
};
|
||||
let history = vec![
|
||||
settings_item(ApprovalsReviewer::AutoReview),
|
||||
turn_context_item("turn-2", Some(ApprovalsReviewer::User)),
|
||||
settings_item(
|
||||
AskForApproval::Never,
|
||||
ApprovalsReviewer::AutoReview,
|
||||
Some(ActivePermissionProfile::new("dev")),
|
||||
),
|
||||
turn_context_item(
|
||||
"turn-2",
|
||||
AskForApproval::UnlessTrusted,
|
||||
Some(ApprovalsReviewer::User),
|
||||
Some(ActivePermissionProfile::read_only()),
|
||||
),
|
||||
];
|
||||
|
||||
assert_eq!(
|
||||
latest_persisted_resume_settings(&history),
|
||||
Some(PersistedResumeSettings {
|
||||
approvals_reviewer: Some(ApprovalsReviewer::User),
|
||||
})
|
||||
);
|
||||
assert_eq!(latest_persisted_resume_settings(&history), Some(expected));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn older_reviewer_is_used_when_latest_turn_context_omits_it() {
|
||||
let history = vec![
|
||||
turn_context_item("turn-1", Some(ApprovalsReviewer::AutoReview)),
|
||||
turn_context_item("turn-2", /*approvals_reviewer*/ None),
|
||||
turn_context_item(
|
||||
"turn-1",
|
||||
AskForApproval::Never,
|
||||
Some(ApprovalsReviewer::AutoReview),
|
||||
/*active_permission_profile*/ None,
|
||||
),
|
||||
turn_context_item(
|
||||
"turn-2",
|
||||
AskForApproval::OnRequest,
|
||||
/*approvals_reviewer*/ None,
|
||||
/*active_permission_profile*/ None,
|
||||
),
|
||||
];
|
||||
|
||||
assert_eq!(
|
||||
latest_persisted_resume_settings(&history),
|
||||
Some(PersistedResumeSettings {
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
approvals_reviewer: Some(ApprovalsReviewer::AutoReview),
|
||||
active_permission_profile: None,
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
use super::persisted_resume_settings::PersistedResumeSettings;
|
||||
use super::persisted_resume_settings::latest_persisted_resume_settings;
|
||||
use super::thread_enrichment::enrich_loaded_threads;
|
||||
use super::thread_fork_goal::inherit_thread_goal_snapshot;
|
||||
@@ -226,42 +227,6 @@ fn merge_persisted_resume_metadata(
|
||||
}
|
||||
}
|
||||
|
||||
fn latest_persisted_approval_policy(
|
||||
history: &[RolloutItem],
|
||||
) -> Option<codex_protocol::protocol::AskForApproval> {
|
||||
history
|
||||
.iter()
|
||||
.enumerate()
|
||||
.rev()
|
||||
.find_map(|(index, item)| match item {
|
||||
RolloutItem::TurnContext(turn_context) => {
|
||||
let updated_policy = turn_context.turn_id.as_ref().and_then(|turn_id| {
|
||||
let turn_start = history[..index].iter().rposition(|item| {
|
||||
matches!(
|
||||
item,
|
||||
RolloutItem::EventMsg(EventMsg::TurnStarted(event))
|
||||
if &event.turn_id == turn_id
|
||||
)
|
||||
})?;
|
||||
history[turn_start + 1..index]
|
||||
.iter()
|
||||
.rev()
|
||||
.find_map(|item| match item {
|
||||
RolloutItem::EventMsg(EventMsg::ThreadSettingsApplied(event)) => {
|
||||
Some(event.thread_settings.approval_policy)
|
||||
}
|
||||
_ => None,
|
||||
})
|
||||
});
|
||||
Some(updated_policy.unwrap_or(turn_context.approval_policy))
|
||||
}
|
||||
RolloutItem::EventMsg(EventMsg::ThreadSettingsApplied(event)) => {
|
||||
Some(event.thread_settings.approval_policy)
|
||||
}
|
||||
_ => None,
|
||||
})
|
||||
}
|
||||
|
||||
fn normalize_thread_list_cwd_filters(
|
||||
cwd: Option<ThreadListCwdFilter>,
|
||||
) -> Result<Option<Vec<PathBuf>>, JSONRPCErrorError> {
|
||||
@@ -297,6 +262,18 @@ fn has_model_resume_override(
|
||||
.is_some_and(|overrides| overrides.contains_key("model_reasoning_effort"))
|
||||
}
|
||||
|
||||
fn has_permission_override(
|
||||
request_overrides: Option<&HashMap<String, serde_json::Value>>,
|
||||
typesafe_overrides: &ConfigOverrides,
|
||||
) -> bool {
|
||||
typesafe_overrides.sandbox_mode.is_some()
|
||||
|| typesafe_overrides.permission_profile.is_some()
|
||||
|| typesafe_overrides.default_permissions.is_some()
|
||||
|| request_overrides.is_some_and(|overrides| {
|
||||
overrides.contains_key("sandbox_mode") || overrides.contains_key("default_permissions")
|
||||
})
|
||||
}
|
||||
|
||||
fn validate_dynamic_tools(tools: &[DynamicToolSpec]) -> Result<(), String> {
|
||||
const DYNAMIC_TOOL_NAME_MAX_LEN: usize = 128;
|
||||
const DYNAMIC_TOOL_NAMESPACE_MAX_LEN: usize = 64;
|
||||
@@ -3931,18 +3908,23 @@ impl ThreadRequestProcessor {
|
||||
let InitialHistory::Resumed(resumed_history) = thread_history else {
|
||||
return None;
|
||||
};
|
||||
if typesafe_overrides.approvals_reviewer.is_none()
|
||||
&& !request_overrides
|
||||
.as_ref()
|
||||
.is_some_and(|overrides| overrides.contains_key("approvals_reviewer"))
|
||||
if let Some(persisted_settings) = latest_persisted_resume_settings(&resumed_history.history)
|
||||
{
|
||||
typesafe_overrides.approvals_reviewer =
|
||||
latest_persisted_resume_settings(&resumed_history.history)
|
||||
.and_then(|settings| settings.approvals_reviewer);
|
||||
}
|
||||
if typesafe_overrides.approval_policy.is_none() {
|
||||
typesafe_overrides.approval_policy =
|
||||
latest_persisted_approval_policy(&resumed_history.history);
|
||||
if typesafe_overrides.approval_policy.is_none() {
|
||||
typesafe_overrides.approval_policy = Some(persisted_settings.approval_policy);
|
||||
}
|
||||
if typesafe_overrides.approvals_reviewer.is_none()
|
||||
&& !request_overrides
|
||||
.as_ref()
|
||||
.is_some_and(|overrides| overrides.contains_key("approvals_reviewer"))
|
||||
{
|
||||
typesafe_overrides.approvals_reviewer = persisted_settings.approvals_reviewer;
|
||||
}
|
||||
if !has_permission_override(request_overrides.as_ref(), typesafe_overrides) {
|
||||
typesafe_overrides.persisted_permission_profile_id = persisted_settings
|
||||
.active_permission_profile
|
||||
.map(|profile| profile.id);
|
||||
}
|
||||
}
|
||||
let state_db_ctx = self.state_db.clone()?;
|
||||
let persisted_metadata = state_db_ctx
|
||||
@@ -4669,44 +4651,66 @@ impl ThreadRequestProcessor {
|
||||
/*personality*/ None,
|
||||
);
|
||||
typesafe_overrides.ephemeral = ephemeral.then_some(true);
|
||||
let latest_context = if paginated_source
|
||||
&& typesafe_overrides.approvals_reviewer.is_none()
|
||||
let restore_approval_policy = typesafe_overrides.approval_policy.is_none();
|
||||
let restore_approvals_reviewer = typesafe_overrides.approvals_reviewer.is_none()
|
||||
&& !request_overrides
|
||||
.as_ref()
|
||||
.is_some_and(|overrides| overrides.contains_key("approvals_reviewer"))
|
||||
{
|
||||
.is_some_and(|overrides| overrides.contains_key("approvals_reviewer"));
|
||||
let restore_permission_profile =
|
||||
!has_permission_override(request_overrides.as_ref(), &typesafe_overrides);
|
||||
let needs_latest_settings =
|
||||
restore_approval_policy || restore_approvals_reviewer || restore_permission_profile;
|
||||
let loaded_parent_settings = if paginated_source && needs_latest_settings {
|
||||
if let Ok(parent) = self.thread_manager.get_thread(source_thread_id).await {
|
||||
typesafe_overrides.approvals_reviewer =
|
||||
Some(parent.config_snapshot().await.approvals_reviewer);
|
||||
None
|
||||
} else if last_turn_id.is_some() || before_turn_id.is_some() {
|
||||
Some(
|
||||
self.thread_store
|
||||
.load_latest_model_context(StoreLoadThreadHistoryParams {
|
||||
thread_id: source_thread_id,
|
||||
include_archived: true,
|
||||
})
|
||||
.await
|
||||
.map_err(thread_store_resume_read_error)?
|
||||
.items,
|
||||
)
|
||||
let snapshot = parent.config_snapshot().await;
|
||||
Some(PersistedResumeSettings {
|
||||
approval_policy: snapshot.approval_policy,
|
||||
approvals_reviewer: Some(snapshot.approvals_reviewer),
|
||||
active_permission_profile: snapshot.active_permission_profile,
|
||||
})
|
||||
} else {
|
||||
None
|
||||
}
|
||||
} else {
|
||||
None
|
||||
};
|
||||
if typesafe_overrides.approvals_reviewer.is_none()
|
||||
&& !request_overrides
|
||||
.as_ref()
|
||||
.is_some_and(|overrides| overrides.contains_key("approvals_reviewer"))
|
||||
let latest_context = if paginated_source
|
||||
&& needs_latest_settings
|
||||
&& loaded_parent_settings.is_none()
|
||||
&& (last_turn_id.is_some() || before_turn_id.is_some())
|
||||
{
|
||||
typesafe_overrides.approvals_reviewer = latest_persisted_resume_settings(
|
||||
Some(
|
||||
self.thread_store
|
||||
.load_latest_model_context(StoreLoadThreadHistoryParams {
|
||||
thread_id: source_thread_id,
|
||||
include_archived: true,
|
||||
})
|
||||
.await
|
||||
.map_err(thread_store_resume_read_error)?
|
||||
.items,
|
||||
)
|
||||
} else {
|
||||
None
|
||||
};
|
||||
let persisted_settings = loaded_parent_settings.or_else(|| {
|
||||
latest_persisted_resume_settings(
|
||||
latest_context
|
||||
.as_deref()
|
||||
.unwrap_or_else(|| source_history_items.as_ref()),
|
||||
)
|
||||
.and_then(|settings| settings.approvals_reviewer);
|
||||
});
|
||||
if let Some(persisted_settings) = persisted_settings {
|
||||
if restore_approval_policy {
|
||||
typesafe_overrides.approval_policy = Some(persisted_settings.approval_policy);
|
||||
}
|
||||
if restore_approvals_reviewer {
|
||||
typesafe_overrides.approvals_reviewer = persisted_settings.approvals_reviewer;
|
||||
}
|
||||
if restore_permission_profile {
|
||||
typesafe_overrides.persisted_permission_profile_id = persisted_settings
|
||||
.active_permission_profile
|
||||
.map(|profile| profile.id);
|
||||
}
|
||||
}
|
||||
// Derive a Config using the same logic as new conversation, honoring overrides if provided.
|
||||
let config = self
|
||||
|
||||
@@ -36,137 +36,6 @@ mod thread_list_cwd_filter_tests {
|
||||
}
|
||||
}
|
||||
|
||||
mod persisted_resume_approval_policy_tests {
|
||||
use super::super::latest_persisted_approval_policy;
|
||||
use codex_protocol::config_types::ApprovalsReviewer;
|
||||
use codex_protocol::config_types::CollaborationMode;
|
||||
use codex_protocol::config_types::ModeKind;
|
||||
use codex_protocol::config_types::Settings;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use codex_protocol::protocol::ThreadSettingsAppliedEvent;
|
||||
use codex_protocol::protocol::ThreadSettingsSnapshot;
|
||||
use codex_protocol::protocol::TurnContextItem;
|
||||
use codex_protocol::protocol::TurnStartedEvent;
|
||||
use codex_rollout::RolloutItem;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
fn cwd() -> AbsolutePathBuf {
|
||||
AbsolutePathBuf::try_from(std::env::current_dir().expect("current directory"))
|
||||
.expect("absolute current directory")
|
||||
}
|
||||
|
||||
fn settings_item(approval_policy: AskForApproval) -> RolloutItem {
|
||||
RolloutItem::EventMsg(EventMsg::ThreadSettingsApplied(
|
||||
ThreadSettingsAppliedEvent {
|
||||
thread_settings: ThreadSettingsSnapshot {
|
||||
model: "gpt-5".to_string(),
|
||||
model_provider_id: "openai".to_string(),
|
||||
service_tier: None,
|
||||
approval_policy,
|
||||
approvals_reviewer: ApprovalsReviewer::User,
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
active_permission_profile: None,
|
||||
cwd: cwd(),
|
||||
reasoning_effort: None,
|
||||
reasoning_summary: None,
|
||||
personality: None,
|
||||
collaboration_mode: CollaborationMode {
|
||||
mode: ModeKind::Default,
|
||||
settings: Settings {
|
||||
model: "gpt-5".to_string(),
|
||||
reasoning_effort: None,
|
||||
developer_instructions: None,
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
))
|
||||
}
|
||||
|
||||
fn turn_started_item(turn_id: &str) -> RolloutItem {
|
||||
RolloutItem::EventMsg(EventMsg::TurnStarted(TurnStartedEvent {
|
||||
turn_id: turn_id.to_string(),
|
||||
trace_id: None,
|
||||
started_at: None,
|
||||
model_context_window: None,
|
||||
collaboration_mode_kind: ModeKind::Default,
|
||||
}))
|
||||
}
|
||||
|
||||
fn turn_context_item(turn_id: &str, approval_policy: AskForApproval) -> RolloutItem {
|
||||
RolloutItem::TurnContext(TurnContextItem {
|
||||
turn_id: Some(turn_id.to_string()),
|
||||
cwd: cwd(),
|
||||
workspace_roots: None,
|
||||
current_date: None,
|
||||
timezone: None,
|
||||
approval_policy,
|
||||
approvals_reviewer: None,
|
||||
sandbox_policy: SandboxPolicy::new_read_only_policy(),
|
||||
permission_profile: None,
|
||||
active_permission_profile: None,
|
||||
network: None,
|
||||
file_system_sandbox_policy: None,
|
||||
model: "gpt-5".to_string(),
|
||||
comp_hash: None,
|
||||
personality: None,
|
||||
collaboration_mode: None,
|
||||
multi_agent_version: None,
|
||||
multi_agent_mode: None,
|
||||
realtime_active: None,
|
||||
effort: None,
|
||||
summary: codex_protocol::config_types::ReasoningSummary::Auto,
|
||||
})
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn latest_settings_snapshot_wins() {
|
||||
let history = vec![
|
||||
settings_item(AskForApproval::Never),
|
||||
settings_item(AskForApproval::OnRequest),
|
||||
];
|
||||
|
||||
assert_eq!(
|
||||
latest_persisted_approval_policy(&history),
|
||||
Some(AskForApproval::OnRequest)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn settings_applied_during_turn_wins_over_stale_compaction_context() {
|
||||
let history = vec![
|
||||
turn_started_item("turn-1"),
|
||||
settings_item(AskForApproval::Never),
|
||||
turn_context_item("turn-1", AskForApproval::OnRequest),
|
||||
];
|
||||
|
||||
assert_eq!(
|
||||
latest_persisted_approval_policy(&history),
|
||||
Some(AskForApproval::Never)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn later_turn_context_wins_over_earlier_settings_update() {
|
||||
let history = vec![
|
||||
turn_started_item("turn-1"),
|
||||
settings_item(AskForApproval::Never),
|
||||
turn_context_item("turn-1", AskForApproval::OnRequest),
|
||||
turn_started_item("turn-2"),
|
||||
turn_context_item("turn-2", AskForApproval::OnRequest),
|
||||
];
|
||||
|
||||
assert_eq!(
|
||||
latest_persisted_approval_policy(&history),
|
||||
Some(AskForApproval::OnRequest)
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
mod background_terminal_pagination_tests {
|
||||
use super::super::paginate_background_terminals;
|
||||
use codex_app_server_protocol::ThreadBackgroundTerminal;
|
||||
|
||||
@@ -346,7 +346,12 @@ async fn thread_resume_and_fork_upgrade_legacy_protected_model_settings() -> Res
|
||||
))
|
||||
.await?;
|
||||
let fork: ForkResponse = timeout(TIMEOUT, server.read_response(id)).await??;
|
||||
assert_protected(&fork.model, fork.approval_policy, fork.approvals_reviewer);
|
||||
assert_protected_with_policy(
|
||||
&fork.model,
|
||||
fork.approval_policy,
|
||||
fork.approvals_reviewer,
|
||||
Never,
|
||||
);
|
||||
let id = server
|
||||
.send_thread_resume_request(params!(
|
||||
ResumeParams,
|
||||
|
||||
@@ -10,12 +10,16 @@ use app_test_support::create_mock_responses_server_sequence_unchecked;
|
||||
use app_test_support::rollout_path;
|
||||
use app_test_support::to_response;
|
||||
use app_test_support::write_chatgpt_auth;
|
||||
use codex_app_server_protocol::ActivePermissionProfile;
|
||||
use codex_app_server_protocol::ApprovalsReviewer;
|
||||
use codex_app_server_protocol::AskForApproval;
|
||||
use codex_app_server_protocol::ClientRequest;
|
||||
use codex_app_server_protocol::JSONRPCError;
|
||||
use codex_app_server_protocol::JSONRPCMessage;
|
||||
use codex_app_server_protocol::JSONRPCResponse;
|
||||
use codex_app_server_protocol::RequestId;
|
||||
use codex_app_server_protocol::SandboxMode;
|
||||
use codex_app_server_protocol::SandboxPolicy;
|
||||
use codex_app_server_protocol::ServerNotification;
|
||||
use codex_app_server_protocol::SessionSource;
|
||||
use codex_app_server_protocol::ThreadForkParams;
|
||||
@@ -293,6 +297,115 @@ async fn paginated_thread_fork_preserves_persisted_approvals_reviewer() -> Resul
|
||||
assert_thread_fork_preserves_persisted_approvals_reviewer(ThreadHistoryMode::Paginated).await
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn thread_fork_preserves_persisted_permission_profile_and_honors_overrides() -> Result<()> {
|
||||
let server = create_mock_responses_server_repeating_assistant("Done").await;
|
||||
|
||||
for history_mode in [ThreadHistoryMode::Legacy, ThreadHistoryMode::Paginated] {
|
||||
let codex_home = TempDir::new()?;
|
||||
MockResponsesConfig::new(&server.uri())
|
||||
.with_root_config("default_permissions = \":danger-full-access\"")
|
||||
.with_extra_config("[permissions.dev]\nextends = \":read-only\"")
|
||||
.write(codex_home.path())?;
|
||||
|
||||
let source_thread_id = {
|
||||
let mut mcp = TestAppServer::builder()
|
||||
.with_codex_home(codex_home.path())
|
||||
.without_managed_config()
|
||||
.build_initialized()
|
||||
.await?;
|
||||
let start_id = mcp
|
||||
.send_thread_start_request_with_auto_env(ThreadStartParams {
|
||||
history_mode: Some(history_mode),
|
||||
approval_policy: Some(AskForApproval::OnRequest),
|
||||
permissions: Some("dev".to_string()),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let ThreadStartResponse { thread, .. } =
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(start_id)).await??;
|
||||
timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.start_turn_and_wait_for_completion(TurnStartParams {
|
||||
thread_id: thread.id.clone(),
|
||||
input: vec![UserInput::Text {
|
||||
text: "persist permission profile".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
..Default::default()
|
||||
}),
|
||||
)
|
||||
.await??;
|
||||
thread.id
|
||||
};
|
||||
|
||||
let mut mcp = TestAppServer::builder()
|
||||
.with_codex_home(codex_home.path())
|
||||
.without_managed_config()
|
||||
.build_initialized()
|
||||
.await?;
|
||||
let fork_id = mcp
|
||||
.send_thread_fork_request(ThreadForkParams {
|
||||
thread_id: source_thread_id.clone(),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let ThreadForkResponse {
|
||||
approval_policy,
|
||||
sandbox,
|
||||
active_permission_profile,
|
||||
..
|
||||
} = timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(fork_id)).await??;
|
||||
assert!(matches!(sandbox, SandboxPolicy::ReadOnly { .. }));
|
||||
assert_eq!(approval_policy, AskForApproval::OnRequest);
|
||||
assert_eq!(
|
||||
active_permission_profile,
|
||||
Some(ActivePermissionProfile {
|
||||
id: "dev".to_string(),
|
||||
extends: Some(":read-only".to_string()),
|
||||
})
|
||||
);
|
||||
|
||||
let fork_id = mcp
|
||||
.send_thread_fork_request(ThreadForkParams {
|
||||
thread_id: source_thread_id.clone(),
|
||||
approval_policy: Some(AskForApproval::Never),
|
||||
sandbox: Some(SandboxMode::DangerFullAccess),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let ThreadForkResponse {
|
||||
approval_policy,
|
||||
sandbox,
|
||||
active_permission_profile,
|
||||
..
|
||||
} = timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(fork_id)).await??;
|
||||
assert!(matches!(sandbox, SandboxPolicy::DangerFullAccess));
|
||||
assert_eq!(approval_policy, AskForApproval::Never);
|
||||
assert_eq!(active_permission_profile, None);
|
||||
|
||||
let fork_id = mcp
|
||||
.send_thread_fork_request(ThreadForkParams {
|
||||
thread_id: source_thread_id,
|
||||
permissions: Some(":workspace".to_string()),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let ThreadForkResponse {
|
||||
sandbox,
|
||||
active_permission_profile,
|
||||
..
|
||||
} = timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(fork_id)).await??;
|
||||
assert!(matches!(sandbox, SandboxPolicy::WorkspaceWrite { .. }));
|
||||
assert_eq!(
|
||||
active_permission_profile,
|
||||
Some(ActivePermissionProfile::new(":workspace"))
|
||||
);
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn assert_thread_fork_preserves_persisted_approvals_reviewer(
|
||||
history_mode: ThreadHistoryMode,
|
||||
) -> Result<()> {
|
||||
@@ -309,6 +422,7 @@ async fn assert_thread_fork_preserves_persisted_approvals_reviewer(
|
||||
let start_id = mcp
|
||||
.send_thread_start_request_with_auto_env(ThreadStartParams {
|
||||
history_mode: Some(history_mode),
|
||||
permissions: Some(":workspace".to_string()),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
@@ -348,7 +462,9 @@ async fn assert_thread_fork_preserves_persisted_approvals_reviewer(
|
||||
text: "switch to auto-review".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
approval_policy: Some(AskForApproval::OnRequest),
|
||||
approvals_reviewer: Some(ApprovalsReviewer::AutoReview),
|
||||
permissions: Some(":read-only".to_string()),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
@@ -372,9 +488,17 @@ async fn assert_thread_fork_preserves_persisted_approvals_reviewer(
|
||||
})
|
||||
.await?;
|
||||
let ThreadForkResponse {
|
||||
approvals_reviewer, ..
|
||||
approval_policy,
|
||||
approvals_reviewer,
|
||||
active_permission_profile,
|
||||
..
|
||||
} = timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(fork_id)).await??;
|
||||
assert_eq!(approval_policy, AskForApproval::OnRequest);
|
||||
assert_eq!(approvals_reviewer, ApprovalsReviewer::AutoReview);
|
||||
assert_eq!(
|
||||
active_permission_profile,
|
||||
Some(ActivePermissionProfile::new(":read-only"))
|
||||
);
|
||||
}
|
||||
|
||||
(thread.id, turn.id)
|
||||
@@ -397,24 +521,41 @@ async fn assert_thread_fork_preserves_persisted_approvals_reviewer(
|
||||
)
|
||||
.await??;
|
||||
let ThreadForkResponse {
|
||||
approvals_reviewer, ..
|
||||
approval_policy,
|
||||
approvals_reviewer,
|
||||
active_permission_profile,
|
||||
..
|
||||
} = to_response(fork_resp)?;
|
||||
|
||||
assert_eq!(approval_policy, AskForApproval::OnRequest);
|
||||
assert_eq!(approvals_reviewer, ApprovalsReviewer::AutoReview);
|
||||
assert_eq!(
|
||||
active_permission_profile,
|
||||
Some(ActivePermissionProfile::new(":read-only"))
|
||||
);
|
||||
|
||||
if matches!(history_mode, ThreadHistoryMode::Paginated) {
|
||||
let fork_id = mcp
|
||||
.send_thread_fork_request(ThreadForkParams {
|
||||
thread_id: source_thread_id,
|
||||
last_turn_id: Some(source_turn_id),
|
||||
approval_policy: Some(AskForApproval::Never),
|
||||
approvals_reviewer: Some(ApprovalsReviewer::User),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let ThreadForkResponse {
|
||||
approvals_reviewer, ..
|
||||
approval_policy,
|
||||
approvals_reviewer,
|
||||
active_permission_profile,
|
||||
..
|
||||
} = timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(fork_id)).await??;
|
||||
assert_eq!(approval_policy, AskForApproval::Never);
|
||||
assert_eq!(approvals_reviewer, ApprovalsReviewer::User);
|
||||
assert_eq!(
|
||||
active_permission_profile,
|
||||
Some(ActivePermissionProfile::new(":read-only"))
|
||||
);
|
||||
}
|
||||
|
||||
Ok(())
|
||||
|
||||
@@ -16,6 +16,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::ActivePermissionProfile;
|
||||
use codex_app_server_protocol::ApprovalsReviewer;
|
||||
use codex_app_server_protocol::AskForApproval;
|
||||
use codex_app_server_protocol::ClientInfo;
|
||||
@@ -30,6 +31,8 @@ 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::SandboxMode;
|
||||
use codex_app_server_protocol::SandboxPolicy as AppSandboxPolicy;
|
||||
use codex_app_server_protocol::ServerNotification;
|
||||
use codex_app_server_protocol::ServerRequest;
|
||||
use codex_app_server_protocol::SessionSource;
|
||||
@@ -75,6 +78,9 @@ use codex_protocol::config_types::ModeKind;
|
||||
use codex_protocol::config_types::Personality;
|
||||
use codex_protocol::config_types::Settings;
|
||||
use codex_protocol::mcp::CallToolResult;
|
||||
use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_DANGER_FULL_ACCESS;
|
||||
use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_READ_ONLY;
|
||||
use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_WORKSPACE;
|
||||
use codex_protocol::models::ContentItem;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use codex_protocol::openai_models::ReasoningEffort;
|
||||
@@ -1126,6 +1132,358 @@ async fn thread_resume_preserves_acknowledged_model_effort_and_approvals_reviewe
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn cold_resume_reresolves_persisted_active_permission_profile() -> Result<()> {
|
||||
let server = create_mock_responses_server_repeating_assistant("Done").await;
|
||||
for history_mode in [ThreadHistoryMode::Legacy, ThreadHistoryMode::Paginated] {
|
||||
let codex_home = TempDir::new()?;
|
||||
let previous_workspace_root = TempDir::new()?;
|
||||
write_dev_permission_config(&server.uri(), codex_home.path(), ":workspace")?;
|
||||
let thread_id = {
|
||||
let mut mcp = TestAppServer::builder()
|
||||
.with_codex_home(codex_home.path())
|
||||
.without_managed_config()
|
||||
.build_initialized()
|
||||
.await?;
|
||||
let thread_id = materialize_dev_permission_thread(&mut mcp, history_mode).await?;
|
||||
timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.start_turn_and_wait_for_completion(TurnStartParams {
|
||||
thread_id: thread_id.clone(),
|
||||
runtime_workspace_roots: Some(vec![AbsolutePathBuf::from_absolute_path(
|
||||
previous_workspace_root.path(),
|
||||
)?]),
|
||||
input: vec![UserInput::Text {
|
||||
text: "update runtime workspace roots".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
..Default::default()
|
||||
}),
|
||||
)
|
||||
.await??;
|
||||
thread_id
|
||||
};
|
||||
|
||||
write_dev_permission_config(&server.uri(), codex_home.path(), ":read-only")?;
|
||||
let mut mcp = TestAppServer::builder()
|
||||
.with_codex_home(codex_home.path())
|
||||
.without_managed_config()
|
||||
.build_initialized()
|
||||
.await?;
|
||||
let resume_id = mcp
|
||||
.send_thread_resume_request(ThreadResumeParams {
|
||||
thread_id,
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let ThreadResumeResponse {
|
||||
sandbox,
|
||||
active_permission_profile,
|
||||
runtime_workspace_roots,
|
||||
..
|
||||
} = timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(resume_id)).await??;
|
||||
|
||||
assert!(matches!(sandbox, AppSandboxPolicy::ReadOnly { .. }));
|
||||
assert_eq!(
|
||||
active_permission_profile,
|
||||
Some(ActivePermissionProfile {
|
||||
id: "dev".to_string(),
|
||||
extends: Some(BUILT_IN_PERMISSION_PROFILE_READ_ONLY.to_string()),
|
||||
})
|
||||
);
|
||||
assert!(
|
||||
!runtime_workspace_roots.contains(&AbsolutePathBuf::from_absolute_path(
|
||||
previous_workspace_root.path(),
|
||||
)?)
|
||||
);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn cold_resume_with_removed_permission_profile_uses_configured_default() -> Result<()> {
|
||||
let server = create_mock_responses_server_repeating_assistant("Done").await;
|
||||
for history_mode in [ThreadHistoryMode::Legacy, ThreadHistoryMode::Paginated] {
|
||||
let codex_home = TempDir::new()?;
|
||||
write_dev_permission_config(&server.uri(), codex_home.path(), ":workspace")?;
|
||||
let thread_id = {
|
||||
let mut mcp = TestAppServer::builder()
|
||||
.with_codex_home(codex_home.path())
|
||||
.without_managed_config()
|
||||
.build_initialized()
|
||||
.await?;
|
||||
materialize_dev_permission_thread(&mut mcp, history_mode).await?
|
||||
};
|
||||
|
||||
MockResponsesConfig::new(&server.uri())
|
||||
.with_root_config(&format!(
|
||||
"default_permissions = \"{BUILT_IN_PERMISSION_PROFILE_DANGER_FULL_ACCESS}\""
|
||||
))
|
||||
.write(codex_home.path())?;
|
||||
let mut mcp = TestAppServer::builder()
|
||||
.with_codex_home(codex_home.path())
|
||||
.without_managed_config()
|
||||
.build_initialized()
|
||||
.await?;
|
||||
let resume_id = mcp
|
||||
.send_thread_resume_request(ThreadResumeParams {
|
||||
thread_id,
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let ThreadResumeResponse {
|
||||
sandbox,
|
||||
active_permission_profile,
|
||||
..
|
||||
} = timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(resume_id)).await??;
|
||||
|
||||
assert!(matches!(sandbox, AppSandboxPolicy::DangerFullAccess));
|
||||
assert_eq!(
|
||||
active_permission_profile,
|
||||
Some(ActivePermissionProfile::new(
|
||||
BUILT_IN_PERMISSION_PROFILE_DANGER_FULL_ACCESS,
|
||||
))
|
||||
);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn cold_resume_permission_overrides_win_over_persisted_profile() -> Result<()> {
|
||||
let server = create_mock_responses_server_repeating_assistant("Done").await;
|
||||
let codex_home = TempDir::new()?;
|
||||
write_dev_permission_config(&server.uri(), codex_home.path(), ":workspace")?;
|
||||
let thread_id = {
|
||||
let mut mcp = TestAppServer::builder()
|
||||
.with_codex_home(codex_home.path())
|
||||
.without_managed_config()
|
||||
.build_initialized()
|
||||
.await?;
|
||||
materialize_dev_permission_thread(&mut mcp, ThreadHistoryMode::Legacy).await?
|
||||
};
|
||||
|
||||
for params in [
|
||||
ThreadResumeParams {
|
||||
thread_id: thread_id.clone(),
|
||||
sandbox: Some(SandboxMode::ReadOnly),
|
||||
..Default::default()
|
||||
},
|
||||
ThreadResumeParams {
|
||||
thread_id: thread_id.clone(),
|
||||
permissions: Some(BUILT_IN_PERMISSION_PROFILE_READ_ONLY.to_string()),
|
||||
..Default::default()
|
||||
},
|
||||
ThreadResumeParams {
|
||||
thread_id: thread_id.clone(),
|
||||
config: Some(std::collections::HashMap::from([(
|
||||
"default_permissions".to_string(),
|
||||
json!(BUILT_IN_PERMISSION_PROFILE_READ_ONLY),
|
||||
)])),
|
||||
..Default::default()
|
||||
},
|
||||
] {
|
||||
let expected_active_permission_profile = params
|
||||
.sandbox
|
||||
.is_none()
|
||||
.then(ActivePermissionProfile::read_only);
|
||||
let mut mcp = TestAppServer::builder()
|
||||
.with_codex_home(codex_home.path())
|
||||
.without_managed_config()
|
||||
.build_initialized()
|
||||
.await?;
|
||||
let resume_id = mcp.send_thread_resume_request(params).await?;
|
||||
let ThreadResumeResponse {
|
||||
sandbox,
|
||||
active_permission_profile,
|
||||
..
|
||||
} = timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(resume_id)).await??;
|
||||
|
||||
assert!(matches!(sandbox, AppSandboxPolicy::ReadOnly { .. }));
|
||||
assert_eq!(
|
||||
active_permission_profile,
|
||||
expected_active_permission_profile
|
||||
);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn cold_resume_without_active_permission_profile_uses_current_config() -> Result<()> {
|
||||
let server = create_mock_responses_server_repeating_assistant("Done").await;
|
||||
let codex_home = TempDir::new()?;
|
||||
mock_responses_config(&server.uri()).write(codex_home.path())?;
|
||||
let thread_id = {
|
||||
let mut mcp = TestAppServer::builder()
|
||||
.with_codex_home(codex_home.path())
|
||||
.without_managed_config()
|
||||
.build_initialized()
|
||||
.await?;
|
||||
let ThreadStartResponse { thread, .. } = mcp
|
||||
.start_thread(ThreadStartParams {
|
||||
model: Some("mock-model".to_string()),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.start_turn_and_wait_for_completion(TurnStartParams {
|
||||
thread_id: thread.id.clone(),
|
||||
input: vec![UserInput::Text {
|
||||
text: "persist full access".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
sandbox_policy: Some(AppSandboxPolicy::DangerFullAccess),
|
||||
..Default::default()
|
||||
}),
|
||||
)
|
||||
.await??;
|
||||
thread.id
|
||||
};
|
||||
|
||||
MockResponsesConfig::new(&server.uri())
|
||||
.with_root_config(&format!(
|
||||
"default_permissions = \"{BUILT_IN_PERMISSION_PROFILE_WORKSPACE}\""
|
||||
))
|
||||
.write(codex_home.path())?;
|
||||
let mut mcp = TestAppServer::builder()
|
||||
.with_codex_home(codex_home.path())
|
||||
.without_managed_config()
|
||||
.build_initialized()
|
||||
.await?;
|
||||
let resume_id = mcp
|
||||
.send_thread_resume_request(ThreadResumeParams {
|
||||
thread_id,
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let ThreadResumeResponse {
|
||||
sandbox,
|
||||
active_permission_profile,
|
||||
..
|
||||
} = timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(resume_id)).await??;
|
||||
|
||||
assert!(matches!(sandbox, AppSandboxPolicy::WorkspaceWrite { .. }));
|
||||
assert_eq!(
|
||||
active_permission_profile,
|
||||
Some(ActivePermissionProfile::new(
|
||||
BUILT_IN_PERMISSION_PROFILE_WORKSPACE
|
||||
))
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn cold_resume_restores_profile_selected_by_settings_update() -> Result<()> {
|
||||
let server = create_mock_responses_server_repeating_assistant("Done").await;
|
||||
let codex_home = TempDir::new()?;
|
||||
mock_responses_config(&server.uri()).write(codex_home.path())?;
|
||||
let thread_id = {
|
||||
let mut mcp = TestAppServer::builder()
|
||||
.with_codex_home(codex_home.path())
|
||||
.without_managed_config()
|
||||
.build_initialized()
|
||||
.await?;
|
||||
let ThreadStartResponse { thread, .. } = mcp
|
||||
.start_thread(ThreadStartParams {
|
||||
model: Some("mock-model".to_string()),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.start_turn_and_wait_for_completion(TurnStartParams {
|
||||
thread_id: thread.id.clone(),
|
||||
input: vec![UserInput::Text {
|
||||
text: "persist permission profile".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
..Default::default()
|
||||
}),
|
||||
)
|
||||
.await??;
|
||||
let update_id = mcp
|
||||
.send_thread_settings_update_request(ThreadSettingsUpdateParams {
|
||||
thread_id: thread.id.clone(),
|
||||
permissions: Some(BUILT_IN_PERMISSION_PROFILE_WORKSPACE.to_string()),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let _: ThreadSettingsUpdateResponse =
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(update_id)).await??;
|
||||
timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_notification_message("thread/settings/updated"),
|
||||
)
|
||||
.await??;
|
||||
thread.id
|
||||
};
|
||||
|
||||
mock_responses_config(&server.uri()).write(codex_home.path())?;
|
||||
let mut mcp = TestAppServer::builder()
|
||||
.with_codex_home(codex_home.path())
|
||||
.without_managed_config()
|
||||
.build_initialized()
|
||||
.await?;
|
||||
let resume_id = mcp
|
||||
.send_thread_resume_request(ThreadResumeParams {
|
||||
thread_id,
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let ThreadResumeResponse {
|
||||
sandbox,
|
||||
active_permission_profile,
|
||||
..
|
||||
} = timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(resume_id)).await??;
|
||||
|
||||
assert!(matches!(sandbox, AppSandboxPolicy::WorkspaceWrite { .. }));
|
||||
assert_eq!(
|
||||
active_permission_profile,
|
||||
Some(ActivePermissionProfile::new(
|
||||
BUILT_IN_PERMISSION_PROFILE_WORKSPACE
|
||||
))
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn materialize_dev_permission_thread(
|
||||
mcp: &mut TestAppServer,
|
||||
history_mode: ThreadHistoryMode,
|
||||
) -> Result<String> {
|
||||
let ThreadStartResponse { thread, .. } = mcp
|
||||
.start_thread(ThreadStartParams {
|
||||
model: Some("mock-model".to_string()),
|
||||
history_mode: Some(history_mode),
|
||||
permissions: Some("dev".to_string()),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.start_turn_and_wait_for_completion(TurnStartParams {
|
||||
thread_id: thread.id.clone(),
|
||||
input: vec![UserInput::Text {
|
||||
text: "persist permission profile".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
..Default::default()
|
||||
}),
|
||||
)
|
||||
.await??;
|
||||
Ok(thread.id)
|
||||
}
|
||||
|
||||
fn write_dev_permission_config(
|
||||
server_uri: &str,
|
||||
codex_home: &Path,
|
||||
dev_extends: &str,
|
||||
) -> std::io::Result<()> {
|
||||
MockResponsesConfig::new(server_uri)
|
||||
.with_root_config("default_permissions = \":danger-full-access\"")
|
||||
.with_extra_config(&format!("[permissions.dev]\nextends = \"{dev_extends}\""))
|
||||
.write(codex_home)
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn thread_goal_get_rejects_unmaterialized_thread() -> Result<()> {
|
||||
let server = create_mock_responses_server_repeating_assistant("Done").await;
|
||||
|
||||
@@ -2360,6 +2360,196 @@ async fn permission_profile_override_populates_runtime_permissions() -> std::io:
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn persisted_permission_profile_id_wins_over_configured_default() -> std::io::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let cwd = TempDir::new()?;
|
||||
let config_toml = toml::from_str(
|
||||
r#"
|
||||
default_permissions = ":read-only"
|
||||
|
||||
[permissions.dev]
|
||||
extends = ":workspace"
|
||||
"#,
|
||||
)
|
||||
.expect("permission profile config should deserialize");
|
||||
|
||||
let config = Config::load_from_base_config_with_overrides(
|
||||
config_toml,
|
||||
ConfigOverrides {
|
||||
cwd: Some(cwd.path().to_path_buf()),
|
||||
persisted_permission_profile_id: Some("dev".to_string()),
|
||||
..Default::default()
|
||||
},
|
||||
codex_home.abs(),
|
||||
)
|
||||
.await?;
|
||||
|
||||
assert_eq!(
|
||||
config.permissions.active_permission_profile(),
|
||||
Some(ActivePermissionProfile {
|
||||
id: "dev".to_string(),
|
||||
extends: Some(BUILT_IN_PERMISSION_PROFILE_WORKSPACE.to_string()),
|
||||
})
|
||||
);
|
||||
assert_eq!(
|
||||
config.legacy_sandbox_policy(),
|
||||
SandboxPolicy::new_workspace_write_policy()
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn missing_persisted_permission_profile_id_uses_configured_default() -> std::io::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let cwd = TempDir::new()?;
|
||||
let project_key = cwd.path().to_string_lossy().to_string();
|
||||
|
||||
for (configured_default, configured_sandbox, expected_active_profile) in [
|
||||
(
|
||||
Some(BUILT_IN_PERMISSION_PROFILE_READ_ONLY.to_string()),
|
||||
None,
|
||||
Some(ActivePermissionProfile::read_only()),
|
||||
),
|
||||
(None, Some(SandboxMode::ReadOnly), None),
|
||||
] {
|
||||
let config = Config::load_from_base_config_with_overrides(
|
||||
ConfigToml {
|
||||
default_permissions: configured_default,
|
||||
sandbox_mode: configured_sandbox,
|
||||
projects: Some(HashMap::from([(
|
||||
project_key.clone(),
|
||||
ProjectConfig {
|
||||
trust_level: Some(TrustLevel::Trusted),
|
||||
},
|
||||
)])),
|
||||
..Default::default()
|
||||
},
|
||||
ConfigOverrides {
|
||||
cwd: Some(cwd.path().to_path_buf()),
|
||||
persisted_permission_profile_id: Some("removed-profile".to_string()),
|
||||
..Default::default()
|
||||
},
|
||||
codex_home.abs(),
|
||||
)
|
||||
.await?;
|
||||
|
||||
assert_eq!(
|
||||
config.permissions.active_permission_profile(),
|
||||
expected_active_profile
|
||||
);
|
||||
assert_eq!(
|
||||
config.permissions.effective_permission_profile(),
|
||||
PermissionProfile::read_only()
|
||||
);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn invalid_persisted_permission_profile_uses_configured_default() -> std::io::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let cwd = TempDir::new()?;
|
||||
for invalid_profile in [
|
||||
"extends = \"removed-parent\"",
|
||||
"extends = \":read-only\"\n\n[permissions.dev.filesystem]\nglob_scan_max_depth = 0",
|
||||
] {
|
||||
let config_toml = toml::from_str(&format!(
|
||||
"default_permissions = \":danger-full-access\"\n\n[permissions.dev]\n{invalid_profile}"
|
||||
))
|
||||
.expect("permission profile config should deserialize");
|
||||
|
||||
let config = Config::load_from_base_config_with_overrides(
|
||||
config_toml,
|
||||
ConfigOverrides {
|
||||
cwd: Some(cwd.path().to_path_buf()),
|
||||
persisted_permission_profile_id: Some("dev".to_string()),
|
||||
..Default::default()
|
||||
},
|
||||
codex_home.abs(),
|
||||
)
|
||||
.await?;
|
||||
|
||||
assert_eq!(
|
||||
config.permissions.active_permission_profile(),
|
||||
Some(ActivePermissionProfile::new(
|
||||
BUILT_IN_PERMISSION_PROFILE_DANGER_FULL_ACCESS,
|
||||
))
|
||||
);
|
||||
assert_eq!(
|
||||
config.permissions.effective_permission_profile(),
|
||||
PermissionProfile::Disabled
|
||||
);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn persisted_profile_cycle_uses_configured_default() -> std::io::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let cwd = TempDir::new()?;
|
||||
let config_toml = toml::from_str(
|
||||
r#"
|
||||
default_permissions = ":danger-full-access"
|
||||
|
||||
[permissions.dev]
|
||||
extends = "base"
|
||||
|
||||
[permissions.base]
|
||||
extends = "dev"
|
||||
"#,
|
||||
)
|
||||
.expect("permission profile config should deserialize");
|
||||
|
||||
let config = Config::load_from_base_config_with_overrides(
|
||||
config_toml,
|
||||
ConfigOverrides {
|
||||
cwd: Some(cwd.path().to_path_buf()),
|
||||
persisted_permission_profile_id: Some("dev".to_string()),
|
||||
..Default::default()
|
||||
},
|
||||
codex_home.abs(),
|
||||
)
|
||||
.await?;
|
||||
|
||||
assert_eq!(
|
||||
config.permissions.active_permission_profile(),
|
||||
Some(ActivePermissionProfile::new(
|
||||
BUILT_IN_PERMISSION_PROFILE_DANGER_FULL_ACCESS,
|
||||
))
|
||||
);
|
||||
assert_eq!(
|
||||
config.permissions.effective_permission_profile(),
|
||||
PermissionProfile::Disabled
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn missing_explicit_default_permissions_remains_an_error() -> std::io::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let cwd = TempDir::new()?;
|
||||
|
||||
let error = Config::load_from_base_config_with_overrides(
|
||||
ConfigToml::default(),
|
||||
ConfigOverrides {
|
||||
cwd: Some(cwd.path().to_path_buf()),
|
||||
default_permissions: Some("removed-profile".to_string()),
|
||||
..Default::default()
|
||||
},
|
||||
codex_home.abs(),
|
||||
)
|
||||
.await
|
||||
.expect_err("explicit missing profile should be rejected");
|
||||
|
||||
assert_eq!(error.kind(), std::io::ErrorKind::InvalidInput);
|
||||
assert_eq!(
|
||||
error.to_string(),
|
||||
"default_permissions requires a `[permissions]` table"
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn permission_snapshot_setter_preserves_permission_constraints() {
|
||||
let initial_profile = PermissionProfile::read_only();
|
||||
@@ -10383,32 +10573,46 @@ async fn permission_profile_override_falls_back_when_disallowed_by_requirements(
|
||||
#[tokio::test]
|
||||
async fn active_profile_is_cleared_when_requirements_force_fallback() -> std::io::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let config = ConfigBuilder::without_managed_config_for_tests()
|
||||
.codex_home(codex_home.path().to_path_buf())
|
||||
.fallback_cwd(Some(codex_home.path().to_path_buf()))
|
||||
.harness_overrides(ConfigOverrides {
|
||||
for overrides in [
|
||||
ConfigOverrides {
|
||||
default_permissions: Some(BUILT_IN_PERMISSION_PROFILE_DANGER_FULL_ACCESS.to_string()),
|
||||
..Default::default()
|
||||
})
|
||||
.cloud_config_bundle(
|
||||
CloudConfigBundleFixture::loader_with_enterprise_requirement(
|
||||
r#"allowed_sandbox_modes = ["read-only"]"#,
|
||||
},
|
||||
ConfigOverrides {
|
||||
persisted_permission_profile_id: Some(
|
||||
BUILT_IN_PERMISSION_PROFILE_DANGER_FULL_ACCESS.to_string(),
|
||||
),
|
||||
)
|
||||
.build()
|
||||
.await?;
|
||||
..Default::default()
|
||||
},
|
||||
] {
|
||||
let config = ConfigBuilder::without_managed_config_for_tests()
|
||||
.codex_home(codex_home.path().to_path_buf())
|
||||
.fallback_cwd(Some(codex_home.path().to_path_buf()))
|
||||
.harness_overrides(overrides)
|
||||
.cloud_config_bundle(
|
||||
CloudConfigBundleFixture::loader_with_enterprise_requirement(
|
||||
r#"allowed_sandbox_modes = ["read-only"]"#,
|
||||
),
|
||||
)
|
||||
.build()
|
||||
.await?;
|
||||
|
||||
assert_eq!(
|
||||
config.permissions.effective_permission_profile(),
|
||||
PermissionProfile::read_only()
|
||||
);
|
||||
assert_eq!(config.permissions.active_permission_profile(), None);
|
||||
assert!(
|
||||
config.startup_warnings.iter().any(|warning| warning
|
||||
.contains("Configured value for `permission_profile` is disallowed by requirements")),
|
||||
"{:?}",
|
||||
config.startup_warnings
|
||||
);
|
||||
assert_eq!(
|
||||
config.permissions.effective_permission_profile(),
|
||||
PermissionProfile::read_only()
|
||||
);
|
||||
assert_eq!(config.permissions.active_permission_profile(), None);
|
||||
assert!(
|
||||
config
|
||||
.startup_warnings
|
||||
.iter()
|
||||
.any(|warning| warning.contains(
|
||||
"Configured value for `permission_profile` is disallowed by requirements"
|
||||
)),
|
||||
"{:?}",
|
||||
config.startup_warnings
|
||||
);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
|
||||
@@ -2388,6 +2388,7 @@ struct PermissionSelectionToml {
|
||||
struct EffectivePermissionSelection<'a> {
|
||||
profiles: Option<PermissionsToml>,
|
||||
selected_profile_id: Option<&'a str>,
|
||||
persisted_profile_id_was_provided: bool,
|
||||
requirements_force_profile_selection: bool,
|
||||
}
|
||||
|
||||
@@ -2403,7 +2404,8 @@ impl EffectivePermissionSelection<'_> {
|
||||
default_permissions_override: Option<&str>,
|
||||
permission_config_syntax: Option<PermissionConfigSyntax>,
|
||||
) -> bool {
|
||||
self.requirements_force_profile_selection
|
||||
self.persisted_profile_id_was_provided
|
||||
|| self.requirements_force_profile_selection
|
||||
|| default_permissions_override.is_some()
|
||||
|| matches!(
|
||||
permission_config_syntax,
|
||||
@@ -2506,6 +2508,10 @@ pub struct ConfigOverrides {
|
||||
pub sandbox_mode: Option<SandboxMode>,
|
||||
pub permission_profile: Option<PermissionProfile>,
|
||||
pub default_permissions: Option<String>,
|
||||
/// Permission profile ID recovered from persisted thread state. Explicit
|
||||
/// permission overrides take precedence, and stale profile IDs fall back
|
||||
/// to the configured default.
|
||||
pub persisted_permission_profile_id: Option<String>,
|
||||
pub model_provider: Option<String>,
|
||||
pub service_tier: Option<Option<String>>,
|
||||
pub codex_self_exe: Option<PathBuf>,
|
||||
@@ -3153,6 +3159,7 @@ impl Config {
|
||||
sandbox_mode,
|
||||
permission_profile,
|
||||
default_permissions: default_permissions_override,
|
||||
persisted_permission_profile_id,
|
||||
model_provider,
|
||||
service_tier: service_tier_override,
|
||||
codex_self_exe,
|
||||
@@ -3297,9 +3304,23 @@ impl Config {
|
||||
sandbox_mode,
|
||||
);
|
||||
let requirements_toml = config_layer_stack.requirements_toml();
|
||||
let windows_sandbox_level = match effective_windows_sandbox_mode {
|
||||
Some(WindowsSandboxModeToml::Elevated) => WindowsSandboxLevel::Elevated,
|
||||
Some(WindowsSandboxModeToml::Unelevated) => WindowsSandboxLevel::RestrictedToken,
|
||||
None => WindowsSandboxLevel::Disabled,
|
||||
};
|
||||
let persisted_permission_profile_id = if sandbox_mode.is_some()
|
||||
|| permission_profile.is_some()
|
||||
|| default_permissions_override.is_some()
|
||||
{
|
||||
None
|
||||
} else {
|
||||
persisted_permission_profile_id.as_deref()
|
||||
};
|
||||
let effective_permission_selection = resolve_effective_permission_selection(
|
||||
cfg.permissions.as_ref(),
|
||||
default_permissions_override.as_deref(),
|
||||
persisted_permission_profile_id,
|
||||
cfg.default_permissions.as_deref(),
|
||||
requirements_toml,
|
||||
&mut startup_warnings,
|
||||
@@ -3318,11 +3339,6 @@ impl Config {
|
||||
));
|
||||
}
|
||||
|
||||
let windows_sandbox_level = match effective_windows_sandbox_mode {
|
||||
Some(WindowsSandboxModeToml::Elevated) => WindowsSandboxLevel::Elevated,
|
||||
Some(WindowsSandboxModeToml::Unelevated) => WindowsSandboxLevel::RestrictedToken,
|
||||
None => WindowsSandboxLevel::Disabled,
|
||||
};
|
||||
let memories_config: MemoriesConfig = cfg.memories.clone().unwrap_or_default().into();
|
||||
let memories_root = memory_root(&codex_home);
|
||||
|
||||
@@ -3330,7 +3346,9 @@ impl Config {
|
||||
default_permissions_override.as_deref(),
|
||||
permission_config_syntax,
|
||||
);
|
||||
let explicit_permission_profile_mode = default_permissions_override.is_some()
|
||||
let explicit_permission_profile_mode = effective_permission_selection
|
||||
.persisted_profile_id_was_provided
|
||||
|| default_permissions_override.is_some()
|
||||
|| matches!(
|
||||
permission_config_syntax,
|
||||
Some(PermissionConfigSyntax::Profiles)
|
||||
@@ -3342,7 +3360,9 @@ impl Config {
|
||||
.into_iter()
|
||||
.filter(|profile| !is_builtin_permission_profile_name(&profile.id))
|
||||
.collect();
|
||||
let using_implicit_builtin_profile = permission_config_syntax.is_none()
|
||||
let using_implicit_builtin_profile = !effective_permission_selection
|
||||
.persisted_profile_id_was_provided
|
||||
&& permission_config_syntax.is_none()
|
||||
&& effective_permission_selection.selected_profile_id.is_none();
|
||||
let should_seed_legacy_workspace_roots = effective_permission_selection
|
||||
.selected_profile_id
|
||||
@@ -4387,18 +4407,31 @@ fn merge_managed_permission_profiles(
|
||||
}
|
||||
|
||||
fn resolve_effective_permission_selection<'a>(
|
||||
configured_permissions: Option<&PermissionsToml>,
|
||||
configured_profiles: Option<&PermissionsToml>,
|
||||
default_permissions_override: Option<&'a str>,
|
||||
configured_default_permissions: Option<&'a str>,
|
||||
persisted_profile_id: Option<&'a str>,
|
||||
configured_default_profile_id: Option<&'a str>,
|
||||
requirements_toml: &'a ConfigRequirementsToml,
|
||||
startup_warnings: &mut Vec<String>,
|
||||
) -> std::io::Result<EffectivePermissionSelection<'a>> {
|
||||
let profiles = merge_managed_permission_profiles(configured_permissions, requirements_toml)?;
|
||||
let profiles = merge_managed_permission_profiles(configured_profiles, requirements_toml)?;
|
||||
validate_user_permission_profile_names(profiles.as_ref())?;
|
||||
validate_required_permission_profile_catalog(requirements_toml, profiles.as_ref())?;
|
||||
let valid_persisted_profile_id = persisted_profile_id.filter(|profile_id| {
|
||||
is_builtin_permission_profile_name(profile_id)
|
||||
|| profiles.as_ref().is_some_and(|profiles| {
|
||||
compile_permission_profile_selection(
|
||||
Some(profiles),
|
||||
profile_id,
|
||||
/*workspace_write*/ None,
|
||||
&mut Vec::new(),
|
||||
)
|
||||
.is_ok()
|
||||
})
|
||||
});
|
||||
let selected_profile_id = resolve_default_permissions(
|
||||
default_permissions_override,
|
||||
configured_default_permissions,
|
||||
default_permissions_override.or(valid_persisted_profile_id),
|
||||
configured_default_profile_id,
|
||||
requirements_toml,
|
||||
startup_warnings,
|
||||
)?;
|
||||
@@ -4406,6 +4439,8 @@ fn resolve_effective_permission_selection<'a>(
|
||||
Ok(EffectivePermissionSelection {
|
||||
profiles,
|
||||
selected_profile_id,
|
||||
persisted_profile_id_was_provided: default_permissions_override.is_none()
|
||||
&& valid_persisted_profile_id.is_some(),
|
||||
requirements_force_profile_selection: requirements_toml
|
||||
.allowed_permission_profiles
|
||||
.is_some(),
|
||||
|
||||
@@ -413,6 +413,7 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result
|
||||
sandbox_mode,
|
||||
permission_profile: None,
|
||||
default_permissions: None,
|
||||
persisted_permission_profile_id: None,
|
||||
cwd: resolved_cwd,
|
||||
workspace_roots: None,
|
||||
model_provider: model_provider.clone(),
|
||||
|
||||
Reference in New Issue
Block a user