mirror of
https://github.com/openai/codex.git
synced 2026-11-02 16:20:35 +00:00
[Codex][CLI] Gate image inputs by model modalities (#10271)
###### Summary - Add input_modalities to model metadata so clients can determine supported input types. - Gate image paste/attach in TUI when the selected model does not support images. - Block submits that include images for unsupported models and show a clear warning. - Propagate modality metadata through app-server protocol/model-list responses. - Update related tests/fixtures. ###### Rationale - Models support different input modalities. - Clients need an explicit capability signal to prevent unsupported requests. - Backward-compatible defaults preserve existing behavior when modality metadata is absent. ###### Scope - codex-rs/protocol, codex-rs/core, codex-rs/tui - codex-rs/app-server-protocol, codex-rs/app-server - Generated app-server types / schema fixtures ###### Trade-offs - Default behavior assumes text + image when field is absent for compatibility. - Server-side validation remains the source of truth. ###### Follow-up - Non-TUI clients should consume input_modalities to disable unsupported attachments. - Model catalogs should explicitly set input_modalities for text-only models. ###### Testing - cargo fmt --all - cargo test -p codex-tui - env -u GITHUB_APP_KEY cargo test -p codex-core --lib - just write-app-server-schema - cargo run -p codex-cli --bin codex -- app-server generate-ts --out app-server-types - test against local backend <img width="695" height="199" alt="image" src="https://github.com/user-attachments/assets/d22dd04f-5eba-4db9-a7c5-a2506f60ec44" /> --------- Co-authored-by: Josh McKinney <joshka@openai.com>
This commit is contained in:
1 parent
b8addcddb9
commit
7e07ec8f73
23 files changed
+373
-3
No files matched your search
@@ -394,6 +394,14 @@ impl ChatComposer {
|
||||
self.skills = skills;
|
||||
}
|
||||
|
||||
/// Toggle composer-side image paste handling.
|
||||
///
|
||||
/// This only affects whether image-like paste content is converted into attachments; the
|
||||
/// `ChatWidget` layer still performs capability checks before images are submitted.
|
||||
pub fn set_image_paste_enabled(&mut self, enabled: bool) {
|
||||
self.config.image_paste_enabled = enabled;
|
||||
}
|
||||
|
||||
pub fn set_connector_mentions(&mut self, connectors_snapshot: Option<ConnectorsSnapshot>) {
|
||||
self.connectors_snapshot = connectors_snapshot;
|
||||
}
|
||||
@@ -712,18 +720,45 @@ impl ChatComposer {
|
||||
}
|
||||
|
||||
/// Replace the entire composer content with `text` and reset cursor.
|
||||
/// This clears any pending paste payloads.
|
||||
///
|
||||
/// This is the "fresh draft" path: it clears pending paste payloads and
|
||||
/// mention link targets. Callers restoring a previously submitted draft
|
||||
/// that must keep `$name -> path` resolution should use
|
||||
/// [`Self::set_text_content_with_mention_paths`] instead.
|
||||
pub(crate) fn set_text_content(
|
||||
&mut self,
|
||||
text: String,
|
||||
text_elements: Vec<TextElement>,
|
||||
local_image_paths: Vec<PathBuf>,
|
||||
) {
|
||||
self.set_text_content_with_mention_paths(
|
||||
text,
|
||||
text_elements,
|
||||
local_image_paths,
|
||||
HashMap::new(),
|
||||
);
|
||||
}
|
||||
|
||||
/// Replace the entire composer content while restoring mention link targets.
|
||||
///
|
||||
/// Mention popup insertion stores both visible text (for example `$file`)
|
||||
/// and hidden `mention_paths` used to resolve the canonical target during
|
||||
/// submission. Use this method when restoring an interrupted or blocked
|
||||
/// draft; if callers restore only text and images, mentions can appear
|
||||
/// intact to users while resolving to the wrong target or dropping on
|
||||
/// retry.
|
||||
pub(crate) fn set_text_content_with_mention_paths(
|
||||
&mut self,
|
||||
text: String,
|
||||
text_elements: Vec<TextElement>,
|
||||
local_image_paths: Vec<PathBuf>,
|
||||
mention_paths: HashMap<String, String>,
|
||||
) {
|
||||
// Clear any existing content, placeholders, and attachments first.
|
||||
self.textarea.set_text_clearing_elements("");
|
||||
self.pending_pastes.clear();
|
||||
self.attached_images.clear();
|
||||
self.mention_paths.clear();
|
||||
self.mention_paths = mention_paths;
|
||||
|
||||
self.textarea.set_text_with_elements(&text, &text_elements);
|
||||
|
||||
|
||||
@@ -209,6 +209,14 @@ impl BottomPane {
|
||||
self.request_redraw();
|
||||
}
|
||||
|
||||
/// Update image-paste behavior for the active composer and repaint immediately.
|
||||
///
|
||||
/// Callers use this to keep composer affordances aligned with model capabilities.
|
||||
pub fn set_image_paste_enabled(&mut self, enabled: bool) {
|
||||
self.composer.set_image_paste_enabled(enabled);
|
||||
self.request_redraw();
|
||||
}
|
||||
|
||||
pub fn set_connectors_snapshot(&mut self, snapshot: Option<ConnectorsSnapshot>) {
|
||||
self.composer.set_connector_mentions(snapshot);
|
||||
self.request_redraw();
|
||||
@@ -402,6 +410,10 @@ impl BottomPane {
|
||||
}
|
||||
|
||||
/// Replace the composer text with `text`.
|
||||
///
|
||||
/// This is intended for fresh input where mention linkage does not need to
|
||||
/// survive; it routes to `ChatComposer::set_text_content`, which resets
|
||||
/// `mention_paths`.
|
||||
pub(crate) fn set_composer_text(
|
||||
&mut self,
|
||||
text: String,
|
||||
@@ -414,6 +426,27 @@ impl BottomPane {
|
||||
self.request_redraw();
|
||||
}
|
||||
|
||||
/// Replace the composer text while preserving mention link targets.
|
||||
///
|
||||
/// Use this when rehydrating a draft after a local validation/gating
|
||||
/// failure (for example unsupported image submit) so previously selected
|
||||
/// mention targets remain stable across retry.
|
||||
pub(crate) fn set_composer_text_with_mention_paths(
|
||||
&mut self,
|
||||
text: String,
|
||||
text_elements: Vec<TextElement>,
|
||||
local_image_paths: Vec<PathBuf>,
|
||||
mention_paths: HashMap<String, String>,
|
||||
) {
|
||||
self.composer.set_text_content_with_mention_paths(
|
||||
text,
|
||||
text_elements,
|
||||
local_image_paths,
|
||||
mention_paths,
|
||||
);
|
||||
self.request_redraw();
|
||||
}
|
||||
|
||||
#[allow(dead_code)]
|
||||
pub(crate) fn set_composer_input_enabled(
|
||||
&mut self,
|
||||
|
||||
@@ -208,6 +208,7 @@ use codex_core::ThreadManager;
|
||||
use codex_core::protocol::AskForApproval;
|
||||
use codex_core::protocol::SandboxPolicy;
|
||||
use codex_file_search::FileMatch;
|
||||
use codex_protocol::openai_models::InputModality;
|
||||
use codex_protocol::openai_models::ModelPreset;
|
||||
use codex_protocol::openai_models::ReasoningEffort as ReasoningEffortConfig;
|
||||
use codex_protocol::plan_tool::UpdatePlanArgs;
|
||||
@@ -2748,7 +2749,18 @@ impl ChatWidget {
|
||||
}
|
||||
}
|
||||
|
||||
/// Attach a local image to the composer when the active model supports image inputs.
|
||||
///
|
||||
/// When the model does not advertise image support, we keep the draft unchanged and surface a
|
||||
/// warning event so users can switch models or remove attachments.
|
||||
pub(crate) fn attach_image(&mut self, path: PathBuf) {
|
||||
if !self.current_model_supports_images() {
|
||||
self.add_to_history(history_cell::new_warning_event(
|
||||
self.image_inputs_not_supported_message(),
|
||||
));
|
||||
self.request_redraw();
|
||||
return;
|
||||
}
|
||||
tracing::info!("attach_image path={path:?}");
|
||||
self.bottom_pane.attach_image(path);
|
||||
self.request_redraw();
|
||||
@@ -3225,6 +3237,10 @@ impl ChatWidget {
|
||||
if text.is_empty() && local_images.is_empty() {
|
||||
return;
|
||||
}
|
||||
if !local_images.is_empty() && !self.current_model_supports_images() {
|
||||
self.restore_blocked_image_submission(text, text_elements, local_images, mention_paths);
|
||||
return;
|
||||
}
|
||||
|
||||
let mut items: Vec<UserInput> = Vec::new();
|
||||
|
||||
@@ -3339,6 +3355,34 @@ impl ChatWidget {
|
||||
self.needs_final_message_separator = false;
|
||||
}
|
||||
|
||||
/// Restore the blocked submission draft without losing mention resolution state.
|
||||
///
|
||||
/// The blocked-image path intentionally keeps the draft in the composer so
|
||||
/// users can remove attachments and retry. We must restore
|
||||
/// `mention_paths` alongside visible text; restoring only `$name` tokens
|
||||
/// makes the draft look correct while degrading mention resolution to
|
||||
/// name-only heuristics on retry.
|
||||
fn restore_blocked_image_submission(
|
||||
&mut self,
|
||||
text: String,
|
||||
text_elements: Vec<TextElement>,
|
||||
local_images: Vec<LocalImageAttachment>,
|
||||
mention_paths: HashMap<String, String>,
|
||||
) {
|
||||
// Preserve the user's composed payload so they can retry after changing models.
|
||||
let local_image_paths = local_images.iter().map(|img| img.path.clone()).collect();
|
||||
self.bottom_pane.set_composer_text_with_mention_paths(
|
||||
text,
|
||||
text_elements,
|
||||
local_image_paths,
|
||||
mention_paths,
|
||||
);
|
||||
self.add_to_history(history_cell::new_warning_event(
|
||||
self.image_inputs_not_supported_message(),
|
||||
));
|
||||
self.request_redraw();
|
||||
}
|
||||
|
||||
/// Replay a subset of initial events into the UI to seed the transcript when
|
||||
/// resuming an existing session. This approximates the live event flow and
|
||||
/// is intentionally conservative: only safe-to-replay items are rendered to
|
||||
@@ -5279,6 +5323,36 @@ impl ChatWidget {
|
||||
.unwrap_or(false)
|
||||
}
|
||||
|
||||
/// Return whether the effective model currently advertises image-input support.
|
||||
///
|
||||
/// We intentionally default to `true` when model metadata cannot be read so transient catalog
|
||||
/// failures do not hard-block user input in the UI.
|
||||
fn current_model_supports_images(&self) -> bool {
|
||||
let model = self.current_model();
|
||||
self.models_manager
|
||||
.try_list_models(&self.config)
|
||||
.ok()
|
||||
.and_then(|models| {
|
||||
models
|
||||
.into_iter()
|
||||
.find(|preset| preset.model == model)
|
||||
.map(|preset| preset.input_modalities.contains(&InputModality::Image))
|
||||
})
|
||||
.unwrap_or(true)
|
||||
}
|
||||
|
||||
fn sync_image_paste_enabled(&mut self) {
|
||||
let enabled = self.current_model_supports_images();
|
||||
self.bottom_pane.set_image_paste_enabled(enabled);
|
||||
}
|
||||
|
||||
fn image_inputs_not_supported_message(&self) -> String {
|
||||
format!(
|
||||
"Model {} does not support image inputs. Remove images or switch models.",
|
||||
self.current_model()
|
||||
)
|
||||
}
|
||||
|
||||
#[allow(dead_code)] // Used in tests
|
||||
pub(crate) fn current_collaboration_mode(&self) -> &CollaborationMode {
|
||||
&self.current_collaboration_mode
|
||||
@@ -5351,6 +5425,8 @@ impl ChatWidget {
|
||||
fn refresh_model_display(&mut self) {
|
||||
let effective = self.effective_collaboration_mode();
|
||||
self.session_header.set_model(effective.model());
|
||||
// Keep composer paste affordances aligned with the currently effective model.
|
||||
self.sync_image_paste_enabled();
|
||||
}
|
||||
|
||||
fn model_display_name(&self) -> &str {
|
||||
|
||||
@@ -70,6 +70,7 @@ use codex_protocol::config_types::Personality;
|
||||
use codex_protocol::config_types::Settings;
|
||||
use codex_protocol::openai_models::ModelPreset;
|
||||
use codex_protocol::openai_models::ReasoningEffortPreset;
|
||||
use codex_protocol::openai_models::default_input_modalities;
|
||||
use codex_protocol::parse_command::ParsedCommand;
|
||||
use codex_protocol::plan_tool::PlanItemArg;
|
||||
use codex_protocol::plan_tool::StepStatus;
|
||||
@@ -324,6 +325,49 @@ async fn submission_preserves_text_elements_and_local_images() {
|
||||
assert_eq!(stored_images, local_images);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn blocked_image_restore_preserves_mention_paths() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await;
|
||||
|
||||
let placeholder = "[Image #1]";
|
||||
let text = format!("{placeholder} check $file");
|
||||
let text_elements = vec![TextElement::new(
|
||||
(0..placeholder.len()).into(),
|
||||
Some(placeholder.to_string()),
|
||||
)];
|
||||
let local_images = vec![LocalImageAttachment {
|
||||
placeholder: placeholder.to_string(),
|
||||
path: PathBuf::from("/tmp/blocked.png"),
|
||||
}];
|
||||
let mention_paths =
|
||||
HashMap::from([("file".to_string(), "/tmp/skills/file/SKILL.md".to_string())]);
|
||||
|
||||
chat.restore_blocked_image_submission(
|
||||
text.clone(),
|
||||
text_elements.clone(),
|
||||
local_images.clone(),
|
||||
mention_paths.clone(),
|
||||
);
|
||||
|
||||
assert_eq!(chat.bottom_pane.composer_text(), text);
|
||||
assert_eq!(chat.bottom_pane.composer_text_elements(), text_elements);
|
||||
assert_eq!(
|
||||
chat.bottom_pane.composer_local_image_paths(),
|
||||
vec![local_images[0].path.clone()],
|
||||
);
|
||||
assert_eq!(chat.bottom_pane.take_mention_paths(), mention_paths);
|
||||
|
||||
let cells = drain_insert_history(&mut rx);
|
||||
let warning = cells
|
||||
.last()
|
||||
.map(|lines| lines_to_single_string(lines))
|
||||
.expect("expected warning cell");
|
||||
assert!(
|
||||
warning.contains("does not support image inputs"),
|
||||
"expected image warning, got: {warning:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn interrupted_turn_restores_queued_messages_with_images_and_elements() {
|
||||
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await;
|
||||
@@ -3154,6 +3198,7 @@ async fn model_picker_hides_show_in_picker_false_models_from_cache() {
|
||||
upgrade: None,
|
||||
show_in_picker,
|
||||
supported_in_api: true,
|
||||
input_modalities: default_input_modalities(),
|
||||
};
|
||||
|
||||
chat.open_model_popup_with_presets(vec![
|
||||
@@ -3392,6 +3437,7 @@ async fn single_reasoning_option_skips_selection() {
|
||||
upgrade: None,
|
||||
show_in_picker: true,
|
||||
supported_in_api: true,
|
||||
input_modalities: default_input_modalities(),
|
||||
};
|
||||
chat.open_reasoning_popup(preset);
|
||||
|
||||
|
||||
Reference in new issue
Block a user