mirror of
https://github.com/openai/codex.git
synced 2026-11-10 21:00:47 +00:00
Serialize default thread permissions as a permission profile (#60472)
## What changed Initialize thread metadata's `sandbox_policy` with a serialized `PermissionProfile::read_only()`, matching the format used by turn contexts and session settings. Remove the legacy `SandboxPolicy` field from `ThreadMetadataBuilder` and redundant rollout metadata defaults. ## Testing Extend thread-store tests to cover reading the default read-only permission profile from SQLite and accepting both string and serialized legacy `danger-full-access` policies. GitOrigin-RevId: ec9b88a47799a20529e28ef8ca6f5f7c2c8faf9e
This commit is contained in:
@@ -11,8 +11,6 @@ use chrono::Timelike;
|
||||
use chrono::Utc;
|
||||
use codex_protocol::RolloutId;
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use codex_protocol::protocol::SessionMeta;
|
||||
use codex_protocol::protocol::SessionMetaLine;
|
||||
use codex_protocol::protocol::SessionSource;
|
||||
@@ -59,8 +57,6 @@ pub(crate) fn builder_from_session_meta(
|
||||
builder.agent_path = session_meta.meta.agent_path.clone();
|
||||
builder.cwd = session_meta.meta.cwd.clone();
|
||||
builder.cli_version = Some(session_meta.meta.cli_version.clone());
|
||||
builder.sandbox_policy = SandboxPolicy::new_read_only_policy();
|
||||
builder.approval_mode = AskForApproval::OnRequest;
|
||||
if let Some(git) = session_meta.git.as_ref() {
|
||||
builder.git_sha = git.commit_hash.as_ref().map(|sha| sha.0.clone());
|
||||
builder.git_branch = git.branch.clone();
|
||||
|
||||
@@ -852,7 +852,8 @@ mod tests {
|
||||
title: String::new(),
|
||||
name: None,
|
||||
preview: None,
|
||||
sandbox_policy: "read-only".to_string(),
|
||||
sandbox_policy: serde_json::to_string(&PermissionProfile::read_only())
|
||||
.unwrap_or_default(),
|
||||
approval_mode: "on-request".to_string(),
|
||||
tokens_used: 1,
|
||||
first_user_message: None,
|
||||
|
||||
@@ -3,9 +3,9 @@ use chrono::DateTime;
|
||||
use chrono::Utc;
|
||||
use codex_protocol::SanitizedGitUrl;
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::openai_models::ReasoningEffort;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use codex_protocol::protocol::SessionSource;
|
||||
use codex_protocol::protocol::ThreadHistoryMode;
|
||||
use codex_protocol::protocol::ThreadSource;
|
||||
@@ -171,7 +171,7 @@ pub struct ThreadMetadata {
|
||||
pub name: Option<String>,
|
||||
/// Best available user-facing preview for discovery and list display.
|
||||
pub preview: Option<String>,
|
||||
/// The sandbox policy (stringified enum).
|
||||
/// Serialized permission profile or legacy sandbox value.
|
||||
pub sandbox_policy: String,
|
||||
/// The approval mode (stringified enum).
|
||||
pub approval_mode: String,
|
||||
@@ -236,8 +236,6 @@ pub struct ThreadMetadataBuilder {
|
||||
pub cwd: PathBuf,
|
||||
/// Version of the CLI that created the thread.
|
||||
pub cli_version: Option<String>,
|
||||
/// The sandbox policy.
|
||||
pub sandbox_policy: SandboxPolicy,
|
||||
/// The approval mode.
|
||||
pub approval_mode: AskForApproval,
|
||||
/// The archive timestamp, if the thread is archived.
|
||||
@@ -276,7 +274,6 @@ impl ThreadMetadataBuilder {
|
||||
model_provider: None,
|
||||
cwd: PathBuf::new(),
|
||||
cli_version: None,
|
||||
sandbox_policy: SandboxPolicy::new_read_only_policy(),
|
||||
approval_mode: AskForApproval::OnRequest,
|
||||
archived_at: None,
|
||||
git_sha: None,
|
||||
@@ -288,7 +285,6 @@ impl ThreadMetadataBuilder {
|
||||
/// Build canonical thread metadata, filling missing values from defaults.
|
||||
pub fn build(&self, default_provider: &str) -> ThreadMetadata {
|
||||
let source = crate::extract::enum_to_string(&self.source);
|
||||
let sandbox_policy = crate::extract::enum_to_string(&self.sandbox_policy);
|
||||
let approval_mode = crate::extract::enum_to_string(&self.approval_mode);
|
||||
let created_at = canonicalize_datetime(self.created_at);
|
||||
let updated_at = self
|
||||
@@ -335,7 +331,8 @@ impl ThreadMetadataBuilder {
|
||||
name: (guardian_review && self.history_mode == ThreadHistoryMode::Paginated)
|
||||
.then(|| crate::GUARDIAN_THREAD_TITLE.to_string()),
|
||||
preview: guardian_review.then(|| crate::GUARDIAN_THREAD_PREVIEW.to_string()),
|
||||
sandbox_policy,
|
||||
sandbox_policy: serde_json::to_string(&PermissionProfile::read_only())
|
||||
.unwrap_or_default(),
|
||||
approval_mode,
|
||||
tokens_used: 0,
|
||||
first_user_message: None,
|
||||
|
||||
@@ -5,12 +5,12 @@ use chrono::Utc;
|
||||
#[cfg(test)]
|
||||
use codex_protocol::ThreadId;
|
||||
#[cfg(test)]
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
#[cfg(test)]
|
||||
use codex_protocol::openai_models::ReasoningEffort;
|
||||
#[cfg(test)]
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
#[cfg(test)]
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
#[cfg(test)]
|
||||
use codex_protocol::protocol::ThreadHistoryMode;
|
||||
#[cfg(test)]
|
||||
use std::path::Path;
|
||||
@@ -68,7 +68,7 @@ pub(super) fn test_thread_metadata(
|
||||
title: String::new(),
|
||||
name: None,
|
||||
preview: Some("hello".to_string()),
|
||||
sandbox_policy: crate::extract::enum_to_string(&SandboxPolicy::new_read_only_policy()),
|
||||
sandbox_policy: serde_json::to_string(&PermissionProfile::read_only()).unwrap_or_default(),
|
||||
approval_mode: crate::extract::enum_to_string(&AskForApproval::OnRequest),
|
||||
tokens_used: 0,
|
||||
first_user_message: Some("hello".to_string()),
|
||||
|
||||
@@ -1038,25 +1038,34 @@ mod tests {
|
||||
builder.model_provider = Some(config.default_model_provider_id.clone());
|
||||
builder.cwd = home.path().to_path_buf();
|
||||
let mut metadata = builder.build(config.default_model_provider_id.as_str());
|
||||
let permission_profile: PermissionProfile = PermissionProfile::Disabled;
|
||||
metadata.sandbox_policy =
|
||||
serde_json::to_string(&permission_profile).expect("serialize profile");
|
||||
runtime
|
||||
.upsert_thread(&metadata)
|
||||
.await
|
||||
.expect("state db upsert should succeed");
|
||||
for (value, expected) in [
|
||||
(
|
||||
metadata.sandbox_policy.clone(),
|
||||
PermissionProfile::read_only(),
|
||||
),
|
||||
(
|
||||
serde_json::to_string(&PermissionProfile::Disabled).expect("serialize profile"),
|
||||
PermissionProfile::Disabled,
|
||||
),
|
||||
] {
|
||||
metadata.sandbox_policy = value;
|
||||
runtime
|
||||
.upsert_thread(&metadata)
|
||||
.await
|
||||
.expect("state db upsert should succeed");
|
||||
|
||||
let thread = store
|
||||
.read_thread(ReadThreadParams {
|
||||
thread_id,
|
||||
include_archived: false,
|
||||
include_history: false,
|
||||
})
|
||||
.await
|
||||
.expect("read thread");
|
||||
let thread = store
|
||||
.read_thread(ReadThreadParams {
|
||||
thread_id,
|
||||
include_archived: false,
|
||||
include_history: false,
|
||||
})
|
||||
.await
|
||||
.expect("read thread");
|
||||
|
||||
assert_eq!(thread.preview, "Hello from user");
|
||||
assert_eq!(thread.permission_profile, PermissionProfile::Disabled);
|
||||
assert_eq!(thread.preview, "Hello from user");
|
||||
assert_eq!(thread.permission_profile, expected);
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -1079,22 +1088,28 @@ mod tests {
|
||||
builder.model_provider = Some(config.default_model_provider_id.clone());
|
||||
builder.cwd = home.path().to_path_buf();
|
||||
let mut metadata = builder.build(config.default_model_provider_id.as_str());
|
||||
metadata.sandbox_policy = "danger-full-access".to_string();
|
||||
runtime
|
||||
.upsert_thread(&metadata)
|
||||
.await
|
||||
.expect("state db upsert should succeed");
|
||||
for value in [
|
||||
"danger-full-access".to_string(),
|
||||
serde_json::to_string(&SandboxPolicy::DangerFullAccess)
|
||||
.expect("serialize legacy policy"),
|
||||
] {
|
||||
metadata.sandbox_policy = value;
|
||||
runtime
|
||||
.upsert_thread(&metadata)
|
||||
.await
|
||||
.expect("state db upsert should succeed");
|
||||
|
||||
let thread = store
|
||||
.read_thread(ReadThreadParams {
|
||||
thread_id,
|
||||
include_archived: false,
|
||||
include_history: true,
|
||||
})
|
||||
.await
|
||||
.expect("read thread");
|
||||
let thread = store
|
||||
.read_thread(ReadThreadParams {
|
||||
thread_id,
|
||||
include_archived: false,
|
||||
include_history: true,
|
||||
})
|
||||
.await
|
||||
.expect("read thread");
|
||||
|
||||
assert_eq!(thread.permission_profile, PermissionProfile::Disabled);
|
||||
assert_eq!(thread.permission_profile, PermissionProfile::Disabled);
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
||||
Reference in New Issue
Block a user