mirror of
https://github.com/openai/codex.git
synced 2026-11-13 06:45:07 +00:00
Support model catalog permission messages (#33147)
## What changed
- Add per-sandbox-mode permission messages to `ModelMessages` and preserve them when applying model configuration overrides.
- Use the selected catalog message in permission instructions, substituting `{{ network_access }}` with the active network policy. Fall back to the existing sandbox text when no override is provided, and allow an empty override to omit only the sandbox section.
- Apply catalog permission messages when a session starts and when its model changes.
## Testing
- Cover catalog deserialization, mode selection, network substitution, fallback and empty-message behavior, remote model catalogs, and model changes.
GitOrigin-RevId: cd5ed3aee3155dca0e7b2358c0f09bae73236856
This commit is contained in:
1 parent
f90e7deea6
commit
eb80df0317
12 files changed
+540
-11
No files matched your search
@@ -46,6 +46,10 @@ fn build_permissions_update_item(
|
|||||||
.model_messages
|
.model_messages
|
||||||
.as_ref()
|
.as_ref()
|
||||||
.and_then(|messages| messages.approvals.as_ref()),
|
.and_then(|messages| messages.approvals.as_ref()),
|
||||||
|
next.model_info
|
||||||
|
.model_messages
|
||||||
|
.as_ref()
|
||||||
|
.and_then(|messages| messages.permissions.as_ref()),
|
||||||
),
|
),
|
||||||
exec_policy,
|
exec_policy,
|
||||||
#[allow(deprecated)]
|
#[allow(deprecated)]
|
||||||
|
|||||||
@@ -1414,6 +1414,7 @@ mod tests {
|
|||||||
auto_review: Some(AutoReviewMessages {
|
auto_review: Some(AutoReviewMessages {
|
||||||
policy: Some("Use the catalog Guardian policy.".to_string()),
|
policy: Some("Use the catalog Guardian policy.".to_string()),
|
||||||
}),
|
}),
|
||||||
|
permissions: None,
|
||||||
};
|
};
|
||||||
|
|
||||||
let guardian_config = build_guardian_review_session_config(
|
let guardian_config = build_guardian_review_session_config(
|
||||||
@@ -1441,6 +1442,7 @@ mod tests {
|
|||||||
auto_review: Some(AutoReviewMessages {
|
auto_review: Some(AutoReviewMessages {
|
||||||
policy: Some(String::new()),
|
policy: Some(String::new()),
|
||||||
}),
|
}),
|
||||||
|
permissions: None,
|
||||||
};
|
};
|
||||||
|
|
||||||
let guardian_config = build_guardian_review_session_config(
|
let guardian_config = build_guardian_review_session_config(
|
||||||
|
|||||||
@@ -3233,6 +3233,11 @@ impl Session {
|
|||||||
.model_messages
|
.model_messages
|
||||||
.as_ref()
|
.as_ref()
|
||||||
.and_then(|messages| messages.approvals.as_ref()),
|
.and_then(|messages| messages.approvals.as_ref()),
|
||||||
|
turn_context
|
||||||
|
.model_info
|
||||||
|
.model_messages
|
||||||
|
.as_ref()
|
||||||
|
.and_then(|messages| messages.permissions.as_ref()),
|
||||||
),
|
),
|
||||||
self.services.exec_policy.current().as_ref(),
|
self.services.exec_policy.current().as_ref(),
|
||||||
#[allow(deprecated)]
|
#[allow(deprecated)]
|
||||||
|
|||||||
@@ -0,0 +1,115 @@
|
|||||||
|
use anyhow::Result;
|
||||||
|
use codex_core::config::Constrained;
|
||||||
|
use codex_login::CodexAuth;
|
||||||
|
use codex_models_manager::manager::RefreshStrategy;
|
||||||
|
use codex_models_manager::model_info::model_info_from_slug;
|
||||||
|
use codex_protocol::models::PermissionProfile;
|
||||||
|
use codex_protocol::openai_models::ModelMessages;
|
||||||
|
use codex_protocol::openai_models::ModelsResponse;
|
||||||
|
use codex_protocol::openai_models::PermissionMessages;
|
||||||
|
use codex_protocol::protocol::AskForApproval;
|
||||||
|
use codex_protocol::protocol::EventMsg;
|
||||||
|
use codex_protocol::protocol::Op;
|
||||||
|
use codex_protocol::protocol::ThreadSettingsOverrides;
|
||||||
|
use codex_protocol::user_input::UserInput;
|
||||||
|
use core_test_support::responses::ev_completed;
|
||||||
|
use core_test_support::responses::ev_response_created;
|
||||||
|
use core_test_support::responses::mount_models_once;
|
||||||
|
use core_test_support::responses::mount_sse_once;
|
||||||
|
use core_test_support::responses::sse;
|
||||||
|
use core_test_support::skip_if_no_network;
|
||||||
|
use core_test_support::test_codex::test_codex;
|
||||||
|
use core_test_support::wait_for_event;
|
||||||
|
use pretty_assertions::assert_eq;
|
||||||
|
use wiremock::MockServer;
|
||||||
|
|
||||||
|
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||||
|
async fn catalog_permission_message_loaded_from_remote_models_is_sent() -> Result<()> {
|
||||||
|
skip_if_no_network!(Ok(()));
|
||||||
|
|
||||||
|
let server = MockServer::start().await;
|
||||||
|
let model_slug = "remote-catalog-permissions-model";
|
||||||
|
let mut model = model_info_from_slug(model_slug);
|
||||||
|
model.model_messages = Some(ModelMessages {
|
||||||
|
instructions_template: None,
|
||||||
|
instructions_variables: None,
|
||||||
|
approvals: None,
|
||||||
|
auto_review: None,
|
||||||
|
permissions: Some(PermissionMessages {
|
||||||
|
danger_full_access: None,
|
||||||
|
workspace_write: None,
|
||||||
|
read_only: Some("remote catalog permissions: {{ network_access }}".to_string()),
|
||||||
|
}),
|
||||||
|
});
|
||||||
|
let models_mock = mount_models_once(
|
||||||
|
&server,
|
||||||
|
ModelsResponse {
|
||||||
|
models: vec![model],
|
||||||
|
},
|
||||||
|
)
|
||||||
|
.await;
|
||||||
|
let response_mock = mount_sse_once(
|
||||||
|
&server,
|
||||||
|
sse(vec![ev_response_created("resp-1"), ev_completed("resp-1")]),
|
||||||
|
)
|
||||||
|
.await;
|
||||||
|
let mut builder = test_codex()
|
||||||
|
.with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing())
|
||||||
|
.with_config(|config| {
|
||||||
|
config.permissions.approval_policy = Constrained::allow_any(AskForApproval::Never);
|
||||||
|
config
|
||||||
|
.permissions
|
||||||
|
.set_permission_profile(PermissionProfile::read_only())
|
||||||
|
.expect("read-only permission profile should be allowed");
|
||||||
|
});
|
||||||
|
let test = builder.build_with_auto_env(&server).await?;
|
||||||
|
test.thread_manager
|
||||||
|
.get_models_manager()
|
||||||
|
.list_models(
|
||||||
|
RefreshStrategy::OnlineIfUncached,
|
||||||
|
codex_core::test_support::default_http_client_factory(),
|
||||||
|
)
|
||||||
|
.await;
|
||||||
|
|
||||||
|
core_test_support::submit_thread_settings(
|
||||||
|
&test.codex,
|
||||||
|
ThreadSettingsOverrides {
|
||||||
|
model: Some(model_slug.to_string()),
|
||||||
|
..Default::default()
|
||||||
|
},
|
||||||
|
)
|
||||||
|
.await?;
|
||||||
|
test.codex
|
||||||
|
.submit(Op::UserInput {
|
||||||
|
items: vec![UserInput::Text {
|
||||||
|
text: "hello".to_string(),
|
||||||
|
text_elements: Vec::new(),
|
||||||
|
}],
|
||||||
|
final_output_json_schema: None,
|
||||||
|
responsesapi_client_metadata: None,
|
||||||
|
additional_context: Default::default(),
|
||||||
|
thread_settings: Default::default(),
|
||||||
|
})
|
||||||
|
.await?;
|
||||||
|
wait_for_event(&test.codex, |event| {
|
||||||
|
matches!(event, EventMsg::TurnComplete(_))
|
||||||
|
})
|
||||||
|
.await;
|
||||||
|
|
||||||
|
assert_eq!(models_mock.single_request_path(), "/v1/models");
|
||||||
|
let permissions = response_mock
|
||||||
|
.single_request()
|
||||||
|
.message_input_texts("developer")
|
||||||
|
.into_iter()
|
||||||
|
.filter(|text| text.contains("<permissions instructions>"))
|
||||||
|
.map(|text| text.replace("\r\n", "\n"))
|
||||||
|
.collect::<Vec<_>>();
|
||||||
|
assert_eq!(
|
||||||
|
permissions,
|
||||||
|
vec![
|
||||||
|
"<permissions instructions>\nremote catalog permissions: restricted\nApproval policy is currently never. Do not provide the `sandbox_permissions` for any reason, commands will be rejected.\n</permissions instructions>"
|
||||||
|
.to_string()
|
||||||
|
]
|
||||||
|
);
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
@@ -38,6 +38,7 @@ mod apply_patch_cli;
|
|||||||
#[cfg(not(target_os = "windows"))]
|
#[cfg(not(target_os = "windows"))]
|
||||||
mod approvals;
|
mod approvals;
|
||||||
mod auto_review;
|
mod auto_review;
|
||||||
|
mod catalog_permission_messages;
|
||||||
mod cli_stream;
|
mod cli_stream;
|
||||||
mod client;
|
mod client;
|
||||||
mod client_websockets;
|
mod client_websockets;
|
||||||
|
|||||||
@@ -11,6 +11,7 @@ use codex_protocol::models::PermissionProfile;
|
|||||||
use codex_protocol::openai_models::ApprovalMessages;
|
use codex_protocol::openai_models::ApprovalMessages;
|
||||||
use codex_protocol::openai_models::ModelMessages;
|
use codex_protocol::openai_models::ModelMessages;
|
||||||
use codex_protocol::openai_models::ModelsResponse;
|
use codex_protocol::openai_models::ModelsResponse;
|
||||||
|
use codex_protocol::openai_models::PermissionMessages;
|
||||||
use codex_protocol::permissions::NetworkSandboxPolicy;
|
use codex_protocol::permissions::NetworkSandboxPolicy;
|
||||||
use codex_protocol::protocol::AskForApproval;
|
use codex_protocol::protocol::AskForApproval;
|
||||||
use codex_protocol::protocol::EventMsg;
|
use codex_protocol::protocol::EventMsg;
|
||||||
@@ -52,6 +53,22 @@ fn model_with_approval_messages(
|
|||||||
on_request_auto_review: Some(on_request_auto_review.to_string()),
|
on_request_auto_review: Some(on_request_auto_review.to_string()),
|
||||||
}),
|
}),
|
||||||
auto_review: None,
|
auto_review: None,
|
||||||
|
permissions: None,
|
||||||
|
});
|
||||||
|
model
|
||||||
|
}
|
||||||
|
|
||||||
|
fn model_with_permission_messages(
|
||||||
|
slug: &str,
|
||||||
|
permissions: PermissionMessages,
|
||||||
|
) -> codex_protocol::openai_models::ModelInfo {
|
||||||
|
let mut model = model_info_from_slug(slug);
|
||||||
|
model.model_messages = Some(ModelMessages {
|
||||||
|
instructions_template: None,
|
||||||
|
instructions_variables: None,
|
||||||
|
approvals: None,
|
||||||
|
auto_review: None,
|
||||||
|
permissions: Some(permissions),
|
||||||
});
|
});
|
||||||
model
|
model
|
||||||
}
|
}
|
||||||
@@ -162,6 +179,128 @@ async fn model_change_appends_new_catalog_approval_message() -> Result<()> {
|
|||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||||
|
async fn catalog_permission_message_is_sent_initially_and_after_model_change() -> Result<()> {
|
||||||
|
skip_if_no_network!(Ok(()));
|
||||||
|
|
||||||
|
let server = start_mock_server().await;
|
||||||
|
let _req1 = mount_sse_once(
|
||||||
|
&server,
|
||||||
|
sse(vec![ev_response_created("resp-1"), ev_completed("resp-1")]),
|
||||||
|
)
|
||||||
|
.await;
|
||||||
|
let req2 = mount_sse_once(
|
||||||
|
&server,
|
||||||
|
sse(vec![ev_response_created("resp-2"), ev_completed("resp-2")]),
|
||||||
|
)
|
||||||
|
.await;
|
||||||
|
let first_slug = "catalog-permissions-model-a";
|
||||||
|
let second_slug = "catalog-permissions-model-b";
|
||||||
|
let first = model_with_permission_messages(
|
||||||
|
first_slug,
|
||||||
|
PermissionMessages {
|
||||||
|
danger_full_access: None,
|
||||||
|
workspace_write: None,
|
||||||
|
read_only: Some("model A permissions: {{ network_access }}".to_string()),
|
||||||
|
},
|
||||||
|
);
|
||||||
|
let second = model_with_permission_messages(
|
||||||
|
second_slug,
|
||||||
|
PermissionMessages {
|
||||||
|
danger_full_access: None,
|
||||||
|
workspace_write: None,
|
||||||
|
read_only: Some("model B permissions: {{ network_access }}".to_string()),
|
||||||
|
},
|
||||||
|
);
|
||||||
|
let mut builder = test_codex()
|
||||||
|
.with_model(first_slug)
|
||||||
|
.with_config(move |config| {
|
||||||
|
config.permissions.approval_policy = Constrained::allow_any(AskForApproval::Never);
|
||||||
|
config
|
||||||
|
.permissions
|
||||||
|
.set_permission_profile(PermissionProfile::read_only())
|
||||||
|
.expect("read-only permission profile should be allowed");
|
||||||
|
config.model_catalog = Some(ModelsResponse {
|
||||||
|
models: vec![first, second],
|
||||||
|
});
|
||||||
|
});
|
||||||
|
let test = builder.build(&server).await?;
|
||||||
|
submit_text_turn(&test, "first").await?;
|
||||||
|
|
||||||
|
core_test_support::submit_thread_settings(
|
||||||
|
&test.codex,
|
||||||
|
codex_protocol::protocol::ThreadSettingsOverrides {
|
||||||
|
model: Some(second_slug.to_string()),
|
||||||
|
..Default::default()
|
||||||
|
},
|
||||||
|
)
|
||||||
|
.await?;
|
||||||
|
submit_text_turn(&test, "second").await?;
|
||||||
|
|
||||||
|
let permissions = permissions_texts(&req2.single_request());
|
||||||
|
assert_eq!(permissions.len(), 2);
|
||||||
|
assert!(permissions[0].contains("model A permissions: restricted"));
|
||||||
|
assert!(
|
||||||
|
permissions
|
||||||
|
.last()
|
||||||
|
.is_some_and(|text| text.contains("model B permissions: restricted"))
|
||||||
|
);
|
||||||
|
assert!(
|
||||||
|
permissions
|
||||||
|
.last()
|
||||||
|
.is_some_and(|text| text.contains("Approval policy is currently never"))
|
||||||
|
);
|
||||||
|
assert!(
|
||||||
|
!permissions
|
||||||
|
.iter()
|
||||||
|
.any(|text| text.contains("`sandbox_mode`"))
|
||||||
|
);
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
|
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||||
|
async fn empty_catalog_permission_message_preserves_approval_instructions() -> 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 = "empty-catalog-permissions-model";
|
||||||
|
let model = model_with_permission_messages(
|
||||||
|
model_slug,
|
||||||
|
PermissionMessages {
|
||||||
|
danger_full_access: None,
|
||||||
|
workspace_write: None,
|
||||||
|
read_only: Some(String::new()),
|
||||||
|
},
|
||||||
|
);
|
||||||
|
let mut builder = test_codex()
|
||||||
|
.with_model(model_slug)
|
||||||
|
.with_config(move |config| {
|
||||||
|
config.permissions.approval_policy = Constrained::allow_any(AskForApproval::Never);
|
||||||
|
config
|
||||||
|
.permissions
|
||||||
|
.set_permission_profile(PermissionProfile::read_only())
|
||||||
|
.expect("read-only permission profile should be allowed");
|
||||||
|
config.model_catalog = Some(ModelsResponse {
|
||||||
|
models: vec![model],
|
||||||
|
});
|
||||||
|
});
|
||||||
|
let test = builder.build(&server).await?;
|
||||||
|
|
||||||
|
submit_text_turn(&test, "hello").await?;
|
||||||
|
|
||||||
|
let permissions = permissions_texts(&req.single_request());
|
||||||
|
assert_eq!(permissions.len(), 1);
|
||||||
|
assert!(permissions[0].contains("Approval policy is currently never"));
|
||||||
|
assert!(!permissions[0].contains("Filesystem sandboxing defines"));
|
||||||
|
assert!(!permissions[0].contains("`sandbox_mode`"));
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||||
async fn permissions_message_sent_once_on_start() -> Result<()> {
|
async fn permissions_message_sent_once_on_start() -> Result<()> {
|
||||||
skip_if_no_network!(Ok(()));
|
skip_if_no_network!(Ok(()));
|
||||||
@@ -692,7 +831,11 @@ async fn permissions_message_includes_writable_roots() -> Result<()> {
|
|||||||
let expected = PermissionsInstructions::from_permission_profile(
|
let expected = PermissionsInstructions::from_permission_profile(
|
||||||
&permission_profile,
|
&permission_profile,
|
||||||
AskForApproval::OnRequest,
|
AskForApproval::OnRequest,
|
||||||
ApprovalPromptContext::new(test.config.approvals_reviewer, /*messages*/ None),
|
ApprovalPromptContext::new(
|
||||||
|
test.config.approvals_reviewer,
|
||||||
|
/*messages*/ None,
|
||||||
|
/*permission_messages*/ None,
|
||||||
|
),
|
||||||
&exec_policy,
|
&exec_policy,
|
||||||
test.config.cwd.as_path(),
|
test.config.cwd.as_path(),
|
||||||
/*exec_permission_approvals_enabled*/ false,
|
/*exec_permission_approvals_enabled*/ false,
|
||||||
|
|||||||
@@ -641,6 +641,7 @@ async fn remote_model_friendly_personality_instructions_with_feature() -> anyhow
|
|||||||
}),
|
}),
|
||||||
approvals: None,
|
approvals: None,
|
||||||
auto_review: None,
|
auto_review: None,
|
||||||
|
permissions: None,
|
||||||
}),
|
}),
|
||||||
include_skills_usage_instructions: false,
|
include_skills_usage_instructions: false,
|
||||||
supports_reasoning_summary_parameter: true,
|
supports_reasoning_summary_parameter: true,
|
||||||
@@ -759,6 +760,7 @@ async fn user_turn_personality_remote_model_template_includes_update_message() -
|
|||||||
}),
|
}),
|
||||||
approvals: None,
|
approvals: None,
|
||||||
auto_review: None,
|
auto_review: None,
|
||||||
|
permissions: None,
|
||||||
}),
|
}),
|
||||||
include_skills_usage_instructions: false,
|
include_skills_usage_instructions: false,
|
||||||
supports_reasoning_summary_parameter: true,
|
supports_reasoning_summary_parameter: true,
|
||||||
|
|||||||
@@ -112,7 +112,10 @@ fn clear_instruction_messages(model: &mut ModelInfo) {
|
|||||||
if let Some(model_messages) = &mut model.model_messages {
|
if let Some(model_messages) = &mut model.model_messages {
|
||||||
model_messages.instructions_template = None;
|
model_messages.instructions_template = None;
|
||||||
model_messages.instructions_variables = None;
|
model_messages.instructions_variables = None;
|
||||||
if model_messages.approvals.is_none() && model_messages.auto_review.is_none() {
|
if model_messages.approvals.is_none()
|
||||||
|
&& model_messages.auto_review.is_none()
|
||||||
|
&& model_messages.permissions.is_none()
|
||||||
|
{
|
||||||
model.model_messages = None;
|
model.model_messages = None;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -177,6 +180,7 @@ fn local_personality_messages_for_slug(slug: &str) -> Option<ModelMessages> {
|
|||||||
}),
|
}),
|
||||||
approvals: None,
|
approvals: None,
|
||||||
auto_review: None,
|
auto_review: None,
|
||||||
|
permissions: None,
|
||||||
}),
|
}),
|
||||||
_ => None,
|
_ => None,
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -3,6 +3,7 @@ use crate::ModelsManagerConfig;
|
|||||||
use codex_protocol::config_types::Personality;
|
use codex_protocol::config_types::Personality;
|
||||||
use codex_protocol::openai_models::ApprovalMessages;
|
use codex_protocol::openai_models::ApprovalMessages;
|
||||||
use codex_protocol::openai_models::AutoReviewMessages;
|
use codex_protocol::openai_models::AutoReviewMessages;
|
||||||
|
use codex_protocol::openai_models::PermissionMessages;
|
||||||
use pretty_assertions::assert_eq;
|
use pretty_assertions::assert_eq;
|
||||||
|
|
||||||
fn config_with_personality(personality: Option<Personality>) -> ModelsManagerConfig {
|
fn config_with_personality(personality: Option<Personality>) -> ModelsManagerConfig {
|
||||||
@@ -29,6 +30,7 @@ fn base_instruction_override_preserves_catalog_approval_messages() {
|
|||||||
}),
|
}),
|
||||||
approvals: Some(approvals.clone()),
|
approvals: Some(approvals.clone()),
|
||||||
auto_review: None,
|
auto_review: None,
|
||||||
|
permissions: None,
|
||||||
});
|
});
|
||||||
let config = ModelsManagerConfig {
|
let config = ModelsManagerConfig {
|
||||||
base_instructions: Some("override".to_string()),
|
base_instructions: Some("override".to_string()),
|
||||||
@@ -44,6 +46,7 @@ fn base_instruction_override_preserves_catalog_approval_messages() {
|
|||||||
instructions_variables: None,
|
instructions_variables: None,
|
||||||
approvals: Some(approvals),
|
approvals: Some(approvals),
|
||||||
auto_review: None,
|
auto_review: None,
|
||||||
|
permissions: None,
|
||||||
})
|
})
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
@@ -60,6 +63,7 @@ fn disabled_personality_preserves_catalog_approval_messages() {
|
|||||||
instructions_variables: None,
|
instructions_variables: None,
|
||||||
approvals: Some(approvals.clone()),
|
approvals: Some(approvals.clone()),
|
||||||
auto_review: None,
|
auto_review: None,
|
||||||
|
permissions: None,
|
||||||
});
|
});
|
||||||
let config = ModelsManagerConfig {
|
let config = ModelsManagerConfig {
|
||||||
personality_enabled: false,
|
personality_enabled: false,
|
||||||
@@ -75,6 +79,7 @@ fn disabled_personality_preserves_catalog_approval_messages() {
|
|||||||
instructions_variables: None,
|
instructions_variables: None,
|
||||||
approvals: Some(approvals),
|
approvals: Some(approvals),
|
||||||
auto_review: None,
|
auto_review: None,
|
||||||
|
permissions: None,
|
||||||
})
|
})
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
@@ -90,6 +95,7 @@ fn base_instruction_override_preserves_catalog_auto_review_messages() {
|
|||||||
instructions_variables: None,
|
instructions_variables: None,
|
||||||
approvals: None,
|
approvals: None,
|
||||||
auto_review: Some(auto_review.clone()),
|
auto_review: Some(auto_review.clone()),
|
||||||
|
permissions: None,
|
||||||
});
|
});
|
||||||
let config = ModelsManagerConfig {
|
let config = ModelsManagerConfig {
|
||||||
base_instructions: Some("override".to_string()),
|
base_instructions: Some("override".to_string()),
|
||||||
@@ -105,6 +111,41 @@ fn base_instruction_override_preserves_catalog_auto_review_messages() {
|
|||||||
instructions_variables: None,
|
instructions_variables: None,
|
||||||
approvals: None,
|
approvals: None,
|
||||||
auto_review: Some(auto_review),
|
auto_review: Some(auto_review),
|
||||||
|
permissions: None,
|
||||||
|
})
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn base_instruction_override_preserves_catalog_permission_messages() {
|
||||||
|
let mut model = model_info_from_slug("unknown-model");
|
||||||
|
let permissions = PermissionMessages {
|
||||||
|
danger_full_access: Some("danger".to_string()),
|
||||||
|
workspace_write: Some(String::new()),
|
||||||
|
read_only: None,
|
||||||
|
};
|
||||||
|
model.model_messages = Some(ModelMessages {
|
||||||
|
instructions_template: Some("template".to_string()),
|
||||||
|
instructions_variables: None,
|
||||||
|
approvals: None,
|
||||||
|
auto_review: None,
|
||||||
|
permissions: Some(permissions.clone()),
|
||||||
|
});
|
||||||
|
let config = ModelsManagerConfig {
|
||||||
|
base_instructions: Some("override".to_string()),
|
||||||
|
..Default::default()
|
||||||
|
};
|
||||||
|
|
||||||
|
let updated = with_config_overrides(model, &config);
|
||||||
|
|
||||||
|
assert_eq!(
|
||||||
|
updated.model_messages,
|
||||||
|
Some(ModelMessages {
|
||||||
|
instructions_template: None,
|
||||||
|
instructions_variables: None,
|
||||||
|
approvals: None,
|
||||||
|
auto_review: None,
|
||||||
|
permissions: Some(permissions),
|
||||||
})
|
})
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
@@ -140,6 +181,7 @@ fn personality_none_strips_catalog_instruction_sources_through_the_next_h1() {
|
|||||||
instructions_variables: None,
|
instructions_variables: None,
|
||||||
approvals: None,
|
approvals: None,
|
||||||
auto_review: None,
|
auto_review: None,
|
||||||
|
permissions: None,
|
||||||
});
|
});
|
||||||
|
|
||||||
let updated = with_config_overrides(model, &config);
|
let updated = with_config_overrides(model, &config);
|
||||||
|
|||||||
@@ -5,6 +5,7 @@ use codex_protocol::config_types::SandboxMode;
|
|||||||
use codex_protocol::models::PermissionProfile;
|
use codex_protocol::models::PermissionProfile;
|
||||||
use codex_protocol::models::format_allow_prefixes;
|
use codex_protocol::models::format_allow_prefixes;
|
||||||
use codex_protocol::openai_models::ApprovalMessages;
|
use codex_protocol::openai_models::ApprovalMessages;
|
||||||
|
use codex_protocol::openai_models::PermissionMessages;
|
||||||
use codex_protocol::permissions::FileSystemSandboxPolicy;
|
use codex_protocol::permissions::FileSystemSandboxPolicy;
|
||||||
use codex_protocol::permissions::NetworkSandboxPolicy;
|
use codex_protocol::permissions::NetworkSandboxPolicy;
|
||||||
use codex_protocol::protocol::AskForApproval;
|
use codex_protocol::protocol::AskForApproval;
|
||||||
@@ -24,6 +25,7 @@ const APPROVAL_POLICY_ON_REQUEST_RULE: &str =
|
|||||||
const APPROVAL_POLICY_ON_REQUEST_RULE_REQUEST_PERMISSION: &str =
|
const APPROVAL_POLICY_ON_REQUEST_RULE_REQUEST_PERMISSION: &str =
|
||||||
include_str!("../templates/permissions/approval_policy/on_request_rule_request_permission.md");
|
include_str!("../templates/permissions/approval_policy/on_request_rule_request_permission.md");
|
||||||
const AUTO_REVIEW_APPROVAL_SUFFIX: &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.";
|
const AUTO_REVIEW_APPROVAL_SUFFIX: &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.";
|
||||||
|
const NETWORK_ACCESS_PLACEHOLDER: &str = "{{ network_access }}";
|
||||||
|
|
||||||
const SANDBOX_MODE_DANGER_FULL_ACCESS: &str =
|
const SANDBOX_MODE_DANGER_FULL_ACCESS: &str =
|
||||||
include_str!("../templates/permissions/sandbox_mode/danger_full_access.md");
|
include_str!("../templates/permissions/sandbox_mode/danger_full_access.md");
|
||||||
@@ -49,6 +51,7 @@ struct PermissionsPromptConfig<'a> {
|
|||||||
approval_policy: AskForApproval,
|
approval_policy: AskForApproval,
|
||||||
approvals_reviewer: ApprovalsReviewer,
|
approvals_reviewer: ApprovalsReviewer,
|
||||||
approval_messages: Option<&'a ApprovalMessages>,
|
approval_messages: Option<&'a ApprovalMessages>,
|
||||||
|
permission_messages: Option<&'a PermissionMessages>,
|
||||||
exec_policy: &'a Policy,
|
exec_policy: &'a Policy,
|
||||||
exec_permission_approvals_enabled: bool,
|
exec_permission_approvals_enabled: bool,
|
||||||
request_permissions_tool_enabled: bool,
|
request_permissions_tool_enabled: bool,
|
||||||
@@ -64,11 +67,20 @@ pub struct PermissionsInstructions {
|
|||||||
pub struct ApprovalPromptContext<'a> {
|
pub struct ApprovalPromptContext<'a> {
|
||||||
reviewer: ApprovalsReviewer,
|
reviewer: ApprovalsReviewer,
|
||||||
messages: Option<&'a ApprovalMessages>,
|
messages: Option<&'a ApprovalMessages>,
|
||||||
|
permission_messages: Option<&'a PermissionMessages>,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl<'a> ApprovalPromptContext<'a> {
|
impl<'a> ApprovalPromptContext<'a> {
|
||||||
pub fn new(reviewer: ApprovalsReviewer, messages: Option<&'a ApprovalMessages>) -> Self {
|
pub fn new(
|
||||||
Self { reviewer, messages }
|
reviewer: ApprovalsReviewer,
|
||||||
|
messages: Option<&'a ApprovalMessages>,
|
||||||
|
permission_messages: Option<&'a PermissionMessages>,
|
||||||
|
) -> Self {
|
||||||
|
Self {
|
||||||
|
reviewer,
|
||||||
|
messages,
|
||||||
|
permission_messages,
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -94,6 +106,7 @@ impl PermissionsInstructions {
|
|||||||
approval_policy,
|
approval_policy,
|
||||||
approvals_reviewer: approval_context.reviewer,
|
approvals_reviewer: approval_context.reviewer,
|
||||||
approval_messages: approval_context.messages,
|
approval_messages: approval_context.messages,
|
||||||
|
permission_messages: approval_context.permission_messages,
|
||||||
exec_policy,
|
exec_policy,
|
||||||
exec_permission_approvals_enabled,
|
exec_permission_approvals_enabled,
|
||||||
request_permissions_tool_enabled,
|
request_permissions_tool_enabled,
|
||||||
@@ -131,7 +144,10 @@ impl PermissionsInstructions {
|
|||||||
denied_reads: Option<String>,
|
denied_reads: Option<String>,
|
||||||
) -> Self {
|
) -> Self {
|
||||||
let mut text = String::new();
|
let mut text = String::new();
|
||||||
append_section(&mut text, &sandbox_text(sandbox_mode, network_access));
|
let sandbox = sandbox_text(sandbox_mode, network_access, config.permission_messages);
|
||||||
|
if !sandbox.is_empty() {
|
||||||
|
append_section(&mut text, &sandbox);
|
||||||
|
}
|
||||||
append_section(
|
append_section(
|
||||||
&mut text,
|
&mut text,
|
||||||
&approval_text(
|
&approval_text(
|
||||||
@@ -272,7 +288,24 @@ fn approval_text(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
fn sandbox_text(mode: SandboxMode, network_access: NetworkAccess) -> String {
|
fn sandbox_text(
|
||||||
|
mode: SandboxMode,
|
||||||
|
network_access: NetworkAccess,
|
||||||
|
permission_messages: Option<&PermissionMessages>,
|
||||||
|
) -> String {
|
||||||
|
let selected = permission_messages.and_then(|messages| match mode {
|
||||||
|
SandboxMode::DangerFullAccess => messages.danger_full_access.as_deref(),
|
||||||
|
SandboxMode::WorkspaceWrite => messages.workspace_write.as_deref(),
|
||||||
|
SandboxMode::ReadOnly => messages.read_only.as_deref(),
|
||||||
|
});
|
||||||
|
if let Some(selected) = selected {
|
||||||
|
if selected.is_empty() {
|
||||||
|
return String::new();
|
||||||
|
}
|
||||||
|
let network_access = network_access.to_string();
|
||||||
|
return selected.replace(NETWORK_ACCESS_PLACEHOLDER, network_access.as_str());
|
||||||
|
}
|
||||||
|
|
||||||
let template = match mode {
|
let template = match mode {
|
||||||
SandboxMode::DangerFullAccess => &*SANDBOX_MODE_DANGER_FULL_ACCESS_TEMPLATE,
|
SandboxMode::DangerFullAccess => &*SANDBOX_MODE_DANGER_FULL_ACCESS_TEMPLATE,
|
||||||
SandboxMode::WorkspaceWrite => &*SANDBOX_MODE_WORKSPACE_WRITE_TEMPLATE,
|
SandboxMode::WorkspaceWrite => &*SANDBOX_MODE_WORKSPACE_WRITE_TEMPLATE,
|
||||||
|
|||||||
@@ -13,21 +13,148 @@ use std::path::PathBuf;
|
|||||||
#[test]
|
#[test]
|
||||||
fn renders_sandbox_mode_text() {
|
fn renders_sandbox_mode_text() {
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
sandbox_text(SandboxMode::WorkspaceWrite, NetworkAccess::Restricted),
|
sandbox_text(
|
||||||
|
SandboxMode::WorkspaceWrite,
|
||||||
|
NetworkAccess::Restricted,
|
||||||
|
/*permission_messages*/ None,
|
||||||
|
),
|
||||||
"Filesystem sandboxing defines which files can be read or written. `sandbox_mode` is `workspace-write`: The sandbox permits reading files, and editing files in `cwd` and `writable_roots`. Editing files in other directories requires approval. Network access is restricted."
|
"Filesystem sandboxing defines which files can be read or written. `sandbox_mode` is `workspace-write`: The sandbox permits reading files, and editing files in `cwd` and `writable_roots`. Editing files in other directories requires approval. Network access is restricted."
|
||||||
);
|
);
|
||||||
|
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
sandbox_text(SandboxMode::ReadOnly, NetworkAccess::Restricted),
|
sandbox_text(
|
||||||
|
SandboxMode::ReadOnly,
|
||||||
|
NetworkAccess::Restricted,
|
||||||
|
/*permission_messages*/ None,
|
||||||
|
),
|
||||||
"Filesystem sandboxing defines which files can be read or written. `sandbox_mode` is `read-only`: The sandbox only permits reading files. Network access is restricted."
|
"Filesystem sandboxing defines which files can be read or written. `sandbox_mode` is `read-only`: The sandbox only permits reading files. Network access is restricted."
|
||||||
);
|
);
|
||||||
|
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
sandbox_text(SandboxMode::DangerFullAccess, NetworkAccess::Enabled),
|
sandbox_text(
|
||||||
|
SandboxMode::DangerFullAccess,
|
||||||
|
NetworkAccess::Enabled,
|
||||||
|
/*permission_messages*/ None,
|
||||||
|
),
|
||||||
"Filesystem sandboxing defines which files can be read or written. `sandbox_mode` is `danger-full-access`: No filesystem sandboxing - all commands are permitted. Network access is enabled."
|
"Filesystem sandboxing defines which files can be read or written. `sandbox_mode` is `danger-full-access`: No filesystem sandboxing - all commands are permitted. Network access is enabled."
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn catalog_permission_messages_select_sandbox_mode_and_render_network_access() {
|
||||||
|
let messages = PermissionMessages {
|
||||||
|
danger_full_access: Some("catalog danger".to_string()),
|
||||||
|
workspace_write: Some("catalog workspace {{ network_access }}".to_string()),
|
||||||
|
read_only: Some("catalog read only {{ network_access }}".to_string()),
|
||||||
|
};
|
||||||
|
|
||||||
|
for (mode, expected) in [
|
||||||
|
(SandboxMode::DangerFullAccess, "catalog danger"),
|
||||||
|
(SandboxMode::WorkspaceWrite, "catalog workspace enabled"),
|
||||||
|
(SandboxMode::ReadOnly, "catalog read only enabled"),
|
||||||
|
] {
|
||||||
|
assert_eq!(
|
||||||
|
sandbox_text(mode, NetworkAccess::Enabled, Some(&messages)),
|
||||||
|
expected
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn missing_catalog_permission_message_uses_legacy_sandbox_text() {
|
||||||
|
let legacy = sandbox_text(
|
||||||
|
SandboxMode::WorkspaceWrite,
|
||||||
|
NetworkAccess::Restricted,
|
||||||
|
/*permission_messages*/ None,
|
||||||
|
);
|
||||||
|
let messages = PermissionMessages {
|
||||||
|
danger_full_access: None,
|
||||||
|
workspace_write: None,
|
||||||
|
read_only: Some("unused".to_string()),
|
||||||
|
};
|
||||||
|
|
||||||
|
assert_eq!(
|
||||||
|
sandbox_text(
|
||||||
|
SandboxMode::WorkspaceWrite,
|
||||||
|
NetworkAccess::Restricted,
|
||||||
|
Some(&messages),
|
||||||
|
),
|
||||||
|
legacy
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn invalid_catalog_permission_message_is_preserved_verbatim() {
|
||||||
|
for workspace_write in ["{{ unterminated", "{{ unsupported }}"] {
|
||||||
|
let messages = PermissionMessages {
|
||||||
|
danger_full_access: None,
|
||||||
|
workspace_write: Some(workspace_write.to_string()),
|
||||||
|
read_only: None,
|
||||||
|
};
|
||||||
|
assert_eq!(
|
||||||
|
sandbox_text(
|
||||||
|
SandboxMode::WorkspaceWrite,
|
||||||
|
NetworkAccess::Restricted,
|
||||||
|
Some(&messages),
|
||||||
|
),
|
||||||
|
workspace_write
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn catalog_permission_message_renders_network_access_and_preserves_other_placeholders() {
|
||||||
|
let source = "network={{ network_access }} compact={{network_access}} other={{ other }}";
|
||||||
|
let messages = PermissionMessages {
|
||||||
|
danger_full_access: None,
|
||||||
|
workspace_write: Some(source.to_string()),
|
||||||
|
read_only: None,
|
||||||
|
};
|
||||||
|
|
||||||
|
assert_eq!(
|
||||||
|
sandbox_text(
|
||||||
|
SandboxMode::WorkspaceWrite,
|
||||||
|
NetworkAccess::Restricted,
|
||||||
|
Some(&messages),
|
||||||
|
),
|
||||||
|
"network=restricted compact={{network_access}} other={{ other }}"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn empty_catalog_permission_message_preserves_non_sandbox_sections() {
|
||||||
|
let messages = PermissionMessages {
|
||||||
|
danger_full_access: None,
|
||||||
|
workspace_write: Some(String::new()),
|
||||||
|
read_only: None,
|
||||||
|
};
|
||||||
|
let writable_root =
|
||||||
|
AbsolutePathBuf::from_absolute_path(test_path_buf("/tmp/repo")).expect("absolute path");
|
||||||
|
let instructions = PermissionsInstructions::from_permissions_with_network(
|
||||||
|
SandboxMode::WorkspaceWrite,
|
||||||
|
NetworkAccess::Restricted,
|
||||||
|
PermissionsPromptConfig {
|
||||||
|
approval_policy: AskForApproval::Never,
|
||||||
|
approvals_reviewer: ApprovalsReviewer::User,
|
||||||
|
approval_messages: None,
|
||||||
|
permission_messages: Some(&messages),
|
||||||
|
exec_policy: &Policy::empty(),
|
||||||
|
exec_permission_approvals_enabled: false,
|
||||||
|
request_permissions_tool_enabled: false,
|
||||||
|
},
|
||||||
|
Some(vec![WritableRoot {
|
||||||
|
root: writable_root.clone(),
|
||||||
|
read_only_subpaths: Vec::new(),
|
||||||
|
protected_metadata_names: Vec::new(),
|
||||||
|
}]),
|
||||||
|
);
|
||||||
|
let text = instructions.body();
|
||||||
|
|
||||||
|
assert!(!text.contains("Filesystem sandboxing defines"));
|
||||||
|
assert!(text.contains("Approval policy is currently never"));
|
||||||
|
assert!(text.contains(writable_root.to_string_lossy().as_ref()));
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn builds_permissions_with_network_access_override() {
|
fn builds_permissions_with_network_access_override() {
|
||||||
let instructions = PermissionsInstructions::from_permissions_with_network(
|
let instructions = PermissionsInstructions::from_permissions_with_network(
|
||||||
@@ -37,6 +164,7 @@ fn builds_permissions_with_network_access_override() {
|
|||||||
approval_policy: AskForApproval::OnRequest,
|
approval_policy: AskForApproval::OnRequest,
|
||||||
approvals_reviewer: ApprovalsReviewer::User,
|
approvals_reviewer: ApprovalsReviewer::User,
|
||||||
approval_messages: None,
|
approval_messages: None,
|
||||||
|
permission_messages: None,
|
||||||
exec_policy: &Policy::empty(),
|
exec_policy: &Policy::empty(),
|
||||||
exec_permission_approvals_enabled: false,
|
exec_permission_approvals_enabled: false,
|
||||||
request_permissions_tool_enabled: false,
|
request_permissions_tool_enabled: false,
|
||||||
@@ -73,7 +201,11 @@ fn builds_permissions_from_profile() {
|
|||||||
let instructions = PermissionsInstructions::from_permission_profile(
|
let instructions = PermissionsInstructions::from_permission_profile(
|
||||||
&permission_profile,
|
&permission_profile,
|
||||||
AskForApproval::UnlessTrusted,
|
AskForApproval::UnlessTrusted,
|
||||||
ApprovalPromptContext::new(ApprovalsReviewer::User, /*messages*/ None),
|
ApprovalPromptContext::new(
|
||||||
|
ApprovalsReviewer::User,
|
||||||
|
/*messages*/ None,
|
||||||
|
/*permission_messages*/ None,
|
||||||
|
),
|
||||||
&Policy::empty(),
|
&Policy::empty(),
|
||||||
&cwd,
|
&cwd,
|
||||||
/*exec_permission_approvals_enabled*/ false,
|
/*exec_permission_approvals_enabled*/ false,
|
||||||
@@ -118,7 +250,11 @@ fn builds_permissions_from_profile_with_denied_reads() {
|
|||||||
let instructions = PermissionsInstructions::from_permission_profile(
|
let instructions = PermissionsInstructions::from_permission_profile(
|
||||||
&permission_profile,
|
&permission_profile,
|
||||||
AskForApproval::OnRequest,
|
AskForApproval::OnRequest,
|
||||||
ApprovalPromptContext::new(ApprovalsReviewer::AutoReview, /*messages*/ None),
|
ApprovalPromptContext::new(
|
||||||
|
ApprovalsReviewer::AutoReview,
|
||||||
|
/*messages*/ None,
|
||||||
|
/*permission_messages*/ None,
|
||||||
|
),
|
||||||
&Policy::empty(),
|
&Policy::empty(),
|
||||||
&cwd,
|
&cwd,
|
||||||
/*exec_permission_approvals_enabled*/ false,
|
/*exec_permission_approvals_enabled*/ false,
|
||||||
@@ -144,6 +280,7 @@ fn includes_request_rule_instructions_for_on_request() {
|
|||||||
approval_policy: AskForApproval::OnRequest,
|
approval_policy: AskForApproval::OnRequest,
|
||||||
approvals_reviewer: ApprovalsReviewer::User,
|
approvals_reviewer: ApprovalsReviewer::User,
|
||||||
approval_messages: None,
|
approval_messages: None,
|
||||||
|
permission_messages: None,
|
||||||
exec_policy: &exec_policy,
|
exec_policy: &exec_policy,
|
||||||
exec_permission_approvals_enabled: false,
|
exec_permission_approvals_enabled: false,
|
||||||
request_permissions_tool_enabled: false,
|
request_permissions_tool_enabled: false,
|
||||||
@@ -166,6 +303,7 @@ fn includes_request_permissions_tool_instructions_for_unless_trusted_when_enable
|
|||||||
approval_policy: AskForApproval::UnlessTrusted,
|
approval_policy: AskForApproval::UnlessTrusted,
|
||||||
approvals_reviewer: ApprovalsReviewer::User,
|
approvals_reviewer: ApprovalsReviewer::User,
|
||||||
approval_messages: None,
|
approval_messages: None,
|
||||||
|
permission_messages: None,
|
||||||
exec_policy: &Policy::empty(),
|
exec_policy: &Policy::empty(),
|
||||||
exec_permission_approvals_enabled: false,
|
exec_permission_approvals_enabled: false,
|
||||||
request_permissions_tool_enabled: true,
|
request_permissions_tool_enabled: true,
|
||||||
@@ -187,6 +325,7 @@ fn includes_request_permission_rule_instructions_for_on_request_when_enabled() {
|
|||||||
approval_policy: AskForApproval::OnRequest,
|
approval_policy: AskForApproval::OnRequest,
|
||||||
approvals_reviewer: ApprovalsReviewer::User,
|
approvals_reviewer: ApprovalsReviewer::User,
|
||||||
approval_messages: None,
|
approval_messages: None,
|
||||||
|
permission_messages: None,
|
||||||
exec_policy: &Policy::empty(),
|
exec_policy: &Policy::empty(),
|
||||||
exec_permission_approvals_enabled: true,
|
exec_permission_approvals_enabled: true,
|
||||||
request_permissions_tool_enabled: false,
|
request_permissions_tool_enabled: false,
|
||||||
@@ -208,6 +347,7 @@ fn includes_request_permissions_tool_instructions_for_on_request_when_tool_is_en
|
|||||||
approval_policy: AskForApproval::OnRequest,
|
approval_policy: AskForApproval::OnRequest,
|
||||||
approvals_reviewer: ApprovalsReviewer::User,
|
approvals_reviewer: ApprovalsReviewer::User,
|
||||||
approval_messages: None,
|
approval_messages: None,
|
||||||
|
permission_messages: None,
|
||||||
exec_policy: &Policy::empty(),
|
exec_policy: &Policy::empty(),
|
||||||
exec_permission_approvals_enabled: false,
|
exec_permission_approvals_enabled: false,
|
||||||
request_permissions_tool_enabled: true,
|
request_permissions_tool_enabled: true,
|
||||||
@@ -229,6 +369,7 @@ fn on_request_includes_tool_guidance_alongside_inline_permission_guidance_when_b
|
|||||||
approval_policy: AskForApproval::OnRequest,
|
approval_policy: AskForApproval::OnRequest,
|
||||||
approvals_reviewer: ApprovalsReviewer::User,
|
approvals_reviewer: ApprovalsReviewer::User,
|
||||||
approval_messages: None,
|
approval_messages: None,
|
||||||
|
permission_messages: None,
|
||||||
exec_policy: &Policy::empty(),
|
exec_policy: &Policy::empty(),
|
||||||
exec_permission_approvals_enabled: true,
|
exec_permission_approvals_enabled: true,
|
||||||
request_permissions_tool_enabled: true,
|
request_permissions_tool_enabled: true,
|
||||||
@@ -289,6 +430,7 @@ fn empty_catalog_approval_message_suppresses_legacy_approval_section() {
|
|||||||
approval_policy: AskForApproval::OnRequest,
|
approval_policy: AskForApproval::OnRequest,
|
||||||
approvals_reviewer: ApprovalsReviewer::User,
|
approvals_reviewer: ApprovalsReviewer::User,
|
||||||
approval_messages: Some(&messages),
|
approval_messages: Some(&messages),
|
||||||
|
permission_messages: None,
|
||||||
exec_policy: &exec_policy,
|
exec_policy: &exec_policy,
|
||||||
exec_permission_approvals_enabled: true,
|
exec_permission_approvals_enabled: true,
|
||||||
request_permissions_tool_enabled: true,
|
request_permissions_tool_enabled: true,
|
||||||
|
|||||||
@@ -506,6 +506,7 @@ pub struct ModelMessages {
|
|||||||
pub instructions_variables: Option<ModelInstructionsVariables>,
|
pub instructions_variables: Option<ModelInstructionsVariables>,
|
||||||
pub approvals: Option<ApprovalMessages>,
|
pub approvals: Option<ApprovalMessages>,
|
||||||
pub auto_review: Option<AutoReviewMessages>,
|
pub auto_review: Option<AutoReviewMessages>,
|
||||||
|
pub permissions: Option<PermissionMessages>,
|
||||||
}
|
}
|
||||||
|
|
||||||
#[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq, TS, JsonSchema)]
|
#[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq, TS, JsonSchema)]
|
||||||
@@ -519,6 +520,13 @@ pub struct AutoReviewMessages {
|
|||||||
pub policy: Option<String>,
|
pub policy: Option<String>,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq, TS, JsonSchema)]
|
||||||
|
pub struct PermissionMessages {
|
||||||
|
pub danger_full_access: Option<String>,
|
||||||
|
pub workspace_write: Option<String>,
|
||||||
|
pub read_only: Option<String>,
|
||||||
|
}
|
||||||
|
|
||||||
impl ModelMessages {
|
impl ModelMessages {
|
||||||
fn has_personality_placeholder(&self) -> bool {
|
fn has_personality_placeholder(&self) -> bool {
|
||||||
self.instructions_template
|
self.instructions_template
|
||||||
@@ -744,6 +752,7 @@ mod tests {
|
|||||||
.expect("model messages should deserialize");
|
.expect("model messages should deserialize");
|
||||||
|
|
||||||
assert_eq!(messages.approvals, None);
|
assert_eq!(messages.approvals, None);
|
||||||
|
assert_eq!(messages.permissions, None);
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
@@ -768,6 +777,29 @@ mod tests {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn permission_messages_preserve_missing_and_empty_values() {
|
||||||
|
let messages: ModelMessages = from_str(
|
||||||
|
r#"{
|
||||||
|
"instructions_template": null,
|
||||||
|
"instructions_variables": null,
|
||||||
|
"permissions": {
|
||||||
|
"workspace_write": ""
|
||||||
|
}
|
||||||
|
}"#,
|
||||||
|
)
|
||||||
|
.expect("permission messages should deserialize");
|
||||||
|
|
||||||
|
assert_eq!(
|
||||||
|
messages.permissions,
|
||||||
|
Some(PermissionMessages {
|
||||||
|
danger_full_access: None,
|
||||||
|
workspace_write: Some(String::new()),
|
||||||
|
read_only: None,
|
||||||
|
})
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn reasoning_effort_accepts_known_and_custom_values() {
|
fn reasoning_effort_accepts_known_and_custom_values() {
|
||||||
let custom = ReasoningEffort::Custom("future".to_string());
|
let custom = ReasoningEffort::Custom("future".to_string());
|
||||||
@@ -841,6 +873,7 @@ mod tests {
|
|||||||
instructions_variables: Some(personality_variables()),
|
instructions_variables: Some(personality_variables()),
|
||||||
approvals: None,
|
approvals: None,
|
||||||
auto_review: None,
|
auto_review: None,
|
||||||
|
permissions: None,
|
||||||
}));
|
}));
|
||||||
|
|
||||||
let instructions = model.get_model_instructions(Some(Personality::Friendly));
|
let instructions = model.get_model_instructions(Some(Personality::Friendly));
|
||||||
@@ -859,6 +892,7 @@ mod tests {
|
|||||||
}),
|
}),
|
||||||
approvals: None,
|
approvals: None,
|
||||||
auto_review: None,
|
auto_review: None,
|
||||||
|
permissions: None,
|
||||||
}));
|
}));
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
model.get_model_instructions(Some(Personality::Friendly)),
|
model.get_model_instructions(Some(Personality::Friendly)),
|
||||||
@@ -886,6 +920,7 @@ mod tests {
|
|||||||
}),
|
}),
|
||||||
approvals: None,
|
approvals: None,
|
||||||
auto_review: None,
|
auto_review: None,
|
||||||
|
permissions: None,
|
||||||
}));
|
}));
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
model_no_personality.get_model_instructions(Some(Personality::Friendly)),
|
model_no_personality.get_model_instructions(Some(Personality::Friendly)),
|
||||||
@@ -916,6 +951,7 @@ mod tests {
|
|||||||
}),
|
}),
|
||||||
approvals: None,
|
approvals: None,
|
||||||
auto_review: None,
|
auto_review: None,
|
||||||
|
permissions: None,
|
||||||
}));
|
}));
|
||||||
|
|
||||||
let instructions = model.get_model_instructions(Some(Personality::Friendly));
|
let instructions = model.get_model_instructions(Some(Personality::Friendly));
|
||||||
|
|||||||
Reference in new issue
Block a user