mirror of
https://github.com/openai/codex.git
synced 2026-10-21 16:38:24 +00:00
Include auto-review state in turn metadata (#38046)
## What changed - Add `auto_review_enabled` to serialized turn metadata for regular and review turns. - Derive the value from approval-policy routing and reserve the metadata key so client-provided metadata cannot override it. ## Testing - Cover enabled metadata, reserved-key handling, Guardian review requests, and updated compaction request snapshots. GitOrigin-RevId: 52773e2bf7d4be251f8466332de9f9c9869dd084
This commit is contained in:
@@ -42,6 +42,7 @@ pub(crate) const SUBAGENT_KIND_KEY: &str = "subagent_kind";
|
||||
pub(crate) const THREAD_SOURCE_KEY: &str = "thread_source";
|
||||
pub(crate) const SANDBOX_KEY: &str = "sandbox";
|
||||
pub(crate) const SANDBOX_MODE_KEY: &str = "sandbox_mode";
|
||||
pub(crate) const AUTO_REVIEW_ENABLED_KEY: &str = "auto_review_enabled";
|
||||
pub(crate) const WORKSPACES_KEY: &str = "workspaces";
|
||||
|
||||
// App-server clients can specify additional metadata in the `responsesapi_client_metadata` param
|
||||
@@ -69,6 +70,7 @@ const RESERVED_METADATA_KEYS: &[&str] = &[
|
||||
THREAD_SOURCE_KEY,
|
||||
SANDBOX_KEY,
|
||||
SANDBOX_MODE_KEY,
|
||||
AUTO_REVIEW_ENABLED_KEY,
|
||||
WORKSPACES_KEY,
|
||||
];
|
||||
const MAX_EXTRA_METADATA_ENTRIES: usize = 16;
|
||||
@@ -207,6 +209,7 @@ pub struct CodexResponsesMetadata {
|
||||
pub(crate) thread_source: Option<ThreadSource>,
|
||||
pub(crate) sandbox: Option<String>,
|
||||
pub(crate) sandbox_mode: Option<String>,
|
||||
pub(crate) auto_review_enabled: Option<bool>,
|
||||
pub(crate) workspaces: BTreeMap<String, TurnMetadataWorkspace>,
|
||||
pub(crate) tool_namespaces_info: Option<TurnToolNamespacesInfo>,
|
||||
pub(crate) turn_started_at_unix_ms: Option<i64>,
|
||||
@@ -236,6 +239,7 @@ impl CodexResponsesMetadata {
|
||||
thread_source: None,
|
||||
sandbox: None,
|
||||
sandbox_mode: None,
|
||||
auto_review_enabled: None,
|
||||
workspaces: BTreeMap::new(),
|
||||
tool_namespaces_info: None,
|
||||
turn_started_at_unix_ms: None,
|
||||
@@ -347,6 +351,7 @@ impl CodexResponsesMetadata {
|
||||
thread_source: self.thread_source.as_ref(),
|
||||
sandbox: self.sandbox.as_deref(),
|
||||
sandbox_mode: self.sandbox_mode.as_deref(),
|
||||
auto_review_enabled: self.auto_review_enabled,
|
||||
workspaces: non_empty_workspaces(&self.workspaces),
|
||||
tool_namespaces_info: self.tool_namespaces_info.as_ref(),
|
||||
turn_started_at_unix_ms: self.turn_started_at_unix_ms,
|
||||
@@ -471,6 +476,8 @@ struct CodexTurnMetadataPayload<'a> {
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
sandbox_mode: Option<&'a str>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
auto_review_enabled: Option<bool>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
workspaces: Option<&'a BTreeMap<String, TurnMetadataWorkspace>>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
tool_namespaces_info: Option<&'a TurnToolNamespacesInfo>,
|
||||
|
||||
@@ -100,6 +100,10 @@ pub(super) async fn spawn_review_thread(
|
||||
};
|
||||
per_turn_config.service_tier = service_tier;
|
||||
|
||||
let auto_review_enabled = crate::guardian::routes_approval_policy_to_guardian(
|
||||
per_turn_config.permissions.approval_policy.value(),
|
||||
per_turn_config.approvals_reviewer,
|
||||
);
|
||||
let per_turn_config = Arc::new(per_turn_config);
|
||||
let review_turn_id = sub_id.to_string();
|
||||
let turn_metadata_state = Arc::new(TurnMetadataState::new(
|
||||
@@ -115,6 +119,7 @@ pub(super) async fn spawn_review_thread(
|
||||
&parent_turn_context.permission_profile(),
|
||||
parent_turn_context.windows_sandbox_level,
|
||||
parent_turn_context.network.is_some(),
|
||||
auto_review_enabled,
|
||||
));
|
||||
|
||||
let extension_data = Arc::new(codex_extension_api::ExtensionData::new(
|
||||
|
||||
@@ -571,6 +571,10 @@ impl Session {
|
||||
&model_info,
|
||||
);
|
||||
let permission_profile = per_turn_config.permissions.effective_permission_profile();
|
||||
let auto_review_enabled = crate::guardian::routes_approval_policy_to_guardian(
|
||||
per_turn_config.permissions.approval_policy.value(),
|
||||
per_turn_config.approvals_reviewer,
|
||||
);
|
||||
let per_turn_config = Arc::new(per_turn_config);
|
||||
let turn_metadata_state = Arc::new(TurnMetadataState::new(
|
||||
session_id.to_string(),
|
||||
@@ -584,6 +588,7 @@ impl Session {
|
||||
&permission_profile,
|
||||
session_configuration.windows_sandbox_level,
|
||||
network.is_some(),
|
||||
auto_review_enabled,
|
||||
));
|
||||
turn_metadata_state
|
||||
.set_responses_api_metadata(per_turn_config.responses_api_metadata.clone());
|
||||
|
||||
@@ -107,6 +107,7 @@ pub(crate) struct TurnMetadataState {
|
||||
turn_id: String,
|
||||
sandbox: Option<String>,
|
||||
sandbox_mode: Option<String>,
|
||||
auto_review_enabled: bool,
|
||||
enriched_workspaces: RwLock<Option<BTreeMap<String, TurnMetadataWorkspace>>>,
|
||||
tool_namespaces_info: RwLock<Option<TurnToolNamespacesInfo>>,
|
||||
turn_started_at_unix_ms: RwLock<Option<i64>>,
|
||||
@@ -130,6 +131,7 @@ impl TurnMetadataState {
|
||||
permission_profile: &PermissionProfile,
|
||||
windows_sandbox_level: WindowsSandboxLevel,
|
||||
enforce_managed_network: bool,
|
||||
auto_review_enabled: bool,
|
||||
) -> Self {
|
||||
let repo_root = get_git_repo_root(&cwd);
|
||||
let sandbox = Some(
|
||||
@@ -156,6 +158,7 @@ impl TurnMetadataState {
|
||||
turn_id,
|
||||
sandbox,
|
||||
sandbox_mode,
|
||||
auto_review_enabled,
|
||||
enriched_workspaces: RwLock::new(None),
|
||||
tool_namespaces_info: RwLock::new(None),
|
||||
turn_started_at_unix_ms: RwLock::new(None),
|
||||
@@ -304,6 +307,7 @@ impl TurnMetadataState {
|
||||
thread_source: self.thread_source.clone(),
|
||||
sandbox: self.sandbox.clone(),
|
||||
sandbox_mode: self.sandbox_mode.clone(),
|
||||
auto_review_enabled: Some(self.auto_review_enabled),
|
||||
workspaces: self.current_workspaces(),
|
||||
tool_namespaces_info: self
|
||||
.tool_namespaces_info
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
use super::*;
|
||||
|
||||
use crate::responses_metadata::AUTO_REVIEW_ENABLED_KEY;
|
||||
use crate::responses_metadata::CodexResponsesRequestKind;
|
||||
use crate::responses_metadata::CompactionTurnMetadata;
|
||||
use crate::responses_metadata::INSTALLATION_ID_KEY;
|
||||
@@ -231,12 +232,14 @@ fn turn_metadata_state_includes_sandbox_metadata() {
|
||||
&permission_profile,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
/*enforce_managed_network*/ false,
|
||||
/*auto_review_enabled*/ true,
|
||||
);
|
||||
|
||||
let header = test_turn_metadata_header(&state);
|
||||
let json: Value = serde_json::from_str(&header).expect("json");
|
||||
let sandbox_name = json.get("sandbox").and_then(Value::as_str);
|
||||
let sandbox_mode = json.get(SANDBOX_MODE_KEY).and_then(Value::as_str);
|
||||
let auto_review_enabled = json.get(AUTO_REVIEW_ENABLED_KEY).and_then(Value::as_bool);
|
||||
let session_id = json.get("session_id").and_then(Value::as_str);
|
||||
let thread_id = json.get("thread_id").and_then(Value::as_str);
|
||||
|
||||
@@ -248,6 +251,7 @@ fn turn_metadata_state_includes_sandbox_metadata() {
|
||||
);
|
||||
assert_eq!(sandbox_name, Some(expected_sandbox));
|
||||
assert_eq!(sandbox_mode, Some("read-only"));
|
||||
assert_eq!(auto_review_enabled, Some(true));
|
||||
assert_eq!(session_id, Some("session-a"));
|
||||
assert_eq!(thread_id, Some("thread-a"));
|
||||
assert!(json.get("forked_from_thread_id").is_none());
|
||||
@@ -276,6 +280,7 @@ fn turn_metadata_state_includes_root_fork_lineage() {
|
||||
&permission_profile,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
/*enforce_managed_network*/ false,
|
||||
/*auto_review_enabled*/ false,
|
||||
);
|
||||
|
||||
let header = test_turn_metadata_header(&state);
|
||||
@@ -315,6 +320,7 @@ fn turn_metadata_state_includes_thread_spawn_subagent_parent_without_fork() {
|
||||
&permission_profile,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
/*enforce_managed_network*/ false,
|
||||
/*auto_review_enabled*/ false,
|
||||
);
|
||||
|
||||
let header = test_turn_metadata_header(&state);
|
||||
@@ -354,6 +360,7 @@ fn turn_metadata_state_includes_forked_thread_spawn_subagent_lineage() {
|
||||
&permission_profile,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
/*enforce_managed_network*/ false,
|
||||
/*auto_review_enabled*/ false,
|
||||
);
|
||||
|
||||
let header = test_turn_metadata_header(&state);
|
||||
@@ -395,6 +402,7 @@ fn turn_metadata_state_includes_known_parent_for_non_thread_spawn_subagents_with
|
||||
&permission_profile,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
/*enforce_managed_network*/ false,
|
||||
/*auto_review_enabled*/ false,
|
||||
);
|
||||
|
||||
let header = test_turn_metadata_header(&state);
|
||||
@@ -427,6 +435,7 @@ fn turn_metadata_state_includes_turn_started_at_unix_ms_after_start() {
|
||||
&permission_profile,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
/*enforce_managed_network*/ false,
|
||||
/*auto_review_enabled*/ false,
|
||||
);
|
||||
state.set_turn_started_at_unix_ms(/*turn_started_at_unix_ms*/ 1_700_000_000_123);
|
||||
|
||||
@@ -457,6 +466,7 @@ fn turn_metadata_state_includes_model_and_reasoning_effort_only_in_request_meta(
|
||||
&permission_profile,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
/*enforce_managed_network*/ false,
|
||||
/*auto_review_enabled*/ false,
|
||||
);
|
||||
|
||||
let header = test_turn_metadata_header(&state);
|
||||
@@ -506,6 +516,7 @@ fn turn_metadata_state_marks_user_input_requested_during_turn_only_for_mcp_reque
|
||||
&permission_profile,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
/*enforce_managed_network*/ false,
|
||||
/*auto_review_enabled*/ false,
|
||||
);
|
||||
|
||||
let header = test_turn_metadata_header(&state);
|
||||
@@ -559,6 +570,7 @@ fn turn_metadata_state_ignores_client_reserved_metadata_before_start() {
|
||||
&permission_profile,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
/*enforce_managed_network*/ false,
|
||||
/*auto_review_enabled*/ false,
|
||||
);
|
||||
state.set_responsesapi_client_metadata(HashMap::from([
|
||||
(
|
||||
@@ -587,6 +599,7 @@ fn turn_metadata_state_ignores_client_reserved_metadata_before_start() {
|
||||
SANDBOX_MODE_KEY.to_string(),
|
||||
"danger-full-access".to_string(),
|
||||
),
|
||||
(AUTO_REVIEW_ENABLED_KEY.to_string(), "true".to_string()),
|
||||
]));
|
||||
|
||||
let header = test_turn_metadata_header(&state);
|
||||
@@ -600,6 +613,7 @@ fn turn_metadata_state_ignores_client_reserved_metadata_before_start() {
|
||||
assert!(json.get("parent_turn_id").is_none());
|
||||
assert!(json.get("subagent_kind").is_none());
|
||||
assert_eq!(json[SANDBOX_MODE_KEY].as_str(), Some("read-only"));
|
||||
assert_eq!(json[AUTO_REVIEW_ENABLED_KEY].as_bool(), Some(false));
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -630,6 +644,7 @@ fn turn_metadata_state_merges_client_metadata_without_replacing_reserved_fields(
|
||||
&permission_profile,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
/*enforce_managed_network*/ false,
|
||||
/*auto_review_enabled*/ false,
|
||||
);
|
||||
state.set_responses_api_metadata(BTreeMap::from([(
|
||||
"codex_security_surface".to_string(),
|
||||
@@ -845,6 +860,7 @@ fn turn_metadata_state_overlays_compaction_only_on_compaction_requests() {
|
||||
&permission_profile,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
/*enforce_managed_network*/ false,
|
||||
/*auto_review_enabled*/ false,
|
||||
);
|
||||
state.set_responses_api_metadata(BTreeMap::from([(
|
||||
"codex_security_surface".to_string(),
|
||||
@@ -924,6 +940,7 @@ async fn turn_metadata_state_preserves_lineage_after_git_enrichment() {
|
||||
&permission_profile,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
/*enforce_managed_network*/ false,
|
||||
/*auto_review_enabled*/ false,
|
||||
));
|
||||
|
||||
state.spawn_git_enrichment_task();
|
||||
@@ -966,6 +983,7 @@ async fn turn_metadata_state_coalesces_concurrent_git_enrichment() {
|
||||
&permission_profile,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
/*enforce_managed_network*/ false,
|
||||
/*auto_review_enabled*/ false,
|
||||
));
|
||||
let barrier = Arc::new(tokio::sync::Barrier::new(8));
|
||||
let tasks = (0..8)
|
||||
@@ -1019,6 +1037,7 @@ async fn turn_metadata_state_git_enrichment_cancellation_is_retryable_and_errors
|
||||
&permission_profile,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
/*enforce_managed_network*/ false,
|
||||
/*auto_review_enabled*/ false,
|
||||
));
|
||||
state.spawn_git_enrichment_task();
|
||||
state.cancel_git_enrichment_task();
|
||||
@@ -1052,6 +1071,7 @@ async fn turn_metadata_state_git_enrichment_cancellation_is_retryable_and_errors
|
||||
&permission_profile,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
/*enforce_managed_network*/ false,
|
||||
/*auto_review_enabled*/ false,
|
||||
));
|
||||
invalid_state.spawn_git_enrichment_task();
|
||||
tokio::time::timeout(Duration::from_secs(2), async {
|
||||
|
||||
@@ -211,6 +211,10 @@ async fn guardian_prewarm_and_review_skip_redundant_git_enrichment() -> Result<(
|
||||
.await?;
|
||||
let user_turn = user_turn.body_json();
|
||||
let guardian_turn = guardian_turn.body_json();
|
||||
assert_eq!(
|
||||
turn_metadata(&user_turn)?["auto_review_enabled"].as_bool(),
|
||||
Some(true)
|
||||
);
|
||||
assert_eq!(
|
||||
turn_metadata(&user_turn)?["workspaces"],
|
||||
expected_workspace(repo.path(), &head, /*has_changes*/ true)
|
||||
|
||||
+1
-1
@@ -11,7 +11,7 @@ Scenario: After five varied API-key-auth turns, remote manual compaction omits s
|
||||
- "thread_id": "<UUID>",
|
||||
- "turn_id": "<UUID>",
|
||||
- "x-codex-installation-id": "<UUID>",
|
||||
- "x-codex-turn-metadata": "{\"installation_id\":\"<UUID>\",\"session_id\":\"<UUID>\",\"thread_id\":\"<UUID>\",\"turn_id\":\"<UUID>\",\"window_id\":\"<UUID>:0\",\"request_kind\":\"turn\",\"sandbox\":\"<SANDBOX>\",\"sandbox_mode\":\"read-only\",\"turn_started_at_unix_ms\":<UNIX_MS>}",
|
||||
- "x-codex-turn-metadata": "{\"installation_id\":\"<UUID>\",\"session_id\":\"<UUID>\",\"thread_id\":\"<UUID>\",\"turn_id\":\"<UUID>\",\"window_id\":\"<UUID>:0\",\"request_kind\":\"turn\",\"sandbox\":\"<SANDBOX>\",\"sandbox_mode\":\"read-only\",\"auto_review_enabled\":false,\"turn_started_at_unix_ms\":<UNIX_MS>}",
|
||||
- "x-codex-window-id": "<UUID>:0"
|
||||
- },
|
||||
- "include": [
|
||||
|
||||
+1
-1
@@ -11,7 +11,7 @@ Scenario: After five varied ChatGPT-auth turns, remote manual compaction reuses
|
||||
- "thread_id": "<UUID>",
|
||||
- "turn_id": "<UUID>",
|
||||
- "x-codex-installation-id": "<UUID>",
|
||||
- "x-codex-turn-metadata": "{\"installation_id\":\"<UUID>\",\"session_id\":\"<UUID>\",\"thread_id\":\"<UUID>\",\"turn_id\":\"<UUID>\",\"window_id\":\"<UUID>:0\",\"request_kind\":\"turn\",\"sandbox\":\"<SANDBOX>\",\"sandbox_mode\":\"read-only\",\"turn_started_at_unix_ms\":<UNIX_MS>}",
|
||||
- "x-codex-turn-metadata": "{\"installation_id\":\"<UUID>\",\"session_id\":\"<UUID>\",\"thread_id\":\"<UUID>\",\"turn_id\":\"<UUID>\",\"window_id\":\"<UUID>:0\",\"request_kind\":\"turn\",\"sandbox\":\"<SANDBOX>\",\"sandbox_mode\":\"read-only\",\"auto_review_enabled\":false,\"turn_started_at_unix_ms\":<UNIX_MS>}",
|
||||
- "x-codex-window-id": "<UUID>:0"
|
||||
- },
|
||||
- "include": [
|
||||
|
||||
Reference in New Issue
Block a user