mirror of
https://github.com/openai/codex.git
synced 2026-11-09 08:40:33 +00:00
Default Guardian v2 to computer-use reviews with images (#40846)
## What changed - Default Guardian v2's review scope to computer-use tools. Set `features.guardianv2.review_scope.computer_use_only = false` to retain the broader tool review scope. - Include images in Guardian transcripts by default. Set `features.guardianv2.transcript.include_images = false` to disable them. ## Testing - Update configuration and app-server coverage for the new defaults and explicit broader-scope overrides. GitOrigin-RevId: 88c3cfe6dd2a96df052111794ad59c5c9ea39b78
This commit is contained in:
1 parent
a9ed4f154a
commit
9dea1f709f
5 files changed
+22
-32
No files matched your search
@@ -536,10 +536,11 @@ async fn guardian_v2_routes_scoped_tool_approvals(
|
|||||||
("approvals_reviewer = \"user\"", ApprovalsReviewer::User)
|
("approvals_reviewer = \"user\"", ApprovalsReviewer::User)
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
let guardian_scope_config = if matches!(scope, GuardianToolScope::ComputerUseOnly { .. }) {
|
let guardian_scope_config = match scope {
|
||||||
"\n\n[features.guardianv2]\nenabled = true\n\n[features.guardianv2.review_scope]\ncomputer_use_only = true"
|
GuardianToolScope::AllTools => {
|
||||||
} else {
|
"\n\n[features.guardianv2]\nenabled = true\n\n[features.guardianv2.review_scope]\ncomputer_use_only = false"
|
||||||
""
|
}
|
||||||
|
GuardianToolScope::ComputerUseOnly { .. } => "\n\n[features.guardianv2]\nenabled = true",
|
||||||
};
|
};
|
||||||
let mut mock_config = MockResponsesConfig::new(&responses_url)
|
let mut mock_config = MockResponsesConfig::new(&responses_url)
|
||||||
.with_model(MODEL)
|
.with_model(MODEL)
|
||||||
@@ -550,9 +551,6 @@ async fn guardian_v2_routes_scoped_tool_approvals(
|
|||||||
"[mcp_servers.{server_name}]\nurl = \"{mcp_server_url}/mcp\"\ndefault_tools_approval_mode = \"prompt\"\n\n[analytics]\nenabled = true\n\n[otel]\nmetrics_exporter = {{ otlp-http = {{ endpoint = \"{responses_url}/metrics\", protocol = \"json\" }} }}{guardian_scope_config}"
|
"[mcp_servers.{server_name}]\nurl = \"{mcp_server_url}/mcp\"\ndefault_tools_approval_mode = \"prompt\"\n\n[analytics]\nenabled = true\n\n[otel]\nmetrics_exporter = {{ otlp-http = {{ endpoint = \"{responses_url}/metrics\", protocol = \"json\" }} }}{guardian_scope_config}"
|
||||||
))
|
))
|
||||||
.enable_feature(Feature::GuardianApproval);
|
.enable_feature(Feature::GuardianApproval);
|
||||||
if matches!(scope, GuardianToolScope::AllTools) {
|
|
||||||
mock_config = mock_config.enable_feature(Feature::GuardianV2);
|
|
||||||
}
|
|
||||||
if lifecycle.has_user_input() || lifecycle.has_root_user_input() {
|
if lifecycle.has_user_input() || lifecycle.has_root_user_input() {
|
||||||
mock_config = mock_config.enable_feature(Feature::DefaultModeRequestUserInput);
|
mock_config = mock_config.enable_feature(Feature::DefaultModeRequestUserInput);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -245,7 +245,7 @@ impl GuardianV2Config {
|
|||||||
.review_scope
|
.review_scope
|
||||||
.as_ref()
|
.as_ref()
|
||||||
.and_then(|review_scope| review_scope.computer_use_only)
|
.and_then(|review_scope| review_scope.computer_use_only)
|
||||||
.unwrap_or(false)
|
.unwrap_or(true)
|
||||||
{
|
{
|
||||||
GuardianV2ReviewScope::ComputerUseOnly
|
GuardianV2ReviewScope::ComputerUseOnly
|
||||||
} else {
|
} else {
|
||||||
@@ -265,7 +265,7 @@ impl GuardianV2Config {
|
|||||||
}),
|
}),
|
||||||
include_images: transcript_config
|
include_images: transcript_config
|
||||||
.and_then(|transcript| transcript.include_images)
|
.and_then(|transcript| transcript.include_images)
|
||||||
.unwrap_or(false),
|
.unwrap_or(true),
|
||||||
max_message_entry_tokens,
|
max_message_entry_tokens,
|
||||||
max_tool_entry_tokens,
|
max_tool_entry_tokens,
|
||||||
max_message_transcript_tokens,
|
max_message_transcript_tokens,
|
||||||
|
|||||||
@@ -13,22 +13,17 @@ use super::GuardianV2ReviewScope;
|
|||||||
use crate::async_scorer::transcript::truncate_entry;
|
use crate::async_scorer::transcript::truncate_entry;
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn sandboxed_exec_commands_are_excluded_by_default() {
|
fn review_scope_is_computer_use_only_by_default() {
|
||||||
let config = GuardianV2Config::from_overrides(GuardianV2ConfigToml::default()).unwrap();
|
let config = GuardianV2Config::from_overrides(GuardianV2ConfigToml::default()).unwrap();
|
||||||
|
|
||||||
assert_eq!(
|
assert_eq!(config.review_scope, GuardianV2ReviewScope::ComputerUseOnly);
|
||||||
config.review_scope,
|
|
||||||
GuardianV2ReviewScope::Standard {
|
|
||||||
sandboxed_exec_commands: false,
|
|
||||||
}
|
|
||||||
);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn sandboxed_exec_commands_can_be_included() {
|
fn sandboxed_exec_commands_can_be_included() {
|
||||||
let config = GuardianV2Config::from_overrides(GuardianV2ConfigToml {
|
let config = GuardianV2Config::from_overrides(GuardianV2ConfigToml {
|
||||||
review_scope: Some(GuardianV2ReviewScopeConfigToml {
|
review_scope: Some(GuardianV2ReviewScopeConfigToml {
|
||||||
computer_use_only: None,
|
computer_use_only: Some(false),
|
||||||
sandboxed_exec_commands: Some(true),
|
sandboxed_exec_commands: Some(true),
|
||||||
}),
|
}),
|
||||||
..Default::default()
|
..Default::default()
|
||||||
|
|||||||
@@ -142,7 +142,7 @@ async fn installed_extension_reconnects_after_auth_refresh() -> Result<()> {
|
|||||||
.get::<GuardianV2ScoreProgress>()
|
.get::<GuardianV2ScoreProgress>()
|
||||||
.expect("Guardian v2 should initialize");
|
.expect("Guardian v2 should initialize");
|
||||||
let turn_store = ExtensionData::new("turn-1");
|
let turn_store = ExtensionData::new("turn-1");
|
||||||
let tool_name = ToolName::plain("read_file");
|
let tool_name = ToolName::namespaced("mcp__node_repl__", "js");
|
||||||
let payload = ToolPayload::Function {
|
let payload = ToolPayload::Function {
|
||||||
arguments: r#"{"path":"README.md"}"#.to_owned(),
|
arguments: r#"{"path":"README.md"}"#.to_owned(),
|
||||||
};
|
};
|
||||||
@@ -175,7 +175,7 @@ async fn installed_extension_reconnects_after_auth_refresh() -> Result<()> {
|
|||||||
.fast_approval_decision(
|
.fast_approval_decision(
|
||||||
&session_store,
|
&session_store,
|
||||||
thread_store,
|
thread_store,
|
||||||
"review action",
|
r#"{"tool":"mcp_tool_call","server":"node_repl"}"#,
|
||||||
/*extension_metrics*/ None,
|
/*extension_metrics*/ None,
|
||||||
)
|
)
|
||||||
.await,
|
.await,
|
||||||
@@ -675,7 +675,9 @@ async fn sample_configured_conversation_history(
|
|||||||
) -> Result<(serde_json::Value, TestCodex, ExtensionRegistry<Config>)> {
|
) -> Result<(serde_json::Value, TestCodex, ExtensionRegistry<Config>)> {
|
||||||
let thread_server = responses::start_mock_server().await;
|
let thread_server = responses::start_mock_server().await;
|
||||||
let guardian_policy = guardian_policy.map(str::to_owned);
|
let guardian_policy = guardian_policy.map(str::to_owned);
|
||||||
let guardian_config = guardian_config.to_owned();
|
let guardian_config = format!(
|
||||||
|
"{guardian_config}\n[features.guardianv2.review_scope]\ncomputer_use_only = false\n"
|
||||||
|
);
|
||||||
let has_model_defaults = model_defaults.is_some();
|
let has_model_defaults = model_defaults.is_some();
|
||||||
let builder = test_codex()
|
let builder = test_codex()
|
||||||
.with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing())
|
.with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing())
|
||||||
@@ -692,10 +694,8 @@ async fn sample_configured_conversation_history(
|
|||||||
.with_model("gpt-5.5")
|
.with_model("gpt-5.5")
|
||||||
.with_config(move |config| config.guardian_policy_config = guardian_policy)
|
.with_config(move |config| config.guardian_policy_config = guardian_policy)
|
||||||
.with_pre_build_hook(move |home| {
|
.with_pre_build_hook(move |home| {
|
||||||
if !guardian_config.is_empty() {
|
std::fs::write(home.join("config.toml"), guardian_config)
|
||||||
std::fs::write(home.join("config.toml"), guardian_config)
|
.expect("Guardian v2 configuration should be written");
|
||||||
.expect("Guardian v2 configuration should be written");
|
|
||||||
}
|
|
||||||
});
|
});
|
||||||
let mut builder = if let Some(model_defaults) = model_defaults {
|
let mut builder = if let Some(model_defaults) = model_defaults {
|
||||||
builder.with_model_info_override("gpt-5.5", move |model| {
|
builder.with_model_info_override("gpt-5.5", move |model| {
|
||||||
@@ -1387,7 +1387,7 @@ max_recent_non_user_entries = 8
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||||
async fn contributor_includes_configured_transcript_images() -> Result<()> {
|
async fn contributor_includes_transcript_images_by_default() -> Result<()> {
|
||||||
skip_if_no_network!(Ok(()));
|
skip_if_no_network!(Ok(()));
|
||||||
|
|
||||||
let history = vec![
|
let history = vec![
|
||||||
@@ -1426,9 +1426,6 @@ async fn contributor_includes_configured_transcript_images() -> Result<()> {
|
|||||||
let configuration = r#"
|
let configuration = r#"
|
||||||
[features.guardianv2]
|
[features.guardianv2]
|
||||||
enabled = true
|
enabled = true
|
||||||
|
|
||||||
[features.guardianv2.transcript]
|
|
||||||
include_images = true
|
|
||||||
"#;
|
"#;
|
||||||
let (request, _test, _registry) = sample_configured_conversation_history(
|
let (request, _test, _registry) = sample_configured_conversation_history(
|
||||||
history,
|
history,
|
||||||
@@ -1869,7 +1866,7 @@ async fn contributor_skips_models_requiring_managed_guardian_review() -> Result<
|
|||||||
.fast_approval_decision(
|
.fast_approval_decision(
|
||||||
&session_store,
|
&session_store,
|
||||||
thread_store,
|
thread_store,
|
||||||
"review action",
|
r#"{"tool":"mcp_tool_call","server":"node_repl"}"#,
|
||||||
/*extension_metrics*/ None,
|
/*extension_metrics*/ None,
|
||||||
)
|
)
|
||||||
.await,
|
.await,
|
||||||
@@ -1877,7 +1874,7 @@ async fn contributor_skips_models_requiring_managed_guardian_review() -> Result<
|
|||||||
);
|
);
|
||||||
|
|
||||||
let turn_store = ExtensionData::new("turn-1");
|
let turn_store = ExtensionData::new("turn-1");
|
||||||
let tool_name = ToolName::plain("read_file");
|
let tool_name = ToolName::namespaced("mcp__node_repl__", "js");
|
||||||
let payload = ToolPayload::Function {
|
let payload = ToolPayload::Function {
|
||||||
arguments: json!({ "path": "protected.md" }).to_string(),
|
arguments: json!({ "path": "protected.md" }).to_string(),
|
||||||
};
|
};
|
||||||
@@ -2262,7 +2259,7 @@ async fn contributor_reuses_the_latest_compatible_parent_compaction() -> Result<
|
|||||||
.with_pre_build_hook(|home| {
|
.with_pre_build_hook(|home| {
|
||||||
std::fs::write(
|
std::fs::write(
|
||||||
home.join("config.toml"),
|
home.join("config.toml"),
|
||||||
"[features.guardianv2]\nenabled = true\nmax_parent_compaction_tokens = 256\n",
|
"[features.guardianv2]\nenabled = true\nmax_parent_compaction_tokens = 256\n\n[features.guardianv2.review_scope]\ncomputer_use_only = false\n",
|
||||||
)
|
)
|
||||||
.expect("Guardian v2 parent compaction configuration should be written");
|
.expect("Guardian v2 parent compaction configuration should be written");
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -69,7 +69,7 @@ impl Default for TranscriptConfig {
|
|||||||
fn default() -> Self {
|
fn default() -> Self {
|
||||||
Self {
|
Self {
|
||||||
sources: vec![TranscriptSource::ToolCalls, TranscriptSource::ToolOutputs],
|
sources: vec![TranscriptSource::ToolCalls, TranscriptSource::ToolOutputs],
|
||||||
include_images: false,
|
include_images: true,
|
||||||
max_message_entry_tokens: MAX_MESSAGE_ENTRY_TOKENS,
|
max_message_entry_tokens: MAX_MESSAGE_ENTRY_TOKENS,
|
||||||
max_tool_entry_tokens: MAX_TOOL_ENTRY_TOKENS,
|
max_tool_entry_tokens: MAX_TOOL_ENTRY_TOKENS,
|
||||||
max_message_transcript_tokens: MAX_MESSAGE_TRANSCRIPT_TOKENS,
|
max_message_transcript_tokens: MAX_MESSAGE_TRANSCRIPT_TOKENS,
|
||||||
|
|||||||
Reference in new issue
Block a user