Allow model catalogs to customize approval instruction headings (#57581)

## What changed

Add optional `request_permissions` and `auto_review_suffix` fields to catalog approval messages. Use them to customize the permission tool heading and introduce automatic review guidance while preserving the bundled instruction bodies and existing policy gates. Accept `request_permissions_tool` as a deserialization alias.

Limit headings to 500 UTF-8 bytes on a single line and fall back to bundled text for missing or invalid values. Complete catalog approval messages retain precedence.

## Testing

Add coverage for heading validation, serialization aliases, policy gates, fallback behavior, and complete-message precedence. Integration tests verify that model switches append the selected model's headings without rewriting history or repeating unchanged instructions.

GitOrigin-RevId: 3311591a464cc086783fe25271a352dd5a229790
This commit is contained in:
goodman-oai
2026-10-27 22:26:12 +00:00
committed by copyberry
parent e343a12d5c
commit 133c11db43
17 changed files with 658 additions and 23 deletions
+3 -1
View File
@@ -18,7 +18,9 @@ codex_rust_crate(
"//codex-rs/windows-sandbox-rs:codex-windows-managed-deny-probe",
"//codex-rs/windows-sandbox-rs:codex-windows-sandbox-setup",
],
integration_compile_data_extra = glob(["assets/**"]),
integration_compile_data_extra = glob(["assets/**"]) + [
"//codex-rs/prompts:templates/permissions/approval_policy/on_request_rule_request_permission.md",
],
integration_test_timeout = "long",
run_tests_with_wine_exec = True,
rustc_env = {
@@ -264,6 +264,7 @@ fn permissions_state(
never: None,
unless_trusted: None,
granular: None,
..Default::default()
};
let permission_messages = PermissionMessages {
danger_full_access: Some("Full access.".to_string()),
@@ -50,6 +50,7 @@ async fn catalog_permission_message_loaded_from_remote_models_is_sent(
never: None,
unless_trusted: None,
granular: None,
..Default::default()
}),
collaboration_modes: None,
auto_review: None,
+127 -11
View File
@@ -4,7 +4,9 @@ use codex_core::ForkSnapshot;
use codex_core::TurnInputRequest;
use codex_core::config::Constrained;
use codex_exec_server::CreateDirectoryOptions;
use codex_features::Feature;
use codex_models_manager::model_info::model_info_from_slug;
use codex_protocol::config_types::ApprovalsReviewer;
use codex_protocol::models::PermissionProfile;
use codex_protocol::openai_models::ApprovalMessages;
use codex_protocol::openai_models::ModelMessages;
@@ -35,6 +37,9 @@ use std::collections::HashSet;
use tempfile::TempDir;
use test_case::test_case;
const REQUEST_TOOL_BODY: &str = "The built-in `request_permissions` tool is available in this session. Invoke it when you need to request additional `network` or `file_system` permissions before later shell-like commands need them. Request only the specific permissions required for the task.";
const AUTO_REVIEW_BODY: &str = "`approvals_reviewer` is `auto_review`: Sandbox escalations with require_escalated will be reviewed for compliance with the policy. If a rejection happens, you should proceed only with a materially safer alternative, or inform the user of the risk and send a final message to ask for approval.";
fn permissions_texts(request: &ResponsesRequest) -> Vec<String> {
request
.message_input_texts("developer")
@@ -87,9 +92,7 @@ fn model_with_approval_messages(
approvals: Some(ApprovalMessages {
on_request: Some(on_request.to_string()),
on_request_auto_review: Some(on_request_auto_review.to_string()),
never: None,
unless_trusted: None,
granular: None,
..Default::default()
}),
..Default::default()
});
@@ -162,6 +165,125 @@ async fn catalog_approval_message_is_sent_in_initial_permissions() -> Result<()>
Ok(())
}
#[derive(Clone, Copy)]
enum ApprovalInstructionCase {
Present,
Absent,
Complete,
}
#[test_case(ApprovalInstructionCase::Present; "present_fields_use_model_wording")]
#[test_case(ApprovalInstructionCase::Absent; "absent_fields_preserve_bundled_instructions")]
#[test_case(ApprovalInstructionCase::Complete; "complete_approval_message_takes_precedence")]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn catalog_approval_instruction_parts_are_sent_in_initial_permissions(
case: ApprovalInstructionCase,
) -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
let req = mount_sse_once(
&server,
sse(vec![ev_response_created("resp-1"), ev_completed("resp-1")]),
)
.await;
let model_slug = "catalog-approvals-model";
let heading =
|value: &str| (!matches!(case, ApprovalInstructionCase::Absent)).then(|| value.to_string());
let sandbox = PermissionMessages {
read_only: Some(String::new()),
..Default::default()
};
let mut model = model_info_from_slug(model_slug);
model.use_responses_lite = true;
model.model_messages = Some(ModelMessages {
approvals: Some(ApprovalMessages {
on_request_auto_review: matches!(case, ApprovalInstructionCase::Complete)
.then(|| "Complete catalog approval instructions".to_string()),
request_permissions: heading("Catalog permission tool"),
auto_review_suffix: heading("Catalog automatic review"),
..Default::default()
}),
permissions: Some(sandbox.clone()),
..Default::default()
});
let fallback_slug = "catalog-approvals-fallback-model";
let mut fallback_model = model_with_permission_messages(fallback_slug, sandbox);
fallback_model.use_responses_lite = true;
let mut builder = test_codex()
.with_model(model_slug)
.with_config(move |config| {
config.permissions.approval_policy = Constrained::allow_any(AskForApproval::OnRequest);
config.approvals_reviewer = ApprovalsReviewer::AutoReview;
config
.permissions
.set_permission_profile(PermissionProfile::read_only())
.expect("read-only permissions should be allowed");
config.config_layer_stack = ConfigLayerStack::default();
for (feature, value) in [
(Feature::ExecPermissionApprovals, true),
(Feature::RequestPermissionsTool, true),
] {
config
.features
.set_enabled(feature, value)
.expect("test feature setting should be allowed");
}
config.model_catalog = Some(ModelsResponse {
models: vec![model, fallback_model],
});
});
let test = builder.build_with_auto_env(&server).await?;
submit_text_turn(&test, "hello").await?;
let request = req.single_request();
let bundled_rule = include_str!("../../../prompts/templates/permissions/approval_policy/on_request_rule_request_permission.md").replace("\r\n", "\n");
let bundled_approval = format!(
"{bundled_rule}\n\n# request_permissions Tool\n\n{REQUEST_TOOL_BODY}\n\n{AUTO_REVIEW_BODY}"
);
let expected = match case {
ApprovalInstructionCase::Complete => "Complete catalog approval instructions".to_string(),
ApprovalInstructionCase::Present => {
format!(
"{bundled_rule}\n\nCatalog permission tool\n\n{REQUEST_TOOL_BODY}\n\nCatalog automatic review\n\n{AUTO_REVIEW_BODY}"
)
}
ApprovalInstructionCase::Absent => bundled_approval.clone(),
};
let first = format!("<permissions instructions>\n{expected}\n</permissions instructions>");
let annotation = (
json!("developer"),
json!("permissions.instructions"),
json!({"provenance": {"type": "harness", "hook": false}}),
);
assert_eq!(permissions_texts(&request), vec![first.clone()]);
assert_eq!(permissions_annotations(&request), vec![annotation.clone()]);
if matches!(case, ApprovalInstructionCase::Present) {
let next = mount_sse_once(
&server,
sse(vec![ev_response_created("resp-2"), ev_completed("resp-2")]),
)
.await;
core_test_support::submit_thread_settings(
&test.codex,
ThreadSettingsOverrides {
model: Some(fallback_slug.to_string()),
..Default::default()
},
)
.await?;
submit_text_turn(&test, "Continue with the fallback model.").await?;
let next = next.single_request();
let fallback =
format!("<permissions instructions>\n{bundled_approval}\n</permissions instructions>");
assert!(next.input().starts_with(&request.input()));
assert_eq!(permissions_texts(&next), vec![first, fallback]);
assert_eq!(permissions_annotations(&next), vec![annotation; 2]);
}
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn model_change_appends_new_catalog_approval_message() -> Result<()> {
skip_if_no_network!(Ok(()));
@@ -220,11 +342,8 @@ async fn catalog_non_on_request_approval_messages_are_sent_in_initial_permission
(
AskForApproval::Never,
ApprovalMessages {
on_request: None,
on_request_auto_review: None,
never: Some("catalog never approval instructions".to_string()),
unless_trusted: None,
granular: None,
..Default::default()
},
"catalog never approval instructions",
"Approval policy is currently never",
@@ -232,11 +351,8 @@ async fn catalog_non_on_request_approval_messages_are_sent_in_initial_permission
(
AskForApproval::UnlessTrusted,
ApprovalMessages {
on_request: None,
on_request_auto_review: None,
never: None,
unless_trusted: Some("catalog unless-trusted approval instructions".to_string()),
granular: None,
..Default::default()
},
"catalog unless-trusted approval instructions",
"`approval_policy` is `unless-trusted`",
+3
View File
@@ -116,6 +116,9 @@ mod incremental_tools_resume;
#[path = "scenarios_model_instructions.rs"]
mod model_instructions;
#[path = "scenarios_catalog_approval_instructions_tests.rs"]
mod catalog_approval_instructions;
#[path = "scenarios_context_management_tools.rs"]
mod context_management_tools;
@@ -0,0 +1,144 @@
//! Catalog approval headings follow model changes without rewriting or repeating context.
#![cfg(test)]
use super::*;
use codex_core::config::Constrained;
use codex_models_manager::model_info::model_info_from_slug;
use codex_protocol::config_types::ApprovalsReviewer;
use codex_protocol::models::PermissionProfile;
use codex_protocol::openai_models::ApprovalMessages;
use codex_protocol::openai_models::ModelMessages;
use codex_protocol::openai_models::ModelsResponse;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::ThreadSettingsOverrides;
use pretty_assertions::assert_eq;
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn catalog_approval_headings_follow_selected_model() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
let mock = mount_sse_sequence(
&server,
[
(
"response-1",
"I can inspect the workspace with the current permissions.",
),
(
"response-2",
"The selected model's approval instructions are now active.",
),
("response-3", "The permissions are unchanged."),
]
.into_iter()
.map(|(id, text)| sse(vec![ev_assistant_message(id, text), ev_completed(id)]))
.collect(),
)
.await;
let first_slug = "catalog-approval-headings-a";
let second_slug = "catalog-approval-headings-b";
let models = [(first_slug, "Model A"), (second_slug, "Model B")]
.into_iter()
.map(|(slug, label)| {
let mut model = model_info_from_slug(slug);
model.use_responses_lite = true;
model.model_messages = Some(ModelMessages {
approvals: Some(ApprovalMessages {
request_permissions: Some(format!("# {label} permission requests")),
auto_review_suffix: Some(format!("# {label} automatic review")),
..Default::default()
}),
..Default::default()
});
model
})
.collect();
let test = test_codex()
.with_model(first_slug)
.with_config(move |config| {
configure_scenario_catalog(config);
config.workspace_roots = vec![config.cwd.clone()];
config.base_instructions = Some("Help with the workspace.".to_string());
config.permissions.approval_policy = Constrained::allow_any(AskForApproval::OnRequest);
config.approvals_reviewer = ApprovalsReviewer::AutoReview;
config
.permissions
.set_permission_profile(PermissionProfile::read_only())
.expect("read-only permissions should be allowed");
for feature in [
Feature::ExecPermissionApprovals,
Feature::RequestPermissionsTool,
] {
config
.features
.enable(feature)
.expect("enable permission requests");
}
config.model_catalog = Some(ModelsResponse { models });
})
.build_with_auto_env(&server)
.await?;
test.submit_text_turn("Inspect the workspace permissions.")
.await?;
core_test_support::submit_thread_settings(
&test.codex,
ThreadSettingsOverrides {
model: Some(second_slug.to_string()),
..Default::default()
},
)
.await?;
test.submit_text_turn("Continue with the selected model.")
.await?;
test.submit_text_turn("Continue without changing permissions.")
.await?;
let requests = mock.requests();
assert_eq!(requests.len(), 3);
let headings = requests
.iter()
.map(|request| {
request
.message_input_texts("developer")
.into_iter()
.filter(|text| text.contains("<permissions instructions>"))
.map(|text| {
text.lines()
.filter(|line| line.starts_with("# Model "))
.map(str::to_string)
.collect::<Vec<_>>()
})
.collect::<Vec<_>>()
})
.collect::<Vec<_>>();
let first_headings = vec![
"# Model A permission requests",
"# Model A automatic review",
];
let second_headings = vec![
"# Model B permission requests",
"# Model B automatic review",
];
assert_eq!(
headings,
vec![
vec![first_headings.clone()],
vec![first_headings.clone(), second_headings.clone()],
vec![first_headings, second_headings],
]
);
for pair in requests.windows(2) {
let previous = pair[0].input();
let next = pair[1].input();
assert_eq!(&next[..previous.len()], previous);
}
insta::assert_snapshot!(context_snapshot::format_request_history_snapshot(
"Catalog permission-request and automatic-review headings follow the selected model. The switch appends new developer instructions after the prior reply and before the next user message; an unchanged follow-up preserves the full history without repeating instructions.",
&requests,
&ContextSnapshotOptions::default().include_request_settings(),
));
Ok(())
}
@@ -138,6 +138,7 @@ async fn protected_model_settings_use_the_proposed_permissions(
never: None,
unless_trusted: None,
granular: None,
..Default::default()
});
messages.permissions = Some(PermissionMessages {
danger_full_access: None,
@@ -0,0 +1,140 @@
---
source: core/tests/suite/scenarios_catalog_approval_instructions_tests.rs
assertion_line: 138
expression: "context_snapshot::format_request_history_snapshot(\"Catalog permission-request and automatic-review headings follow the selected model. The switch appends new developer instructions after the prior reply and before the next user message; an unchanged follow-up preserves the full history without repeating instructions.\",\n&requests, &ContextSnapshotOptions::default().include_request_settings(),)"
---
Scenario: Catalog permission-request and automatic-review headings follow the selected model. The switch appends new developer instructions after the prior reply and before the next user message; an unchanged follow-up preserves the full history without repeating instructions.
## Window 1
Settings:
include: ["reasoning.encrypted_content"]
model: "catalog-approval-headings-a"
parallel_tool_calls: false
prompt_cache_key: "<PROMPT_CACHE_KEY 1>"
reasoning: {"context":"all_turns","summary":"auto"}
store: false
stream: true
tool_choice: "auto"
-- request 1 (turn) --
00:additional_tools/developer (2; hash=23179252be1abe5f):
- namespace/functions; hash=9962be1e4c2d492b
- function/exec_command
- function/write_stdin
- function/request_user_input
- function/request_permissions
- function/view_image
- namespace/multi_agent_v1: Tools for spawning and managing sub-agents.; hash=c826e5b07fd2820f
- function/close_agent
- function/resume_agent
- function/send_input
- function/spawn_agent
- function/wait_agent
01:message/developer:
Help with the workspace.
02:message/developer:
<permissions instructions>
Filesystem sandboxing defines which files can be read or written. `sandbox_mode` is `read-only`: The san... permits reading files. Network access is restricted. [hash=be7da4425f89053c]
# Permission Requests
Commands may require user approval before execution. Prefer requesting sandboxed additional permissions instead of asking to run fully outside the sandbox.
## Preferred request mode
<OMITTED 29 LINES; ~379 TOKENS; hash=d11e3f29e8628ec8>
# Model A permission requests
The built-in `request_permissions` tool is available in this session. Invoke it when you need to request... only the specific permissions required for the task. [hash=a29255aad0722385]
# Model A automatic review
`approvals_reviewer` is `auto_review`: Sandbox escalations with require_escalated will be reviewed for c...he risk and send a final message to ask for approval. [hash=c592d119d24cd2d6]
</permissions instructions>
03:message/user:
<environment_context>
<cwd><CWD></cwd>
<shell><HOST_SHELL></shell>
<current_date><CURRENT_DATE></current_date>
<timezone><HOST_TIMEZONE></timezone>
<filesystem><workspace_roots><root><CWD></root></workspace_roots><permission_profile type="managed"><f...ntry></file_system></permission_profile></filesystem> [hash=e8c1de60307d0a3d]
</environment_context>
04:message/user:
Inspect the workspace permissions.
## Window 2 (after request 1)
Settings: relative to window 1
model: "catalog-approval-headings-b"
-- request 2 (turn) --
00:additional_tools/developer (2; hash=23179252be1abe5f):
- namespace/functions; hash=9962be1e4c2d492b
- function/exec_command
- function/write_stdin
- function/request_user_input
- function/request_permissions
- function/view_image
- namespace/multi_agent_v1: Tools for spawning and managing sub-agents.; hash=c826e5b07fd2820f
- function/close_agent
- function/resume_agent
- function/send_input
- function/spawn_agent
- function/wait_agent
01:message/developer:
Help with the workspace.
02:message/developer:
<permissions instructions>
Filesystem sandboxing defines which files can be read or written. `sandbox_mode` is `read-only`: The san... permits reading files. Network access is restricted. [hash=be7da4425f89053c]
# Permission Requests
Commands may require user approval before execution. Prefer requesting sandboxed additional permissions instead of asking to run fully outside the sandbox.
## Preferred request mode
<OMITTED 29 LINES; ~379 TOKENS; hash=d11e3f29e8628ec8>
# Model A permission requests
The built-in `request_permissions` tool is available in this session. Invoke it when you need to request... only the specific permissions required for the task. [hash=a29255aad0722385]
# Model A automatic review
`approvals_reviewer` is `auto_review`: Sandbox escalations with require_escalated will be reviewed for c...he risk and send a final message to ask for approval. [hash=c592d119d24cd2d6]
</permissions instructions>
03:message/user:
<environment_context>
<cwd><CWD></cwd>
<shell><HOST_SHELL></shell>
<current_date><CURRENT_DATE></current_date>
<timezone><HOST_TIMEZONE></timezone>
<filesystem><workspace_roots><root><CWD></root></workspace_roots><permission_profile type="managed"><f...ntry></file_system></permission_profile></filesystem> [hash=e8c1de60307d0a3d]
</environment_context>
04:message/user:
Inspect the workspace permissions.
05:message/assistant:
I can inspect the workspace with the current permissions.
06:message/developer:
<model_instructions>
The current model is catalog-approval-headings-b.
</model_instructions>
07:message/developer:
<permissions instructions>
Filesystem sandboxing defines which files can be read or written. `sandbox_mode` is `read-only`: The san... permits reading files. Network access is restricted. [hash=be7da4425f89053c]
# Permission Requests
Commands may require user approval before execution. Prefer requesting sandboxed additional permissions instead of asking to run fully outside the sandbox.
## Preferred request mode
<OMITTED 29 LINES; ~379 TOKENS; hash=d11e3f29e8628ec8>
# Model B permission requests
The built-in `request_permissions` tool is available in this session. Invoke it when you need to request... only the specific permissions required for the task. [hash=a29255aad0722385]
# Model B automatic review
`approvals_reviewer` is `auto_review`: Sandbox escalations with require_escalated will be reviewed for c...he risk and send a final message to ask for approval. [hash=c592d119d24cd2d6]
</permissions instructions>
08:message/user:
Continue with the selected model.
-- request 3 (turn) --
09:message/assistant:
The selected model's approval instructions are now active.
10:message/user:
Continue without changing permissions.
@@ -1104,6 +1104,7 @@ async fn active_model_switch_updates_core_context_from_captured_settings(
never: None,
unless_trusted: None,
granular: None,
..Default::default()
});
messages.token_budget =
(initial_model || destination_has_guidance).then(|| ModelTokenBudgetConfig {