mirror of
https://github.com/openai/codex.git
synced 2026-10-16 16:58:46 +00:00
[0.146] Backport safer cyber-model auto-review defaults (#37057)
Backport cyber-model auto-review defaults to the 0.146 stable release line. Cherry-pick #37055. Omit precomputed schema artifacts that do not exist on release/0.146. Validation: git diff --check. --------- Co-authored-by: Won Park <won@openai.com>
This commit is contained in:
co-authored by
Won Park
parent
95637f7056
commit
7558bede75
+7
@@ -13285,6 +13285,13 @@
|
||||
"model": {
|
||||
"type": "string"
|
||||
},
|
||||
"modelSpecialty": {
|
||||
"default": null,
|
||||
"type": [
|
||||
"string",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"serviceTiers": {
|
||||
"default": [],
|
||||
"items": {
|
||||
|
||||
+7
@@ -9627,6 +9627,13 @@
|
||||
"model": {
|
||||
"type": "string"
|
||||
},
|
||||
"modelSpecialty": {
|
||||
"default": null,
|
||||
"type": [
|
||||
"string",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"serviceTiers": {
|
||||
"default": [],
|
||||
"items": {
|
||||
|
||||
@@ -86,6 +86,13 @@
|
||||
"model": {
|
||||
"type": "string"
|
||||
},
|
||||
"modelSpecialty": {
|
||||
"default": null,
|
||||
"type": [
|
||||
"string",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"serviceTiers": {
|
||||
"default": [],
|
||||
"items": {
|
||||
|
||||
+1
-1
@@ -8,7 +8,7 @@ import type { ModelServiceTier } from "./ModelServiceTier";
|
||||
import type { ModelUpgradeInfo } from "./ModelUpgradeInfo";
|
||||
import type { ReasoningEffortOption } from "./ReasoningEffortOption";
|
||||
|
||||
export type Model = { id: string, model: string, upgrade: string | null, upgradeInfo: ModelUpgradeInfo | null, availabilityNux: ModelAvailabilityNux | null, displayName: string, description: string, hidden: boolean, supportedReasoningEfforts: Array<ReasoningEffortOption>, defaultReasoningEffort: ReasoningEffort, inputModalities: Array<InputModality>, supportsPersonality: boolean,
|
||||
export type Model = { id: string, model: string, upgrade: string | null, upgradeInfo: ModelUpgradeInfo | null, availabilityNux: ModelAvailabilityNux | null, displayName: string, description: string, modelSpecialty: string | null, hidden: boolean, supportedReasoningEfforts: Array<ReasoningEffortOption>, defaultReasoningEffort: ReasoningEffort, inputModalities: Array<InputModality>, supportsPersonality: boolean,
|
||||
/**
|
||||
* Deprecated: use `serviceTiers` instead.
|
||||
*/
|
||||
|
||||
@@ -87,6 +87,8 @@ pub struct Model {
|
||||
pub availability_nux: Option<ModelAvailabilityNux>,
|
||||
pub display_name: String,
|
||||
pub description: String,
|
||||
#[serde(default)]
|
||||
pub model_specialty: Option<String>,
|
||||
pub hidden: bool,
|
||||
pub supported_reasoning_efforts: Vec<ReasoningEffortOption>,
|
||||
pub default_reasoning_effort: ReasoningEffort,
|
||||
|
||||
@@ -202,7 +202,7 @@ Example with notification opt-out:
|
||||
- `fs/watch` — subscribe this connection to filesystem change notifications for an absolute file or directory path and caller-provided `watchId`; returns the canonicalized `path`.
|
||||
- `fs/unwatch` — stop sending notifications for a prior `fs/watch`; returns `{}`.
|
||||
- `fs/changed` — notification emitted when watched paths change, including the `watchId` and `changedPaths`.
|
||||
- `model/list` — list available models (set `includeHidden: true` to include entries with `hidden: true`), with model-advertised string reasoning effort options in the catalog's intended progression order, `additionalSpeedTiers`, `serviceTiers`, optional `defaultServiceTier`, optional legacy `upgrade` model ids, optional `upgradeInfo` metadata (`model`, `upgradeCopy`, `modelLink`, `migrationMarkdown`), and optional `availabilityNux` metadata. Clients should preserve the `supportedReasoningEfforts` array order rather than deriving order from the effort names.
|
||||
- `model/list` — list available models (set `includeHidden: true` to include entries with `hidden: true`), with model-advertised string reasoning effort options in the catalog's intended progression order, optional `modelSpecialty`, `additionalSpeedTiers`, `serviceTiers`, optional `defaultServiceTier`, optional legacy `upgrade` model ids, optional `upgradeInfo` metadata (`model`, `upgradeCopy`, `modelLink`, `migrationMarkdown`), and optional `availabilityNux` metadata. Clients should preserve the `supportedReasoningEfforts` array order rather than deriving order from the effort names.
|
||||
- `modelProvider/capabilities/read` — read provider-level capabilities for the currently configured model provider.
|
||||
- `experimentalFeature/list` — list feature flags with stage metadata (`beta`, `underDevelopment`, `stable`, etc.), enabled/default-enabled state, and cursor pagination. Pass `threadId` when showing feature state for an existing loaded thread so `enabled` is computed from that thread's refreshed config, including project-local config for the thread's cwd; if omitted, the server uses its default config resolution context. For non-beta flags, `displayName`/`description`/`announcement` are `null`.
|
||||
- `permissionProfile/list` — beta; list available permission profile ids with optional display `description` text and an `allowed` flag reflecting effective requirements, using cursor pagination. Pass `cwd` when the caller needs project-local `[permissions.<id>]` entries to be included in the current catalog view.
|
||||
|
||||
@@ -38,6 +38,7 @@ fn model_from_preset(preset: ModelPreset) -> Model {
|
||||
availability_nux: preset.availability_nux.map(Into::into),
|
||||
display_name: preset.display_name.to_string(),
|
||||
description: preset.description.to_string(),
|
||||
model_specialty: preset.model_specialty,
|
||||
hidden: !preset.show_in_picker,
|
||||
supported_reasoning_efforts: reasoning_efforts_from_preset(
|
||||
preset.supported_reasoning_efforts,
|
||||
|
||||
@@ -56,6 +56,7 @@ fn preset_to_info(preset: &ModelPreset, priority: i32) -> ModelInfo {
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
auto_review_model_override: None,
|
||||
model_specialty: None,
|
||||
tool_mode: None,
|
||||
multi_agent_version: None,
|
||||
}
|
||||
|
||||
@@ -43,6 +43,7 @@ fn model_from_preset(preset: &ModelPreset) -> Model {
|
||||
availability_nux: preset.availability_nux.clone().map(Into::into),
|
||||
display_name: preset.display_name.clone(),
|
||||
description: preset.description.clone(),
|
||||
model_specialty: preset.model_specialty.clone(),
|
||||
hidden: !preset.show_in_picker,
|
||||
supported_reasoning_efforts: preset
|
||||
.supported_reasoning_efforts
|
||||
@@ -156,6 +157,7 @@ async fn list_models_uses_chatgpt_remote_catalog_as_source_of_truth() -> Result<
|
||||
"slug": "chatgpt-remote-only",
|
||||
"display_name": "ChatGPT Remote Only",
|
||||
"description": "Remote-only model for app-server model/list coverage",
|
||||
"model_specialty": "cyber",
|
||||
"default_reasoning_level": "max",
|
||||
"supported_reasoning_levels": [
|
||||
{"effort": "max", "description": "Maximum"},
|
||||
|
||||
@@ -102,6 +102,7 @@ async fn models_client_hits_models_endpoint() {
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
auto_review_model_override: None,
|
||||
model_specialty: None,
|
||||
tool_mode: None,
|
||||
multi_agent_version: None,
|
||||
}],
|
||||
|
||||
@@ -14,6 +14,7 @@ fn model_preset(id: &str, show_in_picker: bool) -> ModelPreset {
|
||||
model: format!("{id}-model"),
|
||||
display_name: format!("{id} display"),
|
||||
description: format!("{id} description"),
|
||||
model_specialty: None,
|
||||
default_reasoning_effort: ReasoningEffort::Medium,
|
||||
supported_reasoning_efforts: vec![ReasoningEffortPreset {
|
||||
effort: ReasoningEffort::Medium,
|
||||
|
||||
@@ -243,6 +243,7 @@ fn remote_model_with_auto_review_override(slug: &str, review_model: &str) -> Mod
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
auto_review_model_override: Some(review_model.to_string()),
|
||||
model_specialty: None,
|
||||
tool_mode: None,
|
||||
multi_agent_version: None,
|
||||
priority: 1,
|
||||
|
||||
@@ -90,6 +90,7 @@ fn test_model_info(
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
auto_review_model_override: None,
|
||||
model_specialty: None,
|
||||
tool_mode: None,
|
||||
multi_agent_version: None,
|
||||
priority: 1,
|
||||
@@ -954,6 +955,7 @@ async fn model_switch_to_smaller_model_updates_token_context_window() -> Result<
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
auto_review_model_override: None,
|
||||
model_specialty: None,
|
||||
tool_mode: None,
|
||||
multi_agent_version: None,
|
||||
priority: 1,
|
||||
|
||||
@@ -389,6 +389,7 @@ fn test_remote_model(slug: &str, priority: i32) -> ModelInfo {
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
auto_review_model_override: None,
|
||||
model_specialty: None,
|
||||
tool_mode: None,
|
||||
multi_agent_version: None,
|
||||
}
|
||||
|
||||
@@ -665,6 +665,7 @@ async fn remote_model_friendly_personality_instructions_with_feature() -> anyhow
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
auto_review_model_override: None,
|
||||
model_specialty: None,
|
||||
tool_mode: None,
|
||||
multi_agent_version: None,
|
||||
};
|
||||
@@ -784,6 +785,7 @@ async fn user_turn_personality_remote_model_template_includes_update_message() -
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
auto_review_model_override: None,
|
||||
model_specialty: None,
|
||||
tool_mode: None,
|
||||
multi_agent_version: None,
|
||||
};
|
||||
|
||||
@@ -480,6 +480,7 @@ async fn remote_models_remote_model_uses_unified_exec() -> Result<()> {
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
auto_review_model_override: None,
|
||||
model_specialty: None,
|
||||
tool_mode: None,
|
||||
multi_agent_version: None,
|
||||
priority: 1,
|
||||
@@ -734,6 +735,7 @@ async fn remote_models_apply_remote_base_instructions() -> Result<()> {
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
auto_review_model_override: None,
|
||||
model_specialty: None,
|
||||
tool_mode: None,
|
||||
multi_agent_version: None,
|
||||
priority: 1,
|
||||
@@ -1261,6 +1263,7 @@ fn test_remote_model_with_policy(
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
auto_review_model_override: None,
|
||||
model_specialty: None,
|
||||
tool_mode: None,
|
||||
multi_agent_version: None,
|
||||
priority,
|
||||
|
||||
@@ -1981,6 +1981,7 @@ async fn stdio_image_responses_are_sanitized_for_text_only_model() -> anyhow::Re
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
auto_review_model_override: None,
|
||||
model_specialty: None,
|
||||
tool_mode: None,
|
||||
multi_agent_version: None,
|
||||
}],
|
||||
|
||||
@@ -71,6 +71,7 @@ fn test_model_info(
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
auto_review_model_override: None,
|
||||
model_specialty: None,
|
||||
tool_mode: None,
|
||||
multi_agent_version: None,
|
||||
priority: 1,
|
||||
|
||||
@@ -1363,6 +1363,7 @@ async fn view_image_tool_returns_unsupported_message_for_text_only_model() -> an
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
auto_review_model_override: None,
|
||||
model_specialty: None,
|
||||
tool_mode: None,
|
||||
multi_agent_version: None,
|
||||
priority: 1,
|
||||
|
||||
@@ -162,6 +162,7 @@ pub fn model_info_from_slug(slug: &str) -> ModelInfo {
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
auto_review_model_override: None,
|
||||
model_specialty: None,
|
||||
tool_mode: None,
|
||||
multi_agent_version: None,
|
||||
}
|
||||
|
||||
@@ -210,6 +210,8 @@ pub struct ModelPreset {
|
||||
pub display_name: String,
|
||||
/// Short human description shown in UIs.
|
||||
pub description: String,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub model_specialty: Option<String>,
|
||||
/// Reasoning effort applied when none is explicitly chosen.
|
||||
pub default_reasoning_effort: ReasoningEffort,
|
||||
/// Supported reasoning effort options.
|
||||
@@ -437,6 +439,8 @@ pub struct ModelInfo {
|
||||
pub use_responses_lite: bool,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub auto_review_model_override: Option<String>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub model_specialty: Option<String>,
|
||||
#[serde(
|
||||
default,
|
||||
skip_serializing_if = "Option::is_none",
|
||||
@@ -612,6 +616,7 @@ impl From<ModelInfo> for ModelPreset {
|
||||
model: info.slug.clone(),
|
||||
display_name: info.display_name,
|
||||
description: info.description.unwrap_or_default(),
|
||||
model_specialty: info.model_specialty,
|
||||
default_reasoning_effort: info
|
||||
.default_reasoning_level
|
||||
.unwrap_or(ReasoningEffort::None),
|
||||
@@ -737,6 +742,7 @@ mod tests {
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
auto_review_model_override: None,
|
||||
model_specialty: None,
|
||||
tool_mode: None,
|
||||
multi_agent_version: None,
|
||||
}
|
||||
|
||||
@@ -48,6 +48,7 @@ fn model_with_shell_type(shell_type: ConfigShellToolType) -> ModelInfo {
|
||||
supports_search_tool: false,
|
||||
use_responses_lite: false,
|
||||
auto_review_model_override: None,
|
||||
model_specialty: None,
|
||||
tool_mode: None,
|
||||
multi_agent_version: None,
|
||||
}
|
||||
|
||||
@@ -1087,11 +1087,15 @@ impl App {
|
||||
.await;
|
||||
}
|
||||
AppEvent::UpdateModel(model) => {
|
||||
self.chat_widget.set_model(&model);
|
||||
self.sync_active_thread_model_setting(app_server, model)
|
||||
.await;
|
||||
self.sync_active_thread_service_tier_to_cached_session()
|
||||
.await;
|
||||
let model_changed = self.chat_widget.current_model() != model
|
||||
|| self.chat_widget.current_collaboration_mode().model() != model;
|
||||
if model_changed {
|
||||
self.chat_widget.set_model(&model);
|
||||
self.sync_active_thread_model_setting(app_server, model, /*effort*/ None)
|
||||
.await;
|
||||
self.sync_active_thread_service_tier_to_cached_session()
|
||||
.await;
|
||||
}
|
||||
}
|
||||
AppEvent::UpdatePersonality(personality) => {
|
||||
self.on_update_personality(personality);
|
||||
@@ -1115,12 +1119,22 @@ impl App {
|
||||
self.chat_widget.open_advanced_reasoning_popup(model);
|
||||
}
|
||||
AppEvent::ApplyAdvancedReasoning { model, effort } => {
|
||||
let model_changed = self.chat_widget.current_model() != model
|
||||
|| self.chat_widget.current_collaboration_mode().model() != model;
|
||||
let default_effort =
|
||||
self.on_apply_advanced_reasoning(model.as_str(), effort.clone());
|
||||
if let Some(mut params) =
|
||||
self.active_thread_model_setting_update_params(model.clone())
|
||||
if model_changed {
|
||||
self.sync_active_thread_model_setting(
|
||||
app_server,
|
||||
model.clone(),
|
||||
Some(effort.clone()),
|
||||
)
|
||||
.await;
|
||||
} else if let Some(mut params) =
|
||||
self.active_thread_reasoning_setting_update_params(Some(effort.clone()))
|
||||
{
|
||||
params.effort = Some(effort.clone());
|
||||
params.collaboration_mode =
|
||||
Some(self.chat_widget.effective_collaboration_mode());
|
||||
self.send_thread_settings_update(app_server, params).await;
|
||||
}
|
||||
self.sync_active_thread_service_tier_to_cached_session()
|
||||
@@ -1693,6 +1707,11 @@ impl App {
|
||||
}
|
||||
}
|
||||
}
|
||||
AppEvent::CyberModelAutoReviewNotice => {
|
||||
self.chat_widget.add_warning_message(
|
||||
"Cyber models default to \"Approve for me\" for safety reasons.".to_string(),
|
||||
);
|
||||
}
|
||||
AppEvent::PluginUninstallLoaded {
|
||||
cwd,
|
||||
plugin_id: _plugin_id,
|
||||
|
||||
@@ -259,6 +259,30 @@ fn bypass_hook_trust_startup_warning_snapshot() {
|
||||
|
||||
assert_app_snapshot!("bypass_hook_trust_startup_warning", rendered);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn cyber_model_auto_review_notice_snapshot() -> Result<()> {
|
||||
let (mut app, mut app_event_rx, _op_rx) = make_test_app_with_channels().await;
|
||||
let mut app_server =
|
||||
crate::start_embedded_app_server_for_picker(app.chat_widget.config_ref()).await?;
|
||||
let mut tui = crate::tui::test_support::make_test_tui()?;
|
||||
|
||||
app.handle_event(
|
||||
&mut tui,
|
||||
&mut app_server,
|
||||
AppEvent::CyberModelAutoReviewNotice,
|
||||
)
|
||||
.await?;
|
||||
|
||||
let cell = match app_event_rx.try_recv() {
|
||||
Ok(AppEvent::InsertHistoryCell(cell)) => cell,
|
||||
other => panic!("expected InsertHistoryCell event, got {other:?}"),
|
||||
};
|
||||
let rendered = lines_to_single_string(&cell.display_lines(/*width*/ 80));
|
||||
assert_app_snapshot!("cyber_model_auto_review_notice", rendered);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn enqueue_primary_thread_session_replays_buffered_approval_after_attach() -> Result<()> {
|
||||
let (mut app, mut app_event_rx, _op_rx) = make_test_app_with_channels().await;
|
||||
@@ -7108,6 +7132,291 @@ async fn override_turn_context_sends_thread_settings_update() {
|
||||
.await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn selecting_cyber_model_defaults_active_thread_to_auto_review() {
|
||||
Box::pin(async {
|
||||
let mut app = make_test_app().await;
|
||||
app.config
|
||||
.permissions
|
||||
.approval_policy
|
||||
.set(AskForApproval::UnlessTrusted.to_core())
|
||||
.expect("set approval policy");
|
||||
app.chat_widget
|
||||
.set_approval_policy(AskForApproval::UnlessTrusted);
|
||||
let mut model = app
|
||||
.model_catalog
|
||||
.try_list_models()
|
||||
.expect("model catalog")
|
||||
.into_iter()
|
||||
.find(|model| model.model == "gpt-5.4")
|
||||
.expect("gpt-5.4 model");
|
||||
model.model_specialty = Some("cyber".to_string());
|
||||
app.model_catalog = Arc::new(ModelCatalog::new(vec![model]));
|
||||
|
||||
let mut app_server =
|
||||
crate::start_embedded_app_server_for_picker(app.chat_widget.config_ref())
|
||||
.await
|
||||
.expect("embedded app server");
|
||||
let started = app_server
|
||||
.start_thread(app.chat_widget.config_ref())
|
||||
.await
|
||||
.expect("thread/start should succeed");
|
||||
assert_eq!(
|
||||
started.session.approval_policy,
|
||||
AskForApproval::UnlessTrusted
|
||||
);
|
||||
assert_eq!(started.session.approvals_reviewer, ApprovalsReviewer::User);
|
||||
let thread_id = started.session.thread_id;
|
||||
app.enqueue_primary_thread_session(started.session, started.turns)
|
||||
.await
|
||||
.expect("primary thread should be registered");
|
||||
|
||||
let mut tui = crate::tui::test_support::make_test_tui().expect("test tui");
|
||||
app.handle_event(
|
||||
&mut tui,
|
||||
&mut app_server,
|
||||
AppEvent::UpdateModel("gpt-5.4".to_string()),
|
||||
)
|
||||
.await
|
||||
.expect("model selection should succeed");
|
||||
|
||||
let notification = next_thread_settings_updated(&mut app_server, thread_id).await;
|
||||
assert_eq!(
|
||||
notification.thread_settings.approval_policy,
|
||||
AskForApproval::OnRequest
|
||||
);
|
||||
assert_eq!(
|
||||
notification.thread_settings.approvals_reviewer.to_core(),
|
||||
ApprovalsReviewer::AutoReview
|
||||
);
|
||||
assert_eq!(
|
||||
notification
|
||||
.thread_settings
|
||||
.active_permission_profile
|
||||
.expect("active permission profile")
|
||||
.id,
|
||||
codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_WORKSPACE
|
||||
);
|
||||
})
|
||||
.await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn changing_cyber_model_reasoning_preserves_selected_permissions() {
|
||||
Box::pin(async {
|
||||
let mut app = make_test_app().await;
|
||||
let model_name = app.chat_widget.current_model().to_string();
|
||||
let mut model = app
|
||||
.model_catalog
|
||||
.try_list_models()
|
||||
.expect("model catalog")
|
||||
.into_iter()
|
||||
.find(|model| model.model == model_name)
|
||||
.expect("current model");
|
||||
model.model_specialty = Some("cyber".to_string());
|
||||
app.model_catalog = Arc::new(ModelCatalog::new(vec![model]));
|
||||
|
||||
assert!(
|
||||
app.apply_permission_profile_selection(PermissionProfileSelection {
|
||||
profile_id: codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_READ_ONLY
|
||||
.to_string(),
|
||||
approval_policy: Some(AskForApproval::OnRequest),
|
||||
approvals_reviewer: Some(ApprovalsReviewer::User),
|
||||
display_label: "Read Only".to_string(),
|
||||
})
|
||||
.await
|
||||
);
|
||||
|
||||
let mut app_server =
|
||||
crate::start_embedded_app_server_for_picker(app.chat_widget.config_ref())
|
||||
.await
|
||||
.expect("embedded app server");
|
||||
let started = app_server
|
||||
.start_thread(app.chat_widget.config_ref())
|
||||
.await
|
||||
.expect("thread/start should succeed");
|
||||
let thread_id = started.session.thread_id;
|
||||
app.enqueue_primary_thread_session(started.session, started.turns)
|
||||
.await
|
||||
.expect("primary thread should be registered");
|
||||
|
||||
let mut tui = crate::tui::test_support::make_test_tui().expect("test tui");
|
||||
for effort in [ReasoningEffortConfig::High, ReasoningEffortConfig::Ultra] {
|
||||
if effort == ReasoningEffortConfig::Ultra {
|
||||
app.chat_widget
|
||||
.set_feature_enabled(Feature::CollaborationModes, /*enabled*/ true);
|
||||
app.chat_widget
|
||||
.set_collaboration_mask(CollaborationModeMask {
|
||||
name: "Plan".to_string(),
|
||||
mode: Some(ModeKind::Plan),
|
||||
model: Some(model_name.clone()),
|
||||
reasoning_effort: Some(Some(effort.clone())),
|
||||
developer_instructions: None,
|
||||
});
|
||||
app.handle_event(
|
||||
&mut tui,
|
||||
&mut app_server,
|
||||
AppEvent::ApplyAdvancedReasoning {
|
||||
model: model_name.clone(),
|
||||
effort: effort.clone(),
|
||||
},
|
||||
)
|
||||
.await
|
||||
.expect("advanced reasoning selection should succeed");
|
||||
} else {
|
||||
app.handle_event(
|
||||
&mut tui,
|
||||
&mut app_server,
|
||||
AppEvent::UpdateModel(model_name.clone()),
|
||||
)
|
||||
.await
|
||||
.expect("same-model selection should succeed");
|
||||
app.handle_event(
|
||||
&mut tui,
|
||||
&mut app_server,
|
||||
AppEvent::UpdateReasoningEffort(Some(effort.clone())),
|
||||
)
|
||||
.await
|
||||
.expect("reasoning selection should succeed");
|
||||
}
|
||||
|
||||
let settings = next_thread_settings_updated(&mut app_server, thread_id)
|
||||
.await
|
||||
.thread_settings;
|
||||
assert_eq!(settings.effort, Some(effort));
|
||||
assert_eq!(settings.approval_policy, AskForApproval::OnRequest);
|
||||
assert_eq!(
|
||||
settings.approvals_reviewer.to_core(),
|
||||
ApprovalsReviewer::User
|
||||
);
|
||||
assert_eq!(
|
||||
settings
|
||||
.active_permission_profile
|
||||
.expect("active permission profile")
|
||||
.id,
|
||||
codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_READ_ONLY
|
||||
);
|
||||
assert_eq!(
|
||||
settings.collaboration_mode.mode,
|
||||
app.chat_widget.effective_collaboration_mode().mode
|
||||
);
|
||||
assert_eq!(settings.collaboration_mode.settings.model, model_name);
|
||||
}
|
||||
})
|
||||
.await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn selecting_cyber_model_falls_back_to_user_when_auto_review_is_unavailable() {
|
||||
let mut app = make_test_app().await;
|
||||
let mut model = app
|
||||
.model_catalog
|
||||
.try_list_models()
|
||||
.expect("model catalog")
|
||||
.into_iter()
|
||||
.find(|model| model.model == "gpt-5.4")
|
||||
.expect("gpt-5.4 model");
|
||||
model.model_specialty = Some("cyber".to_string());
|
||||
app.model_catalog = Arc::new(ModelCatalog::new(vec![model]));
|
||||
let _ = app.config.features.disable(Feature::GuardianApproval);
|
||||
app.chat_widget
|
||||
.set_feature_enabled(Feature::GuardianApproval, /*enabled*/ false);
|
||||
app.active_thread_id = Some(ThreadId::new());
|
||||
|
||||
let params = app
|
||||
.active_thread_model_setting_update_params("gpt-5.4".to_string())
|
||||
.expect("active thread should produce update params");
|
||||
|
||||
assert_eq!(
|
||||
params.approval_policy,
|
||||
Some(codex_app_server_protocol::AskForApproval::OnRequest)
|
||||
);
|
||||
assert_eq!(
|
||||
params.approvals_reviewer,
|
||||
Some(codex_app_server_protocol::ApprovalsReviewer::User)
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn selecting_cyber_model_respects_auto_review_requirements() {
|
||||
Box::pin(async {
|
||||
let (mut app, mut app_event_rx, _op_rx) = make_test_app_with_channels().await;
|
||||
let requirements_toml = codex_config::ConfigRequirementsToml {
|
||||
allowed_approval_policies: Some(vec![AskForApproval::UnlessTrusted.to_core()]),
|
||||
..Default::default()
|
||||
};
|
||||
let mut requirements_with_sources = codex_config::ConfigRequirementsWithSources::default();
|
||||
requirements_with_sources.merge_unset_fields(
|
||||
codex_config::RequirementSource::Unknown,
|
||||
requirements_toml.clone(),
|
||||
);
|
||||
let requirements = codex_config::ConfigRequirements::try_from(requirements_with_sources)
|
||||
.expect("reviewer requirements");
|
||||
app.config.config_layer_stack =
|
||||
codex_config::ConfigLayerStack::new(Vec::new(), requirements, requirements_toml)
|
||||
.expect("auto-review requirements stack");
|
||||
app.config
|
||||
.permissions
|
||||
.approval_policy
|
||||
.set(AskForApproval::UnlessTrusted.to_core())
|
||||
.expect("set approval policy");
|
||||
app.chat_widget
|
||||
.set_approval_policy(AskForApproval::UnlessTrusted);
|
||||
app.chat_widget.sync_plugin_mentions_config(&app.config);
|
||||
|
||||
let mut model = app
|
||||
.model_catalog
|
||||
.try_list_models()
|
||||
.expect("model catalog")
|
||||
.into_iter()
|
||||
.find(|model| model.model == "gpt-5.4")
|
||||
.expect("gpt-5.4 model");
|
||||
model.model_specialty = Some("cyber".to_string());
|
||||
app.model_catalog = Arc::new(ModelCatalog::new(vec![model]));
|
||||
|
||||
let mut app_server =
|
||||
crate::start_embedded_app_server_for_picker(app.chat_widget.config_ref())
|
||||
.await
|
||||
.expect("embedded app server");
|
||||
let started = app_server
|
||||
.start_thread(app.chat_widget.config_ref())
|
||||
.await
|
||||
.expect("thread/start should succeed");
|
||||
assert_eq!(
|
||||
started.session.approval_policy,
|
||||
AskForApproval::UnlessTrusted
|
||||
);
|
||||
let thread_id = started.session.thread_id;
|
||||
app.enqueue_primary_thread_session(started.session, started.turns)
|
||||
.await
|
||||
.expect("primary thread should be registered");
|
||||
|
||||
let mut tui = crate::tui::test_support::make_test_tui().expect("test tui");
|
||||
app.handle_event(
|
||||
&mut tui,
|
||||
&mut app_server,
|
||||
AppEvent::UpdateModel("gpt-5.4".to_string()),
|
||||
)
|
||||
.await
|
||||
.expect("model selection should succeed");
|
||||
|
||||
let notification = next_thread_settings_updated(&mut app_server, thread_id).await;
|
||||
assert_eq!(
|
||||
notification.thread_settings.approval_policy,
|
||||
AskForApproval::UnlessTrusted
|
||||
);
|
||||
assert_eq!(
|
||||
notification.thread_settings.approvals_reviewer.to_core(),
|
||||
ApprovalsReviewer::User
|
||||
);
|
||||
assert!(
|
||||
std::iter::from_fn(|| app_event_rx.try_recv().ok())
|
||||
.all(|event| !matches!(event, AppEvent::CyberModelAutoReviewNotice))
|
||||
);
|
||||
})
|
||||
.await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn thread_setting_update_params_sync_model_and_default_reasoning() {
|
||||
let mut app = make_test_app().await;
|
||||
|
||||
@@ -2,13 +2,18 @@
|
||||
|
||||
use super::App;
|
||||
use crate::app_command::AppCommand;
|
||||
use crate::app_event::AppEvent;
|
||||
use crate::app_server_session::AppServerSession;
|
||||
use crate::chatwidget::cyber_model_approval_reviewer;
|
||||
use crate::session_state::ThreadSessionState;
|
||||
use codex_app_server_protocol::ApprovalsReviewer as AppServerApprovalsReviewer;
|
||||
use codex_app_server_protocol::AskForApproval as AppServerAskForApproval;
|
||||
use codex_app_server_protocol::ThreadSettings;
|
||||
use codex_app_server_protocol::ThreadSettingsUpdateParams;
|
||||
use codex_config::types::ApprovalsReviewer;
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::config_types::ModeKind;
|
||||
use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_WORKSPACE;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
|
||||
impl App {
|
||||
@@ -16,11 +21,26 @@ impl App {
|
||||
&mut self,
|
||||
app_server: &mut AppServerSession,
|
||||
model: String,
|
||||
effort: Option<codex_protocol::openai_models::ReasoningEffort>,
|
||||
) {
|
||||
let Some(params) = self.active_thread_model_setting_update_params(model) else {
|
||||
let Some(mut params) = self.active_thread_model_setting_update_params(model) else {
|
||||
return;
|
||||
};
|
||||
self.send_thread_settings_update(app_server, params).await;
|
||||
params.effort = effort;
|
||||
let defaulted_to_auto_review = params.approvals_reviewer
|
||||
== Some(AppServerApprovalsReviewer::AutoReview)
|
||||
&& (self.chat_widget.config_ref().approvals_reviewer != ApprovalsReviewer::AutoReview
|
||||
|| AppServerAskForApproval::from(
|
||||
self.chat_widget
|
||||
.config_ref()
|
||||
.permissions
|
||||
.approval_policy
|
||||
.value(),
|
||||
) != AppServerAskForApproval::OnRequest);
|
||||
let settings_updated = self.send_thread_settings_update(app_server, params).await;
|
||||
if defaulted_to_auto_review && settings_updated {
|
||||
self.app_event_tx.send(AppEvent::CyberModelAutoReviewNotice);
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) fn active_thread_model_setting_update_params(
|
||||
@@ -28,12 +48,40 @@ impl App {
|
||||
model: String,
|
||||
) -> Option<ThreadSettingsUpdateParams> {
|
||||
let thread_id = self.active_thread_id?;
|
||||
Some(ThreadSettingsUpdateParams {
|
||||
let is_cyber_model = self.model_catalog.try_list_models().is_ok_and(|models| {
|
||||
models.iter().any(|preset| {
|
||||
preset.model == model && preset.model_specialty.as_deref() == Some("cyber")
|
||||
})
|
||||
});
|
||||
|
||||
let mut params = ThreadSettingsUpdateParams {
|
||||
thread_id: thread_id.to_string(),
|
||||
model: Some(model),
|
||||
collaboration_mode: Some(self.chat_widget.effective_collaboration_mode()),
|
||||
..ThreadSettingsUpdateParams::default()
|
||||
})
|
||||
};
|
||||
|
||||
if is_cyber_model {
|
||||
let workspace_profile = PermissionProfile::workspace_write();
|
||||
let workspace_allowed = self
|
||||
.config
|
||||
.permissions
|
||||
.can_set_permission_profile(&workspace_profile)
|
||||
.is_ok()
|
||||
&& self.config.is_permission_profile_allowed(
|
||||
BUILT_IN_PERMISSION_PROFILE_WORKSPACE,
|
||||
&workspace_profile,
|
||||
);
|
||||
|
||||
if workspace_allowed && let Some(reviewer) = cyber_model_approval_reviewer(&self.config)
|
||||
{
|
||||
params.permissions = Some(BUILT_IN_PERMISSION_PROFILE_WORKSPACE.to_string());
|
||||
params.approval_policy = Some(AppServerAskForApproval::OnRequest);
|
||||
params.approvals_reviewer = Some(reviewer.into());
|
||||
}
|
||||
}
|
||||
|
||||
Some(params)
|
||||
}
|
||||
|
||||
pub(super) async fn sync_active_thread_reasoning_setting(
|
||||
@@ -157,14 +205,18 @@ impl App {
|
||||
&mut self,
|
||||
app_server: &mut AppServerSession,
|
||||
params: ThreadSettingsUpdateParams,
|
||||
) {
|
||||
) -> bool {
|
||||
if !thread_settings_update_has_changes(¶ms) {
|
||||
return;
|
||||
return false;
|
||||
}
|
||||
if let Err(err) = app_server.thread_settings_update(params).await {
|
||||
tracing::warn!("failed to update app-server thread settings from TUI: {err}");
|
||||
self.chat_widget
|
||||
.add_error_message(format!("Failed to update thread settings: {err}"));
|
||||
match app_server.thread_settings_update(params).await {
|
||||
Ok(settings_updated) => settings_updated,
|
||||
Err(err) => {
|
||||
tracing::warn!("failed to update app-server thread settings from TUI: {err}");
|
||||
self.chat_widget
|
||||
.add_error_message(format!("Failed to update thread settings: {err}"));
|
||||
false
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -775,6 +775,9 @@ pub(crate) enum AppEvent {
|
||||
effort: Option<ReasoningEffort>,
|
||||
},
|
||||
|
||||
/// Show the cyber auto-review notice after the model selection confirmation.
|
||||
CyberModelAutoReviewNotice,
|
||||
|
||||
/// Persist the selected personality to the appropriate config.
|
||||
PersistPersonalitySelection {
|
||||
personality: Personality,
|
||||
|
||||
@@ -838,9 +838,9 @@ impl AppServerSession {
|
||||
pub(crate) async fn thread_settings_update(
|
||||
&mut self,
|
||||
params: ThreadSettingsUpdateParams,
|
||||
) -> Result<()> {
|
||||
) -> Result<bool> {
|
||||
if !self.thread_settings_update_supported {
|
||||
return Ok(());
|
||||
return Ok(false);
|
||||
}
|
||||
let request_id = self.next_request_id();
|
||||
match self
|
||||
@@ -851,7 +851,7 @@ impl AppServerSession {
|
||||
})
|
||||
.await
|
||||
{
|
||||
Ok(_) => Ok(()),
|
||||
Ok(_) => Ok(true),
|
||||
Err(TypedRequestError::Server { source, .. })
|
||||
if is_thread_settings_update_unsupported(&source) =>
|
||||
{
|
||||
@@ -862,7 +862,7 @@ impl AppServerSession {
|
||||
// of showing an error every time the user changes model, effort,
|
||||
// personality, or mode.
|
||||
self.thread_settings_update_supported = false;
|
||||
Ok(())
|
||||
Ok(false)
|
||||
}
|
||||
Err(err) => Err(err).wrap_err("thread/settings/update failed in TUI"),
|
||||
}
|
||||
@@ -1338,6 +1338,7 @@ fn model_preset_from_api_model(model: ApiModel) -> ModelPreset {
|
||||
model: model.model,
|
||||
display_name: model.display_name,
|
||||
description: model.description,
|
||||
model_specialty: model.model_specialty,
|
||||
default_reasoning_effort: model.default_reasoning_effort,
|
||||
supported_reasoning_efforts: model
|
||||
.supported_reasoning_efforts
|
||||
|
||||
@@ -169,7 +169,6 @@ use crossterm::event::KeyModifiers;
|
||||
use rand::Rng;
|
||||
use ratatui::buffer::Buffer;
|
||||
use ratatui::layout::Rect;
|
||||
use ratatui::style::Color;
|
||||
use ratatui::style::Modifier;
|
||||
use ratatui::style::Style;
|
||||
use ratatui::style::Stylize;
|
||||
@@ -380,6 +379,8 @@ mod notifications;
|
||||
use self::notifications::Notification;
|
||||
mod permission_popups;
|
||||
mod permissions_menu;
|
||||
pub(crate) use self::permissions_menu::auto_review_available;
|
||||
pub(crate) use self::permissions_menu::cyber_model_approval_reviewer;
|
||||
mod protocol;
|
||||
mod protocol_requests;
|
||||
mod rate_limits;
|
||||
|
||||
@@ -407,19 +407,45 @@ impl ChatWidget {
|
||||
) {
|
||||
let selected_name = preset.label.to_string();
|
||||
let approval = AskForApproval::from(preset.approval);
|
||||
let mut header_children: Vec<Box<dyn Renderable>> = Vec::new();
|
||||
let is_cyber_model = self.model_catalog.try_list_models().is_ok_and(|models| {
|
||||
models.iter().any(|model| {
|
||||
model.model == self.current_model()
|
||||
&& model.model_specialty.as_deref() == Some("cyber")
|
||||
})
|
||||
});
|
||||
let title_line = Line::from("Enable full access?").bold();
|
||||
let info_line = Line::from(vec![
|
||||
"When Codex runs with full access, it can edit any file on your computer and run commands with network, without your approval. "
|
||||
.into(),
|
||||
"Exercise caution when enabling full access. This significantly increases the risk of data loss, leaks, or unexpected behavior."
|
||||
.fg(Color::Red),
|
||||
]);
|
||||
header_children.push(Box::new(title_line));
|
||||
header_children.push(Box::new(
|
||||
Paragraph::new(vec![info_line]).wrap(Wrap { trim: false }),
|
||||
));
|
||||
let header = ColumnRenderable::with(header_children);
|
||||
let info_lines = if is_cyber_model {
|
||||
let recommendation = if auto_review_available(&self.config) {
|
||||
"We strongly recommend selecting \"Approve for me\" instead, and customizing the reviewer policy for your use case."
|
||||
} else {
|
||||
"We strongly recommend selecting \"Ask for approval\" instead."
|
||||
};
|
||||
vec![
|
||||
Line::default(),
|
||||
Line::from(
|
||||
"When Codex runs with full access, it can edit any file on your computer and run commands with network, without your approval.",
|
||||
),
|
||||
Line::default(),
|
||||
Line::from(vec![
|
||||
"Cyber models carry a higher risk of dangerous actions.".red(),
|
||||
" Ensure proper safeguards are in place before granting full access. ".into(),
|
||||
recommendation.into(),
|
||||
]),
|
||||
]
|
||||
} else {
|
||||
vec![Line::from(vec![
|
||||
"When Codex runs with full access, it can edit any file on your computer and run commands with network, without your approval. "
|
||||
.into(),
|
||||
"Exercise caution when enabling full access. This significantly increases the risk of data loss, leaks, or unexpected behavior."
|
||||
.red(),
|
||||
])]
|
||||
};
|
||||
let header = Paragraph::new(
|
||||
std::iter::once(title_line)
|
||||
.chain(info_lines)
|
||||
.collect::<Vec<_>>(),
|
||||
)
|
||||
.wrap(Wrap { trim: false });
|
||||
|
||||
let accept_actions = profile_selection.map_or_else(
|
||||
|| {
|
||||
|
||||
@@ -1,6 +1,37 @@
|
||||
use super::*;
|
||||
use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_DANGER_FULL_ACCESS;
|
||||
|
||||
pub(crate) fn auto_review_available(config: &Config) -> bool {
|
||||
cyber_model_approval_reviewer(config) == Some(ApprovalsReviewer::AutoReview)
|
||||
}
|
||||
|
||||
pub(crate) fn cyber_model_approval_reviewer(config: &Config) -> Option<ApprovalsReviewer> {
|
||||
let requirements = config.config_layer_stack.requirements();
|
||||
if requirements
|
||||
.approval_policy
|
||||
.can_set(&AskForApproval::OnRequest.to_core())
|
||||
.is_err()
|
||||
{
|
||||
return None;
|
||||
}
|
||||
|
||||
let reviewer = if config.features.enabled(Feature::GuardianApproval)
|
||||
&& requirements
|
||||
.approvals_reviewer
|
||||
.can_set(&ApprovalsReviewer::AutoReview)
|
||||
.is_ok()
|
||||
{
|
||||
ApprovalsReviewer::AutoReview
|
||||
} else {
|
||||
ApprovalsReviewer::User
|
||||
};
|
||||
requirements
|
||||
.approvals_reviewer
|
||||
.can_set(&reviewer)
|
||||
.is_ok()
|
||||
.then_some(reviewer)
|
||||
}
|
||||
|
||||
impl ChatWidget {
|
||||
pub(super) fn open_permission_profiles_popup(&mut self) {
|
||||
let active_profile_id = self
|
||||
|
||||
@@ -3121,6 +3121,7 @@ async fn model_picker_hides_show_in_picker_false_models_from_cache() {
|
||||
model: slug.to_string(),
|
||||
display_name: slug.to_string(),
|
||||
description: format!("{slug} description"),
|
||||
model_specialty: None,
|
||||
default_reasoning_effort: ReasoningEffortConfig::Medium,
|
||||
supported_reasoning_efforts: vec![ReasoningEffortPreset {
|
||||
effort: ReasoningEffortConfig::Medium,
|
||||
@@ -3615,6 +3616,7 @@ async fn single_reasoning_option_skips_selection() {
|
||||
model: "model-with-single-reasoning".to_string(),
|
||||
display_name: "model-with-single-reasoning".to_string(),
|
||||
description: "".to_string(),
|
||||
model_specialty: None,
|
||||
default_reasoning_effort: ReasoningEffortConfig::High,
|
||||
supported_reasoning_efforts: single_effort,
|
||||
supports_personality: false,
|
||||
|
||||
@@ -106,8 +106,7 @@ impl ChatWidget {
|
||||
Line::from(vec![
|
||||
"We couldn't complete the world-writable scan, so protections cannot be verified. "
|
||||
.into(),
|
||||
format!("The Windows sandbox cannot guarantee protection in {mode_label}.")
|
||||
.fg(Color::Red),
|
||||
format!("The Windows sandbox cannot guarantee protection in {mode_label}.").red(),
|
||||
])
|
||||
} else {
|
||||
Line::from(vec![
|
||||
|
||||
@@ -0,0 +1,5 @@
|
||||
---
|
||||
source: tui/src/app/tests.rs
|
||||
expression: rendered
|
||||
---
|
||||
⚠ Cyber models default to "Approve for me" for safety reasons.
|
||||
@@ -2,7 +2,6 @@ load("//:defs.bzl", "codex_rust_crate")
|
||||
|
||||
codex_rust_crate(
|
||||
name = "windows-sandbox-rs",
|
||||
binary_test_target_compatible_with = ["@platforms//os:windows"],
|
||||
build_script_data = [
|
||||
"codex-windows-sandbox-setup.manifest",
|
||||
],
|
||||
|
||||
Reference in New Issue
Block a user