mirror of
https://github.com/openai/codex.git
synced 2026-11-13 06:45:07 +00:00
Use model catalog approval messages (#31312)
## Why Approval guidance is currently assembled entirely by the client. Model Messages V2 needs model catalogs to provide model-specific `on_request` guidance for both user-reviewed and auto-reviewed approval flows while retaining the existing generated prompt as a compatibility fallback. ## What changed - add nullable `on_request` and `on_request_auto_review` catalog messages - select the message matching the active approvals reviewer for `on_request` policies - replace the complete legacy approval section when the selected catalog value exists, including support for an empty string that suppresses the section - retain legacy rendering when the object or selected key is absent, and for non-`on_request` policies - preserve approval messages when base-instruction or personality overrides clear instruction templates - refresh permissions instructions when the active model changes - pass catalog messages through initial and incremental permissions construction ## Relationship to reviewer persistence PR #31309 independently persists the approvals reviewer in turn context and refreshes permissions when that reviewer changes. This PR is based directly on `main` and does not duplicate that rollout migration; once both land, reviewer switches will also select and append the new catalog variant. ## Testing - `just test -p codex-protocol` - `just test -p codex-prompts` - `just test -p codex-models-manager` - `just test -p codex-core permissions_messages`
This commit is contained in:
1 parent
1345c16dd7
commit
358575465c
13 files changed
+419
-20
No files matched your search
@@ -62,6 +62,7 @@ pub(crate) use legacy_unified_exec_process_limit_warning::LegacyUnifiedExecProce
|
|||||||
pub(crate) use model_switch_instructions::ModelSwitchInstructions;
|
pub(crate) use model_switch_instructions::ModelSwitchInstructions;
|
||||||
pub(crate) use multi_agent_mode_instructions::MultiAgentModeInstructions;
|
pub(crate) use multi_agent_mode_instructions::MultiAgentModeInstructions;
|
||||||
pub(crate) use network_rule_saved::NetworkRuleSaved;
|
pub(crate) use network_rule_saved::NetworkRuleSaved;
|
||||||
|
pub use permissions_instructions::ApprovalPromptContext;
|
||||||
pub use permissions_instructions::PermissionsInstructions;
|
pub use permissions_instructions::PermissionsInstructions;
|
||||||
pub(crate) use personality_spec_instructions::PersonalitySpecInstructions;
|
pub(crate) use personality_spec_instructions::PersonalitySpecInstructions;
|
||||||
pub(crate) use plugin_instructions::PluginInstructions;
|
pub(crate) use plugin_instructions::PluginInstructions;
|
||||||
|
|||||||
@@ -1 +1,2 @@
|
|||||||
|
pub use codex_prompts::ApprovalPromptContext;
|
||||||
pub use codex_prompts::PermissionsInstructions;
|
pub use codex_prompts::PermissionsInstructions;
|
||||||
@@ -1,3 +1,4 @@
|
|||||||
|
use crate::context::ApprovalPromptContext;
|
||||||
use crate::context::CollaborationModeInstructions;
|
use crate::context::CollaborationModeInstructions;
|
||||||
use crate::context::ContextualUserFragment;
|
use crate::context::ContextualUserFragment;
|
||||||
use crate::context::ModelSwitchInstructions;
|
use crate::context::ModelSwitchInstructions;
|
||||||
@@ -30,6 +31,7 @@ fn build_permissions_update_item(
|
|||||||
let prev = previous?;
|
let prev = previous?;
|
||||||
if prev.permission_profile() == next.permission_profile()
|
if prev.permission_profile() == next.permission_profile()
|
||||||
&& prev.approval_policy == next.approval_policy.value()
|
&& prev.approval_policy == next.approval_policy.value()
|
||||||
|
&& prev.model == next.model_info.slug
|
||||||
{
|
{
|
||||||
return None;
|
return None;
|
||||||
}
|
}
|
||||||
@@ -38,7 +40,13 @@ fn build_permissions_update_item(
|
|||||||
PermissionsInstructions::from_permission_profile(
|
PermissionsInstructions::from_permission_profile(
|
||||||
&next.permission_profile,
|
&next.permission_profile,
|
||||||
next.approval_policy.value(),
|
next.approval_policy.value(),
|
||||||
next.config.approvals_reviewer,
|
ApprovalPromptContext::new(
|
||||||
|
next.config.approvals_reviewer,
|
||||||
|
next.model_info
|
||||||
|
.model_messages
|
||||||
|
.as_ref()
|
||||||
|
.and_then(|messages| messages.approvals.as_ref()),
|
||||||
|
),
|
||||||
exec_policy,
|
exec_policy,
|
||||||
#[allow(deprecated)]
|
#[allow(deprecated)]
|
||||||
&next.cwd,
|
&next.cwd,
|
||||||
|
|||||||
@@ -21,6 +21,7 @@ use crate::build_available_skills;
|
|||||||
use crate::compact;
|
use crate::compact;
|
||||||
use crate::config::ManagedFeatures;
|
use crate::config::ManagedFeatures;
|
||||||
use crate::config::resolve_tool_suggest_config_from_layer_stack;
|
use crate::config::resolve_tool_suggest_config_from_layer_stack;
|
||||||
|
use crate::context::ApprovalPromptContext;
|
||||||
use crate::context::ApprovedCommandPrefixSaved;
|
use crate::context::ApprovedCommandPrefixSaved;
|
||||||
use crate::context::AvailableSkillsInstructions;
|
use crate::context::AvailableSkillsInstructions;
|
||||||
use crate::context::CollaborationModeInstructions;
|
use crate::context::CollaborationModeInstructions;
|
||||||
@@ -3206,7 +3207,14 @@ impl Session {
|
|||||||
PermissionsInstructions::from_permission_profile(
|
PermissionsInstructions::from_permission_profile(
|
||||||
&turn_context.permission_profile,
|
&turn_context.permission_profile,
|
||||||
turn_context.approval_policy.value(),
|
turn_context.approval_policy.value(),
|
||||||
turn_context.config.approvals_reviewer,
|
ApprovalPromptContext::new(
|
||||||
|
turn_context.config.approvals_reviewer,
|
||||||
|
turn_context
|
||||||
|
.model_info
|
||||||
|
.model_messages
|
||||||
|
.as_ref()
|
||||||
|
.and_then(|messages| messages.approvals.as_ref()),
|
||||||
|
),
|
||||||
self.services.exec_policy.current().as_ref(),
|
self.services.exec_policy.current().as_ref(),
|
||||||
#[allow(deprecated)]
|
#[allow(deprecated)]
|
||||||
&turn_context.cwd,
|
&turn_context.cwd,
|
||||||
|
|||||||
@@ -9041,18 +9041,13 @@ async fn record_context_updates_and_set_reference_context_item_reinjects_full_co
|
|||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn record_context_updates_and_set_reference_context_item_persists_baseline_without_emitting_diffs()
|
async fn record_context_updates_and_set_reference_context_item_persists_baseline_without_emitting_diffs()
|
||||||
{
|
{
|
||||||
let (mut session, previous_context) = make_session_and_context().await;
|
let (mut session, turn_context) = make_session_and_context().await;
|
||||||
let next_model = if previous_context.model_info.slug == "gpt-5.4" {
|
let previous_context_item = turn_context.to_turn_context_item();
|
||||||
"gpt-5.2"
|
let previous_context = Arc::new(turn_context);
|
||||||
} else {
|
|
||||||
"gpt-5.4"
|
|
||||||
};
|
|
||||||
let turn_context = previous_context
|
|
||||||
.with_model(next_model.to_string(), &session.services.models_manager)
|
|
||||||
.await;
|
|
||||||
let previous_context_item = previous_context.to_turn_context_item();
|
|
||||||
let previous_context = Arc::new(previous_context);
|
|
||||||
let world_state = build_world_state_from_turn_context(&session, &previous_context).await;
|
let world_state = build_world_state_from_turn_context(&session, &previous_context).await;
|
||||||
|
let mut turn_context = Arc::try_unwrap(previous_context)
|
||||||
|
.unwrap_or_else(|_| panic!("previous turn context should have no remaining references"));
|
||||||
|
turn_context.sub_id = format!("{}-next", turn_context.sub_id);
|
||||||
{
|
{
|
||||||
let mut state = session.state.lock().await;
|
let mut state = session.state.lock().await;
|
||||||
state.set_reference_context_item(Some(previous_context_item.clone()));
|
state.set_reference_context_item(Some(previous_context_item.clone()));
|
||||||
|
|||||||
@@ -2,10 +2,15 @@ use anyhow::Result;
|
|||||||
use codex_config::ConfigLayerStack;
|
use codex_config::ConfigLayerStack;
|
||||||
use codex_core::ForkSnapshot;
|
use codex_core::ForkSnapshot;
|
||||||
use codex_core::config::Constrained;
|
use codex_core::config::Constrained;
|
||||||
|
use codex_core::context::ApprovalPromptContext;
|
||||||
use codex_core::context::ContextualUserFragment;
|
use codex_core::context::ContextualUserFragment;
|
||||||
use codex_core::context::PermissionsInstructions;
|
use codex_core::context::PermissionsInstructions;
|
||||||
use codex_core::load_exec_policy;
|
use codex_core::load_exec_policy;
|
||||||
|
use codex_models_manager::model_info::model_info_from_slug;
|
||||||
use codex_protocol::models::PermissionProfile;
|
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::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;
|
||||||
@@ -33,6 +38,129 @@ fn permissions_texts(request: &ResponsesRequest) -> Vec<String> {
|
|||||||
.collect()
|
.collect()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn model_with_approval_messages(
|
||||||
|
slug: &str,
|
||||||
|
on_request: &str,
|
||||||
|
on_request_auto_review: &str,
|
||||||
|
) -> 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: Some(ApprovalMessages {
|
||||||
|
on_request: Some(on_request.to_string()),
|
||||||
|
on_request_auto_review: Some(on_request_auto_review.to_string()),
|
||||||
|
}),
|
||||||
|
});
|
||||||
|
model
|
||||||
|
}
|
||||||
|
|
||||||
|
async fn submit_text_turn(
|
||||||
|
test: &core_test_support::test_codex::TestCodex,
|
||||||
|
text: &str,
|
||||||
|
) -> Result<()> {
|
||||||
|
test.codex
|
||||||
|
.submit(Op::UserInput {
|
||||||
|
items: vec![UserInput::Text {
|
||||||
|
text: text.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, |ev| matches!(ev, EventMsg::TurnComplete(_))).await;
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
|
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||||
|
async fn catalog_approval_message_is_sent_in_initial_permissions() -> 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 model = model_with_approval_messages(
|
||||||
|
model_slug,
|
||||||
|
"catalog user approval instructions",
|
||||||
|
"catalog auto-review approval instructions",
|
||||||
|
);
|
||||||
|
let mut builder = test_codex()
|
||||||
|
.with_model(model_slug)
|
||||||
|
.with_config(move |config| {
|
||||||
|
config.permissions.approval_policy = Constrained::allow_any(AskForApproval::OnRequest);
|
||||||
|
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("catalog user approval instructions"));
|
||||||
|
assert!(permissions[0].contains("Filesystem sandboxing defines"));
|
||||||
|
assert!(!permissions[0].contains("How to request escalation"));
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
|
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||||
|
async fn model_change_appends_new_catalog_approval_message() -> 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-approvals-model-a";
|
||||||
|
let second_slug = "catalog-approvals-model-b";
|
||||||
|
let first = model_with_approval_messages(first_slug, "model A approvals", "model A auto");
|
||||||
|
let second = model_with_approval_messages(second_slug, "model B approvals", "model B auto");
|
||||||
|
let mut builder = test_codex()
|
||||||
|
.with_model(first_slug)
|
||||||
|
.with_config(move |config| {
|
||||||
|
config.permissions.approval_policy = Constrained::allow_any(AskForApproval::OnRequest);
|
||||||
|
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
|
||||||
|
.last()
|
||||||
|
.is_some_and(|text| text.contains("model B approvals"))
|
||||||
|
);
|
||||||
|
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(()));
|
||||||
@@ -563,7 +691,7 @@ 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,
|
||||||
test.config.approvals_reviewer,
|
ApprovalPromptContext::new(test.config.approvals_reviewer, /*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,
|
||||||
|
|||||||
@@ -573,6 +573,7 @@ async fn remote_model_friendly_personality_instructions_with_feature() -> anyhow
|
|||||||
personality_friendly: Some(friendly_personality_message.to_string()),
|
personality_friendly: Some(friendly_personality_message.to_string()),
|
||||||
personality_pragmatic: Some("Pragmatic variant".to_string()),
|
personality_pragmatic: Some("Pragmatic variant".to_string()),
|
||||||
}),
|
}),
|
||||||
|
approvals: None,
|
||||||
}),
|
}),
|
||||||
include_skills_usage_instructions: false,
|
include_skills_usage_instructions: false,
|
||||||
supports_reasoning_summaries: false,
|
supports_reasoning_summaries: false,
|
||||||
@@ -689,6 +690,7 @@ async fn user_turn_personality_remote_model_template_includes_update_message() -
|
|||||||
personality_friendly: Some(remote_friendly_message.to_string()),
|
personality_friendly: Some(remote_friendly_message.to_string()),
|
||||||
personality_pragmatic: Some(remote_pragmatic_message.to_string()),
|
personality_pragmatic: Some(remote_pragmatic_message.to_string()),
|
||||||
}),
|
}),
|
||||||
|
approvals: None,
|
||||||
}),
|
}),
|
||||||
include_skills_usage_instructions: false,
|
include_skills_usage_instructions: false,
|
||||||
supports_reasoning_summaries: false,
|
supports_reasoning_summaries: false,
|
||||||
|
|||||||
@@ -54,14 +54,24 @@ pub fn with_config_overrides(mut model: ModelInfo, config: &ModelsManagerConfig)
|
|||||||
|
|
||||||
if let Some(base_instructions) = &config.base_instructions {
|
if let Some(base_instructions) = &config.base_instructions {
|
||||||
model.base_instructions = base_instructions.clone();
|
model.base_instructions = base_instructions.clone();
|
||||||
model.model_messages = None;
|
clear_instruction_messages(&mut model);
|
||||||
} else if !config.personality_enabled {
|
} else if !config.personality_enabled {
|
||||||
model.model_messages = None;
|
clear_instruction_messages(&mut model);
|
||||||
}
|
}
|
||||||
|
|
||||||
model
|
model
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn clear_instruction_messages(model: &mut ModelInfo) {
|
||||||
|
if let Some(model_messages) = &mut model.model_messages {
|
||||||
|
model_messages.instructions_template = None;
|
||||||
|
model_messages.instructions_variables = None;
|
||||||
|
if model_messages.approvals.is_none() {
|
||||||
|
model.model_messages = None;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// Build a minimal fallback model descriptor for missing/unknown slugs.
|
/// Build a minimal fallback model descriptor for missing/unknown slugs.
|
||||||
pub fn model_info_from_slug(slug: &str) -> ModelInfo {
|
pub fn model_info_from_slug(slug: &str) -> ModelInfo {
|
||||||
warn!("Unknown model {slug} is used. This will use fallback model metadata.");
|
warn!("Unknown model {slug} is used. This will use fallback model metadata.");
|
||||||
@@ -119,6 +129,7 @@ fn local_personality_messages_for_slug(slug: &str) -> Option<ModelMessages> {
|
|||||||
personality_friendly: Some(LOCAL_FRIENDLY_TEMPLATE.to_string()),
|
personality_friendly: Some(LOCAL_FRIENDLY_TEMPLATE.to_string()),
|
||||||
personality_pragmatic: Some(LOCAL_PRAGMATIC_TEMPLATE.to_string()),
|
personality_pragmatic: Some(LOCAL_PRAGMATIC_TEMPLATE.to_string()),
|
||||||
}),
|
}),
|
||||||
|
approvals: None,
|
||||||
}),
|
}),
|
||||||
_ => None,
|
_ => None,
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,5 +1,6 @@
|
|||||||
use super::*;
|
use super::*;
|
||||||
use crate::ModelsManagerConfig;
|
use crate::ModelsManagerConfig;
|
||||||
|
use codex_protocol::openai_models::ApprovalMessages;
|
||||||
use pretty_assertions::assert_eq;
|
use pretty_assertions::assert_eq;
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
@@ -44,6 +45,68 @@ fn reasoning_summaries_override_false_is_noop_when_model_is_false() {
|
|||||||
assert_eq!(updated, model);
|
assert_eq!(updated, model);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn base_instruction_override_preserves_catalog_approval_messages() {
|
||||||
|
let mut model = model_info_from_slug("unknown-model");
|
||||||
|
let approvals = ApprovalMessages {
|
||||||
|
on_request: Some("user approvals".to_string()),
|
||||||
|
on_request_auto_review: Some("auto approvals".to_string()),
|
||||||
|
};
|
||||||
|
model.model_messages = Some(ModelMessages {
|
||||||
|
instructions_template: Some("template".to_string()),
|
||||||
|
instructions_variables: Some(ModelInstructionsVariables {
|
||||||
|
personality_default: Some("default".to_string()),
|
||||||
|
personality_friendly: Some("friendly".to_string()),
|
||||||
|
personality_pragmatic: Some("pragmatic".to_string()),
|
||||||
|
}),
|
||||||
|
approvals: Some(approvals.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: Some(approvals),
|
||||||
|
})
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn disabled_personality_preserves_catalog_approval_messages() {
|
||||||
|
let mut model = model_info_from_slug("unknown-model");
|
||||||
|
let approvals = ApprovalMessages {
|
||||||
|
on_request: Some("user approvals".to_string()),
|
||||||
|
on_request_auto_review: None,
|
||||||
|
};
|
||||||
|
model.model_messages = Some(ModelMessages {
|
||||||
|
instructions_template: Some("template".to_string()),
|
||||||
|
instructions_variables: None,
|
||||||
|
approvals: Some(approvals.clone()),
|
||||||
|
});
|
||||||
|
let config = ModelsManagerConfig {
|
||||||
|
personality_enabled: false,
|
||||||
|
..Default::default()
|
||||||
|
};
|
||||||
|
|
||||||
|
let updated = with_config_overrides(model, &config);
|
||||||
|
|
||||||
|
assert_eq!(
|
||||||
|
updated.model_messages,
|
||||||
|
Some(ModelMessages {
|
||||||
|
instructions_template: None,
|
||||||
|
instructions_variables: None,
|
||||||
|
approvals: Some(approvals),
|
||||||
|
})
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn model_context_window_override_clamps_to_max_context_window() {
|
fn model_context_window_override_clamps_to_max_context_window() {
|
||||||
let mut model = model_info_from_slug("unknown-model");
|
let mut model = model_info_from_slug("unknown-model");
|
||||||
|
|||||||
@@ -12,6 +12,7 @@ pub use compact::SUMMARY_PREFIX;
|
|||||||
pub use goals::budget_limit_prompt;
|
pub use goals::budget_limit_prompt;
|
||||||
pub use goals::continuation_prompt;
|
pub use goals::continuation_prompt;
|
||||||
pub use goals::objective_updated_prompt;
|
pub use goals::objective_updated_prompt;
|
||||||
|
pub use permissions_instructions::ApprovalPromptContext;
|
||||||
pub use permissions_instructions::PermissionsInstructions;
|
pub use permissions_instructions::PermissionsInstructions;
|
||||||
pub use realtime::BACKEND_PROMPT;
|
pub use realtime::BACKEND_PROMPT;
|
||||||
pub use realtime::END_INSTRUCTIONS;
|
pub use realtime::END_INSTRUCTIONS;
|
||||||
|
|||||||
@@ -4,6 +4,7 @@ use codex_protocol::config_types::ApprovalsReviewer;
|
|||||||
use codex_protocol::config_types::SandboxMode;
|
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::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;
|
||||||
@@ -47,6 +48,7 @@ static SANDBOX_MODE_READ_ONLY_TEMPLATE: LazyLock<Template> = LazyLock::new(|| {
|
|||||||
struct PermissionsPromptConfig<'a> {
|
struct PermissionsPromptConfig<'a> {
|
||||||
approval_policy: AskForApproval,
|
approval_policy: AskForApproval,
|
||||||
approvals_reviewer: ApprovalsReviewer,
|
approvals_reviewer: ApprovalsReviewer,
|
||||||
|
approval_messages: Option<&'a ApprovalMessages>,
|
||||||
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,
|
||||||
@@ -58,12 +60,24 @@ pub struct PermissionsInstructions {
|
|||||||
text: String,
|
text: String,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[derive(Debug, Clone, Copy)]
|
||||||
|
pub struct ApprovalPromptContext<'a> {
|
||||||
|
reviewer: ApprovalsReviewer,
|
||||||
|
messages: Option<&'a ApprovalMessages>,
|
||||||
|
}
|
||||||
|
|
||||||
|
impl<'a> ApprovalPromptContext<'a> {
|
||||||
|
pub fn new(reviewer: ApprovalsReviewer, messages: Option<&'a ApprovalMessages>) -> Self {
|
||||||
|
Self { reviewer, messages }
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
impl PermissionsInstructions {
|
impl PermissionsInstructions {
|
||||||
/// Builds permissions instructions from the effective permission profile and approval policy.
|
/// Builds permissions instructions from the effective permission profile and approval policy.
|
||||||
pub fn from_permission_profile(
|
pub fn from_permission_profile(
|
||||||
permission_profile: &PermissionProfile,
|
permission_profile: &PermissionProfile,
|
||||||
approval_policy: AskForApproval,
|
approval_policy: AskForApproval,
|
||||||
approvals_reviewer: ApprovalsReviewer,
|
approval_context: ApprovalPromptContext<'_>,
|
||||||
exec_policy: &Policy,
|
exec_policy: &Policy,
|
||||||
cwd: &Path,
|
cwd: &Path,
|
||||||
exec_permission_approvals_enabled: bool,
|
exec_permission_approvals_enabled: bool,
|
||||||
@@ -78,7 +92,8 @@ impl PermissionsInstructions {
|
|||||||
network_access_from_policy(permission_profile.network_sandbox_policy()),
|
network_access_from_policy(permission_profile.network_sandbox_policy()),
|
||||||
PermissionsPromptConfig {
|
PermissionsPromptConfig {
|
||||||
approval_policy,
|
approval_policy,
|
||||||
approvals_reviewer,
|
approvals_reviewer: approval_context.reviewer,
|
||||||
|
approval_messages: approval_context.messages,
|
||||||
exec_policy,
|
exec_policy,
|
||||||
exec_permission_approvals_enabled,
|
exec_permission_approvals_enabled,
|
||||||
request_permissions_tool_enabled,
|
request_permissions_tool_enabled,
|
||||||
@@ -122,6 +137,7 @@ impl PermissionsInstructions {
|
|||||||
&approval_text(
|
&approval_text(
|
||||||
config.approval_policy,
|
config.approval_policy,
|
||||||
config.approvals_reviewer,
|
config.approvals_reviewer,
|
||||||
|
config.approval_messages,
|
||||||
config.exec_policy,
|
config.exec_policy,
|
||||||
config.exec_permission_approvals_enabled,
|
config.exec_permission_approvals_enabled,
|
||||||
config.request_permissions_tool_enabled,
|
config.request_permissions_tool_enabled,
|
||||||
@@ -192,10 +208,23 @@ fn append_section(text: &mut String, section: &str) {
|
|||||||
fn approval_text(
|
fn approval_text(
|
||||||
approval_policy: AskForApproval,
|
approval_policy: AskForApproval,
|
||||||
approvals_reviewer: ApprovalsReviewer,
|
approvals_reviewer: ApprovalsReviewer,
|
||||||
|
approval_messages: Option<&ApprovalMessages>,
|
||||||
exec_policy: &Policy,
|
exec_policy: &Policy,
|
||||||
exec_permission_approvals_enabled: bool,
|
exec_permission_approvals_enabled: bool,
|
||||||
request_permissions_tool_enabled: bool,
|
request_permissions_tool_enabled: bool,
|
||||||
) -> String {
|
) -> String {
|
||||||
|
if approval_policy == AskForApproval::OnRequest
|
||||||
|
&& let Some(approval_messages) = approval_messages
|
||||||
|
{
|
||||||
|
let selected = match approvals_reviewer {
|
||||||
|
ApprovalsReviewer::User => approval_messages.on_request.as_ref(),
|
||||||
|
ApprovalsReviewer::AutoReview => approval_messages.on_request_auto_review.as_ref(),
|
||||||
|
};
|
||||||
|
if let Some(selected) = selected {
|
||||||
|
return selected.clone();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
let with_request_permissions_tool = |text: &str| {
|
let with_request_permissions_tool = |text: &str| {
|
||||||
if request_permissions_tool_enabled {
|
if request_permissions_tool_enabled {
|
||||||
format!("{text}\n\n{}", request_permissions_tool_prompt_section())
|
format!("{text}\n\n{}", request_permissions_tool_prompt_section())
|
||||||
|
|||||||
@@ -36,6 +36,7 @@ fn builds_permissions_with_network_access_override() {
|
|||||||
PermissionsPromptConfig {
|
PermissionsPromptConfig {
|
||||||
approval_policy: AskForApproval::OnRequest,
|
approval_policy: AskForApproval::OnRequest,
|
||||||
approvals_reviewer: ApprovalsReviewer::User,
|
approvals_reviewer: ApprovalsReviewer::User,
|
||||||
|
approval_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,
|
||||||
@@ -72,7 +73,7 @@ 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,
|
||||||
ApprovalsReviewer::User,
|
ApprovalPromptContext::new(ApprovalsReviewer::User, /*messages*/ None),
|
||||||
&Policy::empty(),
|
&Policy::empty(),
|
||||||
&cwd,
|
&cwd,
|
||||||
/*exec_permission_approvals_enabled*/ false,
|
/*exec_permission_approvals_enabled*/ false,
|
||||||
@@ -117,7 +118,7 @@ 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,
|
||||||
ApprovalsReviewer::AutoReview,
|
ApprovalPromptContext::new(ApprovalsReviewer::AutoReview, /*messages*/ None),
|
||||||
&Policy::empty(),
|
&Policy::empty(),
|
||||||
&cwd,
|
&cwd,
|
||||||
/*exec_permission_approvals_enabled*/ false,
|
/*exec_permission_approvals_enabled*/ false,
|
||||||
@@ -142,6 +143,7 @@ fn includes_request_rule_instructions_for_on_request() {
|
|||||||
PermissionsPromptConfig {
|
PermissionsPromptConfig {
|
||||||
approval_policy: AskForApproval::OnRequest,
|
approval_policy: AskForApproval::OnRequest,
|
||||||
approvals_reviewer: ApprovalsReviewer::User,
|
approvals_reviewer: ApprovalsReviewer::User,
|
||||||
|
approval_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,
|
||||||
@@ -163,6 +165,7 @@ fn includes_request_permissions_tool_instructions_for_unless_trusted_when_enable
|
|||||||
PermissionsPromptConfig {
|
PermissionsPromptConfig {
|
||||||
approval_policy: AskForApproval::UnlessTrusted,
|
approval_policy: AskForApproval::UnlessTrusted,
|
||||||
approvals_reviewer: ApprovalsReviewer::User,
|
approvals_reviewer: ApprovalsReviewer::User,
|
||||||
|
approval_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,
|
||||||
@@ -183,6 +186,7 @@ fn includes_request_permission_rule_instructions_for_on_request_when_enabled() {
|
|||||||
PermissionsPromptConfig {
|
PermissionsPromptConfig {
|
||||||
approval_policy: AskForApproval::OnRequest,
|
approval_policy: AskForApproval::OnRequest,
|
||||||
approvals_reviewer: ApprovalsReviewer::User,
|
approvals_reviewer: ApprovalsReviewer::User,
|
||||||
|
approval_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,
|
||||||
@@ -203,6 +207,7 @@ fn includes_request_permissions_tool_instructions_for_on_request_when_tool_is_en
|
|||||||
PermissionsPromptConfig {
|
PermissionsPromptConfig {
|
||||||
approval_policy: AskForApproval::OnRequest,
|
approval_policy: AskForApproval::OnRequest,
|
||||||
approvals_reviewer: ApprovalsReviewer::User,
|
approvals_reviewer: ApprovalsReviewer::User,
|
||||||
|
approval_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,
|
||||||
@@ -223,6 +228,7 @@ fn on_request_includes_tool_guidance_alongside_inline_permission_guidance_when_b
|
|||||||
PermissionsPromptConfig {
|
PermissionsPromptConfig {
|
||||||
approval_policy: AskForApproval::OnRequest,
|
approval_policy: AskForApproval::OnRequest,
|
||||||
approvals_reviewer: ApprovalsReviewer::User,
|
approvals_reviewer: ApprovalsReviewer::User,
|
||||||
|
approval_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,
|
||||||
@@ -235,11 +241,108 @@ fn on_request_includes_tool_guidance_alongside_inline_permission_guidance_when_b
|
|||||||
assert!(text.contains("# request_permissions Tool"));
|
assert!(text.contains("# request_permissions Tool"));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn catalog_approval_messages_select_reviewer_variant() {
|
||||||
|
let messages = ApprovalMessages {
|
||||||
|
on_request: Some("user catalog approvals".to_string()),
|
||||||
|
on_request_auto_review: Some("auto-review catalog approvals".to_string()),
|
||||||
|
};
|
||||||
|
|
||||||
|
for (reviewer, expected) in [
|
||||||
|
(ApprovalsReviewer::User, "user catalog approvals"),
|
||||||
|
(
|
||||||
|
ApprovalsReviewer::AutoReview,
|
||||||
|
"auto-review catalog approvals",
|
||||||
|
),
|
||||||
|
] {
|
||||||
|
assert_eq!(
|
||||||
|
approval_text(
|
||||||
|
AskForApproval::OnRequest,
|
||||||
|
reviewer,
|
||||||
|
Some(&messages),
|
||||||
|
&Policy::empty(),
|
||||||
|
/*exec_permission_approvals_enabled*/ true,
|
||||||
|
/*request_permissions_tool_enabled*/ true,
|
||||||
|
),
|
||||||
|
expected
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn empty_catalog_approval_message_suppresses_legacy_approval_section() {
|
||||||
|
let messages = ApprovalMessages {
|
||||||
|
on_request: Some(String::new()),
|
||||||
|
on_request_auto_review: None,
|
||||||
|
};
|
||||||
|
let mut exec_policy = Policy::empty();
|
||||||
|
exec_policy
|
||||||
|
.add_prefix_rule(&["git".to_string(), "pull".to_string()], Decision::Allow)
|
||||||
|
.expect("add rule");
|
||||||
|
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::OnRequest,
|
||||||
|
approvals_reviewer: ApprovalsReviewer::User,
|
||||||
|
approval_messages: Some(&messages),
|
||||||
|
exec_policy: &exec_policy,
|
||||||
|
exec_permission_approvals_enabled: true,
|
||||||
|
request_permissions_tool_enabled: true,
|
||||||
|
},
|
||||||
|
Some(vec![WritableRoot {
|
||||||
|
root: writable_root.clone(),
|
||||||
|
read_only_subpaths: Vec::new(),
|
||||||
|
protected_metadata_names: Vec::new(),
|
||||||
|
}]),
|
||||||
|
);
|
||||||
|
let text = instructions.body();
|
||||||
|
|
||||||
|
assert!(text.contains("`sandbox_mode` is `workspace-write`"));
|
||||||
|
assert!(text.contains("Network access is restricted."));
|
||||||
|
assert!(text.contains(writable_root.to_string_lossy().as_ref()));
|
||||||
|
assert!(!text.contains("How to request escalation"));
|
||||||
|
assert!(!text.contains("request_permissions Tool"));
|
||||||
|
assert!(!text.contains("Approved command prefixes"));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn missing_catalog_key_and_non_on_request_policy_use_legacy_approval_text() {
|
||||||
|
let messages = ApprovalMessages {
|
||||||
|
on_request: None,
|
||||||
|
on_request_auto_review: Some("unused catalog approvals".to_string()),
|
||||||
|
};
|
||||||
|
|
||||||
|
let on_request = approval_text(
|
||||||
|
AskForApproval::OnRequest,
|
||||||
|
ApprovalsReviewer::User,
|
||||||
|
Some(&messages),
|
||||||
|
&Policy::empty(),
|
||||||
|
/*exec_permission_approvals_enabled*/ false,
|
||||||
|
/*request_permissions_tool_enabled*/ false,
|
||||||
|
);
|
||||||
|
let never = approval_text(
|
||||||
|
AskForApproval::Never,
|
||||||
|
ApprovalsReviewer::User,
|
||||||
|
Some(&messages),
|
||||||
|
&Policy::empty(),
|
||||||
|
/*exec_permission_approvals_enabled*/ false,
|
||||||
|
/*request_permissions_tool_enabled*/ false,
|
||||||
|
);
|
||||||
|
|
||||||
|
assert!(on_request.contains("How to request escalation"));
|
||||||
|
assert_eq!(never, APPROVAL_POLICY_NEVER);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn auto_review_approvals_append_auto_review_specific_guidance() {
|
fn auto_review_approvals_append_auto_review_specific_guidance() {
|
||||||
let text = approval_text(
|
let text = approval_text(
|
||||||
AskForApproval::OnRequest,
|
AskForApproval::OnRequest,
|
||||||
ApprovalsReviewer::AutoReview,
|
ApprovalsReviewer::AutoReview,
|
||||||
|
/*approval_messages*/ None,
|
||||||
&Policy::empty(),
|
&Policy::empty(),
|
||||||
/*exec_permission_approvals_enabled*/ false,
|
/*exec_permission_approvals_enabled*/ false,
|
||||||
/*request_permissions_tool_enabled*/ false,
|
/*request_permissions_tool_enabled*/ false,
|
||||||
@@ -255,6 +358,7 @@ fn auto_review_approvals_omit_auto_review_specific_guidance_when_approval_is_nev
|
|||||||
let text = approval_text(
|
let text = approval_text(
|
||||||
AskForApproval::Never,
|
AskForApproval::Never,
|
||||||
ApprovalsReviewer::AutoReview,
|
ApprovalsReviewer::AutoReview,
|
||||||
|
/*approval_messages*/ None,
|
||||||
&Policy::empty(),
|
&Policy::empty(),
|
||||||
/*exec_permission_approvals_enabled*/ false,
|
/*exec_permission_approvals_enabled*/ false,
|
||||||
/*request_permissions_tool_enabled*/ false,
|
/*request_permissions_tool_enabled*/ false,
|
||||||
@@ -307,6 +411,7 @@ fn granular_policy_lists_prompted_and_rejected_categories_separately() {
|
|||||||
mcp_elicitations: false,
|
mcp_elicitations: false,
|
||||||
}),
|
}),
|
||||||
ApprovalsReviewer::User,
|
ApprovalsReviewer::User,
|
||||||
|
/*approval_messages*/ None,
|
||||||
&Policy::empty(),
|
&Policy::empty(),
|
||||||
/*exec_permission_approvals_enabled*/ true,
|
/*exec_permission_approvals_enabled*/ true,
|
||||||
/*request_permissions_tool_enabled*/ false,
|
/*request_permissions_tool_enabled*/ false,
|
||||||
@@ -344,6 +449,7 @@ fn granular_policy_includes_command_permission_instructions_when_sandbox_approva
|
|||||||
mcp_elicitations: true,
|
mcp_elicitations: true,
|
||||||
}),
|
}),
|
||||||
ApprovalsReviewer::User,
|
ApprovalsReviewer::User,
|
||||||
|
/*approval_messages*/ None,
|
||||||
&Policy::empty(),
|
&Policy::empty(),
|
||||||
/*exec_permission_approvals_enabled*/ true,
|
/*exec_permission_approvals_enabled*/ true,
|
||||||
/*request_permissions_tool_enabled*/ false,
|
/*request_permissions_tool_enabled*/ false,
|
||||||
@@ -376,6 +482,7 @@ fn granular_policy_omits_shell_permission_instructions_when_inline_requests_are_
|
|||||||
mcp_elicitations: true,
|
mcp_elicitations: true,
|
||||||
}),
|
}),
|
||||||
ApprovalsReviewer::User,
|
ApprovalsReviewer::User,
|
||||||
|
/*approval_messages*/ None,
|
||||||
&Policy::empty(),
|
&Policy::empty(),
|
||||||
/*exec_permission_approvals_enabled*/ false,
|
/*exec_permission_approvals_enabled*/ false,
|
||||||
/*request_permissions_tool_enabled*/ false,
|
/*request_permissions_tool_enabled*/ false,
|
||||||
@@ -408,6 +515,7 @@ fn granular_policy_includes_request_permissions_tool_only_when_that_prompt_can_s
|
|||||||
mcp_elicitations: true,
|
mcp_elicitations: true,
|
||||||
}),
|
}),
|
||||||
ApprovalsReviewer::User,
|
ApprovalsReviewer::User,
|
||||||
|
/*approval_messages*/ None,
|
||||||
&Policy::empty(),
|
&Policy::empty(),
|
||||||
/*exec_permission_approvals_enabled*/ true,
|
/*exec_permission_approvals_enabled*/ true,
|
||||||
/*request_permissions_tool_enabled*/ true,
|
/*request_permissions_tool_enabled*/ true,
|
||||||
@@ -423,6 +531,7 @@ fn granular_policy_includes_request_permissions_tool_only_when_that_prompt_can_s
|
|||||||
mcp_elicitations: true,
|
mcp_elicitations: true,
|
||||||
}),
|
}),
|
||||||
ApprovalsReviewer::User,
|
ApprovalsReviewer::User,
|
||||||
|
/*approval_messages*/ None,
|
||||||
&Policy::empty(),
|
&Policy::empty(),
|
||||||
/*exec_permission_approvals_enabled*/ true,
|
/*exec_permission_approvals_enabled*/ true,
|
||||||
/*request_permissions_tool_enabled*/ true,
|
/*request_permissions_tool_enabled*/ true,
|
||||||
@@ -441,6 +550,7 @@ fn granular_policy_lists_request_permissions_category_without_tool_section_when_
|
|||||||
mcp_elicitations: false,
|
mcp_elicitations: false,
|
||||||
}),
|
}),
|
||||||
ApprovalsReviewer::User,
|
ApprovalsReviewer::User,
|
||||||
|
/*approval_messages*/ None,
|
||||||
&Policy::empty(),
|
&Policy::empty(),
|
||||||
/*exec_permission_approvals_enabled*/ true,
|
/*exec_permission_approvals_enabled*/ true,
|
||||||
/*request_permissions_tool_enabled*/ false,
|
/*request_permissions_tool_enabled*/ false,
|
||||||
|
|||||||
@@ -485,6 +485,13 @@ impl ModelInfo {
|
|||||||
pub struct ModelMessages {
|
pub struct ModelMessages {
|
||||||
pub instructions_template: Option<String>,
|
pub instructions_template: Option<String>,
|
||||||
pub instructions_variables: Option<ModelInstructionsVariables>,
|
pub instructions_variables: Option<ModelInstructionsVariables>,
|
||||||
|
pub approvals: Option<ApprovalMessages>,
|
||||||
|
}
|
||||||
|
|
||||||
|
#[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq, TS, JsonSchema)]
|
||||||
|
pub struct ApprovalMessages {
|
||||||
|
pub on_request: Option<String>,
|
||||||
|
pub on_request_auto_review: Option<String>,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl ModelMessages {
|
impl ModelMessages {
|
||||||
@@ -704,6 +711,37 @@ mod tests {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn model_messages_deserialize_without_approvals() {
|
||||||
|
let messages: ModelMessages =
|
||||||
|
from_str(r#"{"instructions_template":null,"instructions_variables":null}"#)
|
||||||
|
.expect("model messages should deserialize");
|
||||||
|
|
||||||
|
assert_eq!(messages.approvals, None);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn approval_messages_preserve_missing_and_empty_values() {
|
||||||
|
let messages: ModelMessages = from_str(
|
||||||
|
r#"{
|
||||||
|
"instructions_template": null,
|
||||||
|
"instructions_variables": null,
|
||||||
|
"approvals": {
|
||||||
|
"on_request": ""
|
||||||
|
}
|
||||||
|
}"#,
|
||||||
|
)
|
||||||
|
.expect("approval messages should deserialize");
|
||||||
|
|
||||||
|
assert_eq!(
|
||||||
|
messages.approvals,
|
||||||
|
Some(ApprovalMessages {
|
||||||
|
on_request: Some(String::new()),
|
||||||
|
on_request_auto_review: 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());
|
||||||
@@ -775,6 +813,7 @@ mod tests {
|
|||||||
let model = test_model(Some(ModelMessages {
|
let model = test_model(Some(ModelMessages {
|
||||||
instructions_template: Some("Hello {{ personality }}".to_string()),
|
instructions_template: Some("Hello {{ personality }}".to_string()),
|
||||||
instructions_variables: Some(personality_variables()),
|
instructions_variables: Some(personality_variables()),
|
||||||
|
approvals: None,
|
||||||
}));
|
}));
|
||||||
|
|
||||||
let instructions = model.get_model_instructions(Some(Personality::Friendly));
|
let instructions = model.get_model_instructions(Some(Personality::Friendly));
|
||||||
@@ -791,6 +830,7 @@ mod tests {
|
|||||||
personality_friendly: Some("friendly".to_string()),
|
personality_friendly: Some("friendly".to_string()),
|
||||||
personality_pragmatic: None,
|
personality_pragmatic: None,
|
||||||
}),
|
}),
|
||||||
|
approvals: None,
|
||||||
}));
|
}));
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
model.get_model_instructions(Some(Personality::Friendly)),
|
model.get_model_instructions(Some(Personality::Friendly)),
|
||||||
@@ -816,6 +856,7 @@ mod tests {
|
|||||||
personality_friendly: None,
|
personality_friendly: None,
|
||||||
personality_pragmatic: None,
|
personality_pragmatic: None,
|
||||||
}),
|
}),
|
||||||
|
approvals: None,
|
||||||
}));
|
}));
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
model_no_personality.get_model_instructions(Some(Personality::Friendly)),
|
model_no_personality.get_model_instructions(Some(Personality::Friendly)),
|
||||||
@@ -844,6 +885,7 @@ mod tests {
|
|||||||
personality_friendly: None,
|
personality_friendly: None,
|
||||||
personality_pragmatic: None,
|
personality_pragmatic: None,
|
||||||
}),
|
}),
|
||||||
|
approvals: 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