diff --git a/codex-rs/app-server-protocol/schema/json/ClientRequest.json b/codex-rs/app-server-protocol/schema/json/ClientRequest.json index 85e5940bc3..6f5ebcdf0f 100644 --- a/codex-rs/app-server-protocol/schema/json/ClientRequest.json +++ b/codex-rs/app-server-protocol/schema/json/ClientRequest.json @@ -1038,6 +1038,13 @@ ], "type": "string" }, + "MessagePhase": { + "enum": [ + "commentary", + "final_answer" + ], + "type": "string" + }, "ModeKind": { "description": "Initial collaboration mode to use when the TUI starts.", "enum": [ @@ -1317,6 +1324,16 @@ ], "writeOnly": true }, + "phase": { + "anyOf": [ + { + "$ref": "#/definitions/MessagePhase" + }, + { + "type": "null" + } + ] + }, "role": { "type": "string" }, diff --git a/codex-rs/app-server-protocol/schema/json/EventMsg.json b/codex-rs/app-server-protocol/schema/json/EventMsg.json index 9e3cbb4156..3e73bdc8b6 100644 --- a/codex-rs/app-server-protocol/schema/json/EventMsg.json +++ b/codex-rs/app-server-protocol/schema/json/EventMsg.json @@ -3111,6 +3111,13 @@ } ] }, + "MessagePhase": { + "enum": [ + "commentary", + "final_answer" + ], + "type": "string" + }, "ModeKind": { "description": "Initial collaboration mode to use when the TUI starts.", "enum": [ @@ -3596,6 +3603,16 @@ ], "writeOnly": true }, + "phase": { + "anyOf": [ + { + "$ref": "#/definitions/MessagePhase" + }, + { + "type": "null" + } + ] + }, "role": { "type": "string" }, diff --git a/codex-rs/app-server-protocol/schema/json/ServerNotification.json b/codex-rs/app-server-protocol/schema/json/ServerNotification.json index 6df9a010b1..5d9e719d8f 100644 --- a/codex-rs/app-server-protocol/schema/json/ServerNotification.json +++ b/codex-rs/app-server-protocol/schema/json/ServerNotification.json @@ -3890,6 +3890,13 @@ ], "type": "string" }, + "MessagePhase": { + "enum": [ + "commentary", + "final_answer" + ], + "type": "string" + }, "ModeKind": { "description": "Initial collaboration mode to use when the TUI starts.", "enum": [ @@ -4636,6 +4643,16 @@ ], "writeOnly": true }, + "phase": { + "anyOf": [ + { + "$ref": "#/definitions/MessagePhase" + }, + { + "type": "null" + } + ] + }, "role": { "type": "string" }, diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index e92d46e7b6..5a5ddd1011 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -5839,6 +5839,13 @@ } ] }, + "MessagePhase": { + "enum": [ + "commentary", + "final_answer" + ], + "type": "string" + }, "ModeKind": { "description": "Initial collaboration mode to use when the TUI starts.", "enum": [ @@ -6536,6 +6543,16 @@ ], "writeOnly": true }, + "phase": { + "anyOf": [ + { + "$ref": "#/definitions/MessagePhase" + }, + { + "type": "null" + } + ] + }, "role": { "type": "string" }, @@ -11263,6 +11280,25 @@ }, "type": "object" }, + "InputModality": { + "description": "Canonical user-input modality tags advertised by a model.", + "oneOf": [ + { + "description": "Plain text turns and tool payloads.", + "enum": [ + "text" + ], + "type": "string" + }, + { + "description": "Image attachments included in user turns.", + "enum": [ + "image" + ], + "type": "string" + } + ] + }, "ItemCompletedNotification": { "$schema": "http://json-schema.org/draft-07/schema#", "properties": { @@ -11731,6 +11767,13 @@ ], "type": "string" }, + "MessagePhase": { + "enum": [ + "commentary", + "final_answer" + ], + "type": "string" + }, "ModeKind": { "description": "Initial collaboration mode to use when the TUI starts.", "enum": [ @@ -11756,6 +11799,16 @@ "id": { "type": "string" }, + "inputModalities": { + "default": [ + "text", + "image" + ], + "items": { + "$ref": "#/definitions/v2/InputModality" + }, + "type": "array" + }, "isDefault": { "type": "boolean" }, @@ -12449,6 +12502,16 @@ ], "writeOnly": true }, + "phase": { + "anyOf": [ + { + "$ref": "#/definitions/v2/MessagePhase" + }, + { + "type": "null" + } + ] + }, "role": { "type": "string" }, diff --git a/codex-rs/app-server-protocol/schema/json/v1/ForkConversationResponse.json b/codex-rs/app-server-protocol/schema/json/v1/ForkConversationResponse.json index ae6e705221..ad251a461f 100644 --- a/codex-rs/app-server-protocol/schema/json/v1/ForkConversationResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v1/ForkConversationResponse.json @@ -3111,6 +3111,13 @@ } ] }, + "MessagePhase": { + "enum": [ + "commentary", + "final_answer" + ], + "type": "string" + }, "ModeKind": { "description": "Initial collaboration mode to use when the TUI starts.", "enum": [ @@ -3596,6 +3603,16 @@ ], "writeOnly": true }, + "phase": { + "anyOf": [ + { + "$ref": "#/definitions/MessagePhase" + }, + { + "type": "null" + } + ] + }, "role": { "type": "string" }, diff --git a/codex-rs/app-server-protocol/schema/json/v1/ResumeConversationParams.json b/codex-rs/app-server-protocol/schema/json/v1/ResumeConversationParams.json index e1cc626cc8..41284b009b 100644 --- a/codex-rs/app-server-protocol/schema/json/v1/ResumeConversationParams.json +++ b/codex-rs/app-server-protocol/schema/json/v1/ResumeConversationParams.json @@ -266,6 +266,13 @@ ], "type": "string" }, + "MessagePhase": { + "enum": [ + "commentary", + "final_answer" + ], + "type": "string" + }, "NewConversationParams": { "properties": { "approvalPolicy": { @@ -437,6 +444,16 @@ ], "writeOnly": true }, + "phase": { + "anyOf": [ + { + "$ref": "#/definitions/MessagePhase" + }, + { + "type": "null" + } + ] + }, "role": { "type": "string" }, diff --git a/codex-rs/app-server-protocol/schema/json/v1/ResumeConversationResponse.json b/codex-rs/app-server-protocol/schema/json/v1/ResumeConversationResponse.json index cdad2018c6..abd5beb211 100644 --- a/codex-rs/app-server-protocol/schema/json/v1/ResumeConversationResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v1/ResumeConversationResponse.json @@ -3111,6 +3111,13 @@ } ] }, + "MessagePhase": { + "enum": [ + "commentary", + "final_answer" + ], + "type": "string" + }, "ModeKind": { "description": "Initial collaboration mode to use when the TUI starts.", "enum": [ @@ -3596,6 +3603,16 @@ ], "writeOnly": true }, + "phase": { + "anyOf": [ + { + "$ref": "#/definitions/MessagePhase" + }, + { + "type": "null" + } + ] + }, "role": { "type": "string" }, diff --git a/codex-rs/app-server-protocol/schema/json/v1/SessionConfiguredNotification.json b/codex-rs/app-server-protocol/schema/json/v1/SessionConfiguredNotification.json index e6ed4d0427..69c2c4b8b0 100644 --- a/codex-rs/app-server-protocol/schema/json/v1/SessionConfiguredNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v1/SessionConfiguredNotification.json @@ -3111,6 +3111,13 @@ } ] }, + "MessagePhase": { + "enum": [ + "commentary", + "final_answer" + ], + "type": "string" + }, "ModeKind": { "description": "Initial collaboration mode to use when the TUI starts.", "enum": [ @@ -3596,6 +3603,16 @@ ], "writeOnly": true }, + "phase": { + "anyOf": [ + { + "$ref": "#/definitions/MessagePhase" + }, + { + "type": "null" + } + ] + }, "role": { "type": "string" }, diff --git a/codex-rs/app-server-protocol/schema/json/v2/ModelListResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ModelListResponse.json index d41f13a87d..8c8126bb6b 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ModelListResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ModelListResponse.json @@ -1,6 +1,25 @@ { "$schema": "http://json-schema.org/draft-07/schema#", "definitions": { + "InputModality": { + "description": "Canonical user-input modality tags advertised by a model.", + "oneOf": [ + { + "description": "Plain text turns and tool payloads.", + "enum": [ + "text" + ], + "type": "string" + }, + { + "description": "Image attachments included in user turns.", + "enum": [ + "image" + ], + "type": "string" + } + ] + }, "Model": { "properties": { "defaultReasoningEffort": { @@ -15,6 +34,16 @@ "id": { "type": "string" }, + "inputModalities": { + "default": [ + "text", + "image" + ], + "items": { + "$ref": "#/definitions/InputModality" + }, + "type": "array" + }, "isDefault": { "type": "boolean" }, diff --git a/codex-rs/app-server-protocol/schema/json/v2/RawResponseItemCompletedNotification.json b/codex-rs/app-server-protocol/schema/json/v2/RawResponseItemCompletedNotification.json index 47039df05b..aecb8c2b58 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/RawResponseItemCompletedNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v2/RawResponseItemCompletedNotification.json @@ -233,6 +233,13 @@ ], "type": "string" }, + "MessagePhase": { + "enum": [ + "commentary", + "final_answer" + ], + "type": "string" + }, "ReasoningItemContent": { "oneOf": [ { @@ -324,6 +331,16 @@ ], "writeOnly": true }, + "phase": { + "anyOf": [ + { + "$ref": "#/definitions/MessagePhase" + }, + { + "type": "null" + } + ] + }, "role": { "type": "string" }, diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeParams.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeParams.json index 3da1e3c749..52567cec82 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeParams.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeParams.json @@ -242,6 +242,13 @@ ], "type": "string" }, + "MessagePhase": { + "enum": [ + "commentary", + "final_answer" + ], + "type": "string" + }, "Personality": { "enum": [ "friendly", @@ -340,6 +347,16 @@ ], "writeOnly": true }, + "phase": { + "anyOf": [ + { + "$ref": "#/definitions/MessagePhase" + }, + { + "type": "null" + } + ] + }, "role": { "type": "string" }, diff --git a/codex-rs/app-server-protocol/schema/typescript/InputModality.ts b/codex-rs/app-server-protocol/schema/typescript/InputModality.ts new file mode 100644 index 0000000000..73661938b3 --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/InputModality.ts @@ -0,0 +1,8 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. + +/** + * Canonical user-input modality tags advertised by a model. + */ +export type InputModality = "text" | "image"; diff --git a/codex-rs/app-server-protocol/schema/typescript/MessagePhase.ts b/codex-rs/app-server-protocol/schema/typescript/MessagePhase.ts new file mode 100644 index 0000000000..d339c0fa83 --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/MessagePhase.ts @@ -0,0 +1,5 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. + +export type MessagePhase = "commentary" | "final_answer"; diff --git a/codex-rs/app-server-protocol/schema/typescript/ResponseItem.ts b/codex-rs/app-server-protocol/schema/typescript/ResponseItem.ts index d720021612..611c7fb22d 100644 --- a/codex-rs/app-server-protocol/schema/typescript/ResponseItem.ts +++ b/codex-rs/app-server-protocol/schema/typescript/ResponseItem.ts @@ -6,11 +6,12 @@ import type { FunctionCallOutputPayload } from "./FunctionCallOutputPayload"; import type { GhostCommit } from "./GhostCommit"; import type { LocalShellAction } from "./LocalShellAction"; import type { LocalShellStatus } from "./LocalShellStatus"; +import type { MessagePhase } from "./MessagePhase"; import type { ReasoningItemContent } from "./ReasoningItemContent"; import type { ReasoningItemReasoningSummary } from "./ReasoningItemReasoningSummary"; import type { WebSearchAction } from "./WebSearchAction"; -export type ResponseItem = { "type": "message", role: string, content: Array, end_turn?: boolean, } | { "type": "reasoning", summary: Array, content?: Array, encrypted_content: string | null, } | { "type": "local_shell_call", +export type ResponseItem = { "type": "message", role: string, content: Array, end_turn?: boolean, phase?: MessagePhase, } | { "type": "reasoning", summary: Array, content?: Array, encrypted_content: string | null, } | { "type": "local_shell_call", /** * Set when using the Responses API. */ diff --git a/codex-rs/app-server-protocol/schema/typescript/index.ts b/codex-rs/app-server-protocol/schema/typescript/index.ts index 191f993864..91da0708e1 100644 --- a/codex-rs/app-server-protocol/schema/typescript/index.ts +++ b/codex-rs/app-server-protocol/schema/typescript/index.ts @@ -90,6 +90,7 @@ export type { InitializeCapabilities } from "./InitializeCapabilities"; export type { InitializeParams } from "./InitializeParams"; export type { InitializeResponse } from "./InitializeResponse"; export type { InputItem } from "./InputItem"; +export type { InputModality } from "./InputModality"; export type { InterruptConversationParams } from "./InterruptConversationParams"; export type { InterruptConversationResponse } from "./InterruptConversationResponse"; export type { ItemCompletedEvent } from "./ItemCompletedEvent"; @@ -115,6 +116,7 @@ export type { McpStartupStatus } from "./McpStartupStatus"; export type { McpStartupUpdateEvent } from "./McpStartupUpdateEvent"; export type { McpToolCallBeginEvent } from "./McpToolCallBeginEvent"; export type { McpToolCallEndEvent } from "./McpToolCallEndEvent"; +export type { MessagePhase } from "./MessagePhase"; export type { ModeKind } from "./ModeKind"; export type { NetworkAccess } from "./NetworkAccess"; export type { NewConversationParams } from "./NewConversationParams"; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/Model.ts b/codex-rs/app-server-protocol/schema/typescript/v2/Model.ts index b664024d0a..72daa93edd 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/Model.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/Model.ts @@ -1,7 +1,8 @@ // GENERATED CODE! DO NOT MODIFY BY HAND! // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. +import type { InputModality } from "../InputModality"; import type { ReasoningEffort } from "../ReasoningEffort"; import type { ReasoningEffortOption } from "./ReasoningEffortOption"; -export type Model = { id: string, model: string, displayName: string, description: string, supportedReasoningEfforts: Array, defaultReasoningEffort: ReasoningEffort, supportsPersonality: boolean, isDefault: boolean, }; +export type Model = { id: string, model: string, displayName: string, description: string, supportedReasoningEfforts: Array, defaultReasoningEffort: ReasoningEffort, inputModalities: Array, supportsPersonality: boolean, isDefault: boolean, }; diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 4f37e2889d..19db13723a 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -19,7 +19,9 @@ use codex_protocol::mcp::Resource as McpResource; use codex_protocol::mcp::ResourceTemplate as McpResourceTemplate; use codex_protocol::mcp::Tool as McpTool; use codex_protocol::models::ResponseItem; +use codex_protocol::openai_models::InputModality; use codex_protocol::openai_models::ReasoningEffort; +use codex_protocol::openai_models::default_input_modalities; use codex_protocol::parse_command::ParsedCommand as CoreParsedCommand; use codex_protocol::plan_tool::PlanItemArg as CorePlanItemArg; use codex_protocol::plan_tool::StepStatus as CorePlanStepStatus; @@ -992,6 +994,8 @@ pub struct Model { pub description: String, pub supported_reasoning_efforts: Vec, pub default_reasoning_effort: ReasoningEffort, + #[serde(default = "default_input_modalities")] + pub input_modalities: Vec, #[serde(default)] pub supports_personality: bool, // Only one model should be marked as default. diff --git a/codex-rs/app-server/src/models.rs b/codex-rs/app-server/src/models.rs index 4189435c05..133dc73cc0 100644 --- a/codex-rs/app-server/src/models.rs +++ b/codex-rs/app-server/src/models.rs @@ -28,6 +28,7 @@ fn model_from_preset(preset: ModelPreset) -> Model { preset.supported_reasoning_efforts, ), default_reasoning_effort: preset.default_reasoning_effort, + input_modalities: preset.input_modalities, supports_personality: preset.supports_personality, is_default: preset.is_default, } diff --git a/codex-rs/app-server/tests/common/models_cache.rs b/codex-rs/app-server/tests/common/models_cache.rs index 59fddc2427..14b4e8d458 100644 --- a/codex-rs/app-server/tests/common/models_cache.rs +++ b/codex-rs/app-server/tests/common/models_cache.rs @@ -6,6 +6,7 @@ use codex_protocol::openai_models::ModelInfo; use codex_protocol::openai_models::ModelPreset; use codex_protocol::openai_models::ModelVisibility; use codex_protocol::openai_models::TruncationPolicyConfig; +use codex_protocol::openai_models::default_input_modalities; use serde_json::json; use std::path::Path; @@ -38,6 +39,7 @@ fn preset_to_info(preset: &ModelPreset, priority: i32) -> ModelInfo { auto_compact_token_limit: None, effective_context_window_percent: 95, experimental_supported_tools: Vec::new(), + input_modalities: default_input_modalities(), } } @@ -75,9 +77,11 @@ pub fn write_models_cache_with_models( let cache_path = codex_home.join("models_cache.json"); // DateTime serializes to RFC3339 format by default with serde let fetched_at: DateTime = Utc::now(); + let client_version = codex_core::models_manager::client_version_to_whole(); let cache = json!({ "fetched_at": fetched_at, "etag": null, + "client_version": client_version, "models": models }); std::fs::write(cache_path, serde_json::to_string_pretty(&cache)?) diff --git a/codex-rs/app-server/tests/suite/list_resume.rs b/codex-rs/app-server/tests/suite/list_resume.rs index f2b82c6111..efa90ea351 100644 --- a/codex-rs/app-server/tests/suite/list_resume.rs +++ b/codex-rs/app-server/tests/suite/list_resume.rs @@ -308,6 +308,7 @@ async fn test_list_and_resume_conversations() -> Result<()> { text: fork_history_text.to_string(), }], end_turn: None, + phase: None, }]; let resume_with_history_req_id = mcp .send_resume_conversation_request(ResumeConversationParams { diff --git a/codex-rs/app-server/tests/suite/v2/compaction.rs b/codex-rs/app-server/tests/suite/v2/compaction.rs index 66cf43bc0a..eeedcb286b 100644 --- a/codex-rs/app-server/tests/suite/v2/compaction.rs +++ b/codex-rs/app-server/tests/suite/v2/compaction.rs @@ -126,6 +126,7 @@ async fn auto_compaction_remote_emits_started_and_completed_items() -> Result<() text: "REMOTE_COMPACT_SUMMARY".to_string(), }], end_turn: None, + phase: None, }, ResponseItem::Compaction { encrypted_content: "ENCRYPTED_COMPACTION_SUMMARY".to_string(), diff --git a/codex-rs/app-server/tests/suite/v2/model_list.rs b/codex-rs/app-server/tests/suite/v2/model_list.rs index a496e4a590..c3b4ec7088 100644 --- a/codex-rs/app-server/tests/suite/v2/model_list.rs +++ b/codex-rs/app-server/tests/suite/v2/model_list.rs @@ -12,6 +12,7 @@ use codex_app_server_protocol::ModelListParams; use codex_app_server_protocol::ModelListResponse; use codex_app_server_protocol::ReasoningEffortOption; use codex_app_server_protocol::RequestId; +use codex_protocol::openai_models::InputModality; use codex_protocol::openai_models::ReasoningEffort; use pretty_assertions::assert_eq; use tempfile::TempDir; @@ -72,6 +73,7 @@ async fn list_models_returns_all_models_with_large_limit() -> Result<()> { }, ], default_reasoning_effort: ReasoningEffort::Medium, + input_modalities: vec![InputModality::Text, InputModality::Image], supports_personality: false, is_default: true, }, @@ -100,6 +102,7 @@ async fn list_models_returns_all_models_with_large_limit() -> Result<()> { }, ], default_reasoning_effort: ReasoningEffort::Medium, + input_modalities: vec![InputModality::Text, InputModality::Image], supports_personality: false, is_default: false, }, @@ -120,6 +123,7 @@ async fn list_models_returns_all_models_with_large_limit() -> Result<()> { }, ], default_reasoning_effort: ReasoningEffort::Medium, + input_modalities: vec![InputModality::Text, InputModality::Image], supports_personality: false, is_default: false, }, @@ -154,6 +158,7 @@ async fn list_models_returns_all_models_with_large_limit() -> Result<()> { }, ], default_reasoning_effort: ReasoningEffort::Medium, + input_modalities: vec![InputModality::Text, InputModality::Image], supports_personality: false, is_default: false, }, diff --git a/codex-rs/app-server/tests/suite/v2/thread_resume.rs b/codex-rs/app-server/tests/suite/v2/thread_resume.rs index efa51573cb..358fec351f 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_resume.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_resume.rs @@ -338,6 +338,7 @@ async fn thread_resume_supports_history_and_overrides() -> Result<()> { text: history_text.to_string(), }], end_turn: None, + phase: None, }]; // Resume with explicit history and override the model. diff --git a/codex-rs/codex-api/src/endpoint/chat.rs b/codex-rs/codex-api/src/endpoint/chat.rs index 2148a5ad93..b183597d65 100644 --- a/codex-rs/codex-api/src/endpoint/chat.rs +++ b/codex-rs/codex-api/src/endpoint/chat.rs @@ -194,6 +194,7 @@ impl Stream for AggregatedStream { text: std::mem::take(&mut this.cumulative), }], end_turn: None, + phase: None, }; this.pending .push_back(ResponseEvent::OutputItemDone(aggregated_message)); diff --git a/codex-rs/codex-api/src/requests/chat.rs b/codex-rs/codex-api/src/requests/chat.rs index 5c16a5fb58..60a591e5c2 100644 --- a/codex-rs/codex-api/src/requests/chat.rs +++ b/codex-rs/codex-api/src/requests/chat.rs @@ -387,6 +387,7 @@ mod tests { text: "hi".to_string(), }], end_turn: None, + phase: None, }]; let req = ChatRequestBuilder::new("gpt-test", "inst", &prompt_input, &[]) .conversation_id(Some("conv-1".into())) @@ -414,6 +415,7 @@ mod tests { text: "read these".to_string(), }], end_turn: None, + phase: None, }, ResponseItem::FunctionCall { id: None, diff --git a/codex-rs/codex-api/src/requests/responses.rs b/codex-rs/codex-api/src/requests/responses.rs index 73a413dd93..a0e90db066 100644 --- a/codex-rs/codex-api/src/requests/responses.rs +++ b/codex-rs/codex-api/src/requests/responses.rs @@ -224,12 +224,14 @@ mod tests { role: "assistant".into(), content: Vec::new(), end_turn: None, + phase: None, }, ResponseItem::Message { id: None, role: "assistant".into(), content: Vec::new(), end_turn: None, + phase: None, }, ]; diff --git a/codex-rs/codex-api/src/sse/chat.rs b/codex-rs/codex-api/src/sse/chat.rs index a3effce2e7..4b78845285 100644 --- a/codex-rs/codex-api/src/sse/chat.rs +++ b/codex-rs/codex-api/src/sse/chat.rs @@ -331,6 +331,7 @@ async fn append_assistant_text( role: "assistant".to_string(), content: vec![], end_turn: None, + phase: None, }; *assistant_item = Some(item.clone()); let _ = tx_event diff --git a/codex-rs/codex-api/src/sse/responses.rs b/codex-rs/codex-api/src/sse/responses.rs index b363671a11..2c91104727 100644 --- a/codex-rs/codex-api/src/sse/responses.rs +++ b/codex-rs/codex-api/src/sse/responses.rs @@ -429,6 +429,7 @@ mod tests { use super::*; use assert_matches::assert_matches; use bytes::Bytes; + use codex_protocol::models::MessagePhase; use codex_protocol::models::ResponseItem; use futures::stream; use pretty_assertions::assert_eq; @@ -492,7 +493,8 @@ mod tests { "item": { "type": "message", "role": "assistant", - "content": [{"type": "output_text", "text": "Hello"}] + "content": [{"type": "output_text", "text": "Hello"}], + "phase": "commentary" } }) .to_string(); @@ -523,8 +525,11 @@ mod tests { assert_matches!( &events[0], - Ok(ResponseEvent::OutputItemDone(ResponseItem::Message { role, .. })) - if role == "assistant" + Ok(ResponseEvent::OutputItemDone(ResponseItem::Message { + role, + phase: Some(MessagePhase::Commentary), + .. + })) if role == "assistant" ); assert_matches!( diff --git a/codex-rs/codex-api/tests/clients.rs b/codex-rs/codex-api/tests/clients.rs index b71edf3244..3040fb8724 100644 --- a/codex-rs/codex-api/tests/clients.rs +++ b/codex-rs/codex-api/tests/clients.rs @@ -309,6 +309,7 @@ async fn streaming_client_retries_on_transport_error() -> Result<()> { text: "hi".to_string(), }], end_turn: None, + phase: None, }], tools: Vec::::new(), parallel_tool_calls: false, diff --git a/codex-rs/codex-api/tests/models_integration.rs b/codex-rs/codex-api/tests/models_integration.rs index c75c46a28a..2145be6542 100644 --- a/codex-rs/codex-api/tests/models_integration.rs +++ b/codex-rs/codex-api/tests/models_integration.rs @@ -11,6 +11,7 @@ use codex_protocol::openai_models::ModelsResponse; use codex_protocol::openai_models::ReasoningEffort; use codex_protocol::openai_models::ReasoningEffortPreset; use codex_protocol::openai_models::TruncationPolicyConfig; +use codex_protocol::openai_models::default_input_modalities; use http::HeaderMap; use http::Method; use wiremock::Mock; @@ -88,6 +89,7 @@ async fn models_client_hits_models_endpoint() { auto_compact_token_limit: None, effective_context_window_percent: 95, experimental_supported_tools: Vec::new(), + input_modalities: default_input_modalities(), }], }; diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 6cc389b531..ba84b6b432 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -1051,6 +1051,11 @@ impl Session { state.get_total_token_usage(state.server_reasoning_included()) } + async fn get_estimated_token_count(&self, turn_context: &TurnContext) -> Option { + let state = self.state.lock().await; + state.history.estimate_token_count(turn_context) + } + pub(crate) async fn get_base_instructions(&self) -> BaseInstructions { let state = self.state.lock().await; BaseInstructions { @@ -1834,6 +1839,7 @@ impl Session { text: format!("Warning: {}", message.into()), }], end_turn: None, + phase: None, }; self.record_conversation_items(ctx, &[item]).await; @@ -3309,6 +3315,7 @@ pub(crate) async fn run_turn( let model_info = turn_context.client.get_model_info(); let auto_compact_limit = model_info.auto_compact_token_limit().unwrap_or(i64::MAX); let total_usage_tokens = sess.get_total_token_usage().await; + let event = EventMsg::TurnStarted(TurnStartedEvent { model_context_window: turn_context.client.get_model_context_window(), collaboration_mode_kind: turn_context.collaboration_mode.mode, @@ -3464,6 +3471,19 @@ pub(crate) async fn run_turn( let total_usage_tokens = sess.get_total_token_usage().await; let token_limit_reached = total_usage_tokens >= auto_compact_limit; + let estimated_token_count = + sess.get_estimated_token_count(turn_context.as_ref()).await; + + info!( + turn_id = %turn_context.sub_id, + total_usage_tokens, + estimated_token_count = ?estimated_token_count, + auto_compact_limit, + token_limit_reached, + needs_follow_up, + "post sampling token usage" + ); + // as long as compaction works well in getting us way below the token limit, we shouldn't worry about being in an infinite loop. if token_limit_reached && needs_follow_up { run_auto_compact(&sess, &turn_context).await; @@ -4542,6 +4562,7 @@ mod tests { text: text.to_string(), }], end_turn: None, + phase: None, } } @@ -4823,6 +4844,7 @@ mod tests { text: "turn 1 user".to_string(), }], end_turn: None, + phase: None, }, ResponseItem::Message { id: None, @@ -4831,6 +4853,7 @@ mod tests { text: "turn 1 assistant".to_string(), }], end_turn: None, + phase: None, }, ]; sess.record_into_history(&turn_1, tc.as_ref()).await; @@ -4843,6 +4866,7 @@ mod tests { text: "turn 2 user".to_string(), }], end_turn: None, + phase: None, }, ResponseItem::Message { id: None, @@ -4851,6 +4875,7 @@ mod tests { text: "turn 2 assistant".to_string(), }], end_turn: None, + phase: None, }, ]; sess.record_into_history(&turn_2, tc.as_ref()).await; @@ -4883,6 +4908,7 @@ mod tests { text: "turn 1 user".to_string(), }], end_turn: None, + phase: None, }]; sess.record_into_history(&turn_1, tc.as_ref()).await; @@ -5832,6 +5858,7 @@ mod tests { text: "first user".to_string(), }], end_turn: None, + phase: None, }; live_history.record_items(std::iter::once(&user1), turn_context.truncation_policy); rollout_items.push(RolloutItem::ResponseItem(user1.clone())); @@ -5843,6 +5870,7 @@ mod tests { text: "assistant reply one".to_string(), }], end_turn: None, + phase: None, }; live_history.record_items(std::iter::once(&assistant1), turn_context.truncation_policy); rollout_items.push(RolloutItem::ResponseItem(assistant1.clone())); @@ -5868,6 +5896,7 @@ mod tests { text: "second user".to_string(), }], end_turn: None, + phase: None, }; live_history.record_items(std::iter::once(&user2), turn_context.truncation_policy); rollout_items.push(RolloutItem::ResponseItem(user2.clone())); @@ -5879,6 +5908,7 @@ mod tests { text: "assistant reply two".to_string(), }], end_turn: None, + phase: None, }; live_history.record_items(std::iter::once(&assistant2), turn_context.truncation_policy); rollout_items.push(RolloutItem::ResponseItem(assistant2.clone())); @@ -5904,6 +5934,7 @@ mod tests { text: "third user".to_string(), }], end_turn: None, + phase: None, }; live_history.record_items(std::iter::once(&user3), turn_context.truncation_policy); rollout_items.push(RolloutItem::ResponseItem(user3)); @@ -5915,6 +5946,7 @@ mod tests { text: "assistant reply three".to_string(), }], end_turn: None, + phase: None, }; live_history.record_items(std::iter::once(&assistant3), turn_context.truncation_policy); rollout_items.push(RolloutItem::ResponseItem(assistant3)); diff --git a/codex-rs/core/src/compact.rs b/codex-rs/core/src/compact.rs index b44d43f42e..ee94e4994b 100644 --- a/codex-rs/core/src/compact.rs +++ b/codex-rs/core/src/compact.rs @@ -311,6 +311,7 @@ fn build_compacted_history_with_limit( text: message.clone(), }], end_turn: None, + phase: None, }); } @@ -325,6 +326,7 @@ fn build_compacted_history_with_limit( role: "user".to_string(), content: vec![ContentItem::InputText { text: summary_text }], end_turn: None, + phase: None, }); history @@ -416,6 +418,7 @@ mod tests { text: "ignored".to_string(), }], end_turn: None, + phase: None, }, ResponseItem::Message { id: Some("user".to_string()), @@ -424,6 +427,7 @@ mod tests { text: "first".to_string(), }], end_turn: None, + phase: None, }, ResponseItem::Other, ]; @@ -444,6 +448,7 @@ mod tests { .to_string(), }], end_turn: None, + phase: None, }, ResponseItem::Message { id: None, @@ -452,6 +457,7 @@ mod tests { text: "cwd=/tmp".to_string(), }], end_turn: None, + phase: None, }, ResponseItem::Message { id: None, @@ -460,6 +466,7 @@ mod tests { text: "real user message".to_string(), }], end_turn: None, + phase: None, }, ]; @@ -545,6 +552,7 @@ mod tests { text: marker.clone(), }], end_turn: None, + phase: None, }, ResponseItem::Message { id: None, @@ -553,6 +561,7 @@ mod tests { text: "real user message".to_string(), }], end_turn: None, + phase: None, }, ]; diff --git a/codex-rs/core/src/compact_remote.rs b/codex-rs/core/src/compact_remote.rs index 9f7a8dfea3..12bc769ce2 100644 --- a/codex-rs/core/src/compact_remote.rs +++ b/codex-rs/core/src/compact_remote.rs @@ -3,6 +3,8 @@ use std::sync::Arc; use crate::Prompt; use crate::codex::Session; use crate::codex::TurnContext; +use crate::context_manager::ContextManager; +use crate::context_manager::is_codex_generated_item; use crate::error::Result as CodexResult; use crate::protocol::CompactedItem; use crate::protocol::EventMsg; @@ -11,6 +13,7 @@ use crate::protocol::TurnStartedEvent; use codex_protocol::items::ContextCompactionItem; use codex_protocol::items::TurnItem; use codex_protocol::models::ResponseItem; +use tracing::info; pub(crate) async fn run_inline_remote_auto_compact_task( sess: Arc, @@ -45,7 +48,16 @@ async fn run_remote_compact_task_inner_impl( let compaction_item = TurnItem::ContextCompaction(ContextCompactionItem::new()); sess.emit_turn_item_started(turn_context, &compaction_item) .await; - let history = sess.clone_history().await; + let mut history = sess.clone_history().await; + let deleted_items = + trim_function_call_history_to_fit_context_window(&mut history, turn_context.as_ref()); + if deleted_items > 0 { + info!( + turn_id = %turn_context.sub_id, + deleted_items, + "trimmed history items before remote compaction" + ); + } // Required to keep `/undo` available after compaction let ghost_snapshots: Vec = history @@ -86,3 +98,31 @@ async fn run_remote_compact_task_inner_impl( .await; Ok(()) } + +fn trim_function_call_history_to_fit_context_window( + history: &mut ContextManager, + turn_context: &TurnContext, +) -> usize { + let mut deleted_items = 0usize; + let Some(context_window) = turn_context.client.get_model_context_window() else { + return deleted_items; + }; + + while history + .estimate_token_count(turn_context) + .is_some_and(|estimated_tokens| estimated_tokens > context_window) + { + let Some(last_item) = history.raw_items().last() else { + break; + }; + if !is_codex_generated_item(last_item) { + break; + } + if !history.remove_last_item() { + break; + } + deleted_items += 1; + } + + deleted_items +} diff --git a/codex-rs/core/src/context_manager/history.rs b/codex-rs/core/src/context_manager/history.rs index 92dcef5ba4..a29f7df7e0 100644 --- a/codex-rs/core/src/context_manager/history.rs +++ b/codex-rs/core/src/context_manager/history.rs @@ -93,24 +93,7 @@ impl ContextManager { let base_tokens = i64::try_from(approx_token_count(&base_instructions)).unwrap_or(i64::MAX); let items_tokens = self.items.iter().fold(0i64, |acc, item| { - acc + match item { - ResponseItem::GhostSnapshot { .. } => 0, - ResponseItem::Reasoning { - encrypted_content: Some(content), - .. - } - | ResponseItem::Compaction { - encrypted_content: content, - } => { - let reasoning_bytes = estimate_reasoning_length(content.len()); - i64::try_from(approx_tokens_from_byte_count(reasoning_bytes)) - .unwrap_or(i64::MAX) - } - item => { - let serialized = serde_json::to_string(item).unwrap_or_default(); - i64::try_from(approx_token_count(&serialized)).unwrap_or(i64::MAX) - } - } + acc.saturating_add(estimate_item_token_count(item)) }); Some(base_tokens.saturating_add(items_tokens)) @@ -128,6 +111,15 @@ impl ContextManager { } } + pub(crate) fn remove_last_item(&mut self) -> bool { + if let Some(removed) = self.items.pop() { + normalize::remove_corresponding_for(&mut self.items, &removed); + true + } else { + false + } + } + pub(crate) fn replace(&mut self, items: Vec) { self.items = items; } @@ -207,36 +199,42 @@ impl ContextManager { ); } - fn get_non_last_reasoning_items_tokens(&self) -> usize { - // get reasoning items excluding all the ones after the last user message + fn get_non_last_reasoning_items_tokens(&self) -> i64 { + // Get reasoning items excluding all the ones after the last user message. let Some(last_user_index) = self .items .iter() .rposition(|item| matches!(item, ResponseItem::Message { role, .. } if role == "user")) else { - return 0usize; + return 0; }; - let total_reasoning_bytes = self - .items + self.items .iter() .take(last_user_index) - .filter_map(|item| { - if let ResponseItem::Reasoning { - encrypted_content: Some(content), - .. - } = item - { - Some(content.len()) - } else { - None - } + .filter(|item| { + matches!( + item, + ResponseItem::Reasoning { + encrypted_content: Some(_), + .. + } + ) }) - .map(estimate_reasoning_length) - .fold(0usize, usize::saturating_add); + .fold(0i64, |acc, item| { + acc.saturating_add(estimate_item_token_count(item)) + }) + } - let token_estimate = approx_tokens_from_byte_count(total_reasoning_bytes); - token_estimate as usize + fn get_trailing_codex_generated_items_tokens(&self) -> i64 { + let mut total = 0i64; + for item in self.items.iter().rev() { + if !is_codex_generated_item(item) { + break; + } + total = total.saturating_add(estimate_item_token_count(item)); + } + total } /// When true, the server already accounted for past reasoning tokens and @@ -247,10 +245,13 @@ impl ContextManager { .as_ref() .map(|info| info.last_token_usage.total_tokens) .unwrap_or(0); + let trailing_codex_generated_tokens = self.get_trailing_codex_generated_items_tokens(); if server_reasoning_included { - last_tokens + last_tokens.saturating_add(trailing_codex_generated_tokens) } else { - last_tokens.saturating_add(self.get_non_last_reasoning_items_tokens() as i64) + last_tokens + .saturating_add(self.get_non_last_reasoning_items_tokens()) + .saturating_add(trailing_codex_generated_tokens) } } @@ -332,6 +333,33 @@ fn estimate_reasoning_length(encoded_len: usize) -> usize { .saturating_sub(650) } +fn estimate_item_token_count(item: &ResponseItem) -> i64 { + match item { + ResponseItem::GhostSnapshot { .. } => 0, + ResponseItem::Reasoning { + encrypted_content: Some(content), + .. + } + | ResponseItem::Compaction { + encrypted_content: content, + } => { + let reasoning_bytes = estimate_reasoning_length(content.len()); + i64::try_from(approx_tokens_from_byte_count(reasoning_bytes)).unwrap_or(i64::MAX) + } + item => { + let serialized = serde_json::to_string(item).unwrap_or_default(); + i64::try_from(approx_token_count(&serialized)).unwrap_or(i64::MAX) + } + } +} + +pub(crate) fn is_codex_generated_item(item: &ResponseItem) -> bool { + matches!( + item, + ResponseItem::FunctionCallOutput { .. } | ResponseItem::CustomToolCallOutput { .. } + ) || matches!(item, ResponseItem::Message { role, .. } if role == "developer") +} + pub(crate) fn is_user_turn_boundary(item: &ResponseItem) -> bool { let ResponseItem::Message { role, content, .. } = item else { return false; diff --git a/codex-rs/core/src/context_manager/history_tests.rs b/codex-rs/core/src/context_manager/history_tests.rs index 50ee98e5b6..a6eba62f1a 100644 --- a/codex-rs/core/src/context_manager/history_tests.rs +++ b/codex-rs/core/src/context_manager/history_tests.rs @@ -24,6 +24,7 @@ fn assistant_msg(text: &str) -> ResponseItem { text: text.to_string(), }], end_turn: None, + phase: None, } } @@ -43,6 +44,7 @@ fn user_msg(text: &str) -> ResponseItem { text: text.to_string(), }], end_turn: None, + phase: None, } } @@ -54,6 +56,24 @@ fn user_input_text_msg(text: &str) -> ResponseItem { text: text.to_string(), }], end_turn: None, + phase: None, + } +} + +fn function_call_output(call_id: &str, content: &str) -> ResponseItem { + ResponseItem::FunctionCallOutput { + call_id: call_id.to_string(), + output: FunctionCallOutputPayload { + content: content.to_string(), + ..Default::default() + }, + } +} + +fn custom_tool_call_output(call_id: &str, output: &str) -> ResponseItem { + ResponseItem::CustomToolCallOutput { + call_id: call_id.to_string(), + output: output.to_string(), } } @@ -97,6 +117,7 @@ fn filters_non_api_messages() { text: "ignored".to_string(), }], end_turn: None, + phase: None, }; let reasoning = reasoning_msg("thinking..."); h.record_items([&system, &reasoning, &ResponseItem::Other], policy); @@ -127,6 +148,7 @@ fn filters_non_api_messages() { text: "hi".to_string() }], end_turn: None, + phase: None, }, ResponseItem::Message { id: None, @@ -135,6 +157,7 @@ fn filters_non_api_messages() { text: "hello".to_string() }], end_turn: None, + phase: None, } ] ); @@ -162,6 +185,63 @@ fn non_last_reasoning_tokens_ignore_entries_after_last_user() { assert_eq!(history.get_non_last_reasoning_items_tokens(), 32); } +#[test] +fn trailing_codex_generated_tokens_stop_at_first_non_generated_item() { + let earlier_output = function_call_output("call-earlier", "earlier output"); + let trailing_function_output = function_call_output("call-tail-1", "tail function output"); + let trailing_custom_output = custom_tool_call_output("call-tail-2", "tail custom output"); + let history = create_history_with_items(vec![ + earlier_output, + user_msg("boundary item"), + trailing_function_output.clone(), + trailing_custom_output.clone(), + ]); + let expected_tokens = estimate_item_token_count(&trailing_function_output) + .saturating_add(estimate_item_token_count(&trailing_custom_output)); + + assert_eq!( + history.get_trailing_codex_generated_items_tokens(), + expected_tokens + ); +} + +#[test] +fn trailing_codex_generated_tokens_exclude_function_call_tail() { + let history = create_history_with_items(vec![ResponseItem::FunctionCall { + id: None, + name: "not-generated".to_string(), + arguments: "{}".to_string(), + call_id: "call-tail".to_string(), + }]); + + assert_eq!(history.get_trailing_codex_generated_items_tokens(), 0); +} + +#[test] +fn total_token_usage_includes_only_trailing_codex_generated_items() { + let non_trailing_output = function_call_output("call-before-message", "not trailing"); + let trailing_assistant = assistant_msg("assistant boundary"); + let trailing_output = custom_tool_call_output("tool-tail", "trailing output"); + let mut history = create_history_with_items(vec![ + non_trailing_output, + user_msg("boundary"), + trailing_assistant, + trailing_output.clone(), + ]); + history.update_token_info( + &TokenUsage { + total_tokens: 100, + ..Default::default() + }, + None, + ); + + assert_eq!( + history.get_total_token_usage(true), + 100 + estimate_item_token_count(&trailing_output) + ); +} + #[test] fn get_history_for_prompt_drops_ghost_commits() { let items = vec![ResponseItem::GhostSnapshot { @@ -216,6 +296,30 @@ fn remove_first_item_removes_matching_call_for_output() { assert_eq!(h.raw_items(), vec![]); } +#[test] +fn remove_last_item_removes_matching_call_for_output() { + let items = vec![ + user_msg("before tool call"), + ResponseItem::FunctionCall { + id: None, + name: "do_it".to_string(), + arguments: "{}".to_string(), + call_id: "call-delete-last".to_string(), + }, + ResponseItem::FunctionCallOutput { + call_id: "call-delete-last".to_string(), + output: FunctionCallOutputPayload { + content: "ok".to_string(), + ..Default::default() + }, + }, + ]; + let mut h = create_history_with_items(items); + + assert!(h.remove_last_item()); + assert_eq!(h.raw_items(), vec![user_msg("before tool call")]); +} + #[test] fn replace_last_turn_images_replaces_tool_output_images() { let items = vec![ @@ -262,6 +366,7 @@ fn replace_last_turn_images_does_not_touch_user_images() { image_url: "data:image/png;base64,AAA".to_string(), }], end_turn: None, + phase: None, }]; let mut history = create_history_with_items(items.clone()); diff --git a/codex-rs/core/src/context_manager/mod.rs b/codex-rs/core/src/context_manager/mod.rs index baae93c775..22e9682fe3 100644 --- a/codex-rs/core/src/context_manager/mod.rs +++ b/codex-rs/core/src/context_manager/mod.rs @@ -2,4 +2,5 @@ mod history; mod normalize; pub(crate) use history::ContextManager; +pub(crate) use history::is_codex_generated_item; pub(crate) use history::is_user_turn_boundary; diff --git a/codex-rs/core/src/environment_context.rs b/codex-rs/core/src/environment_context.rs index 87692a2f05..9f5455a69f 100644 --- a/codex-rs/core/src/environment_context.rs +++ b/codex-rs/core/src/environment_context.rs @@ -81,6 +81,7 @@ impl From for ResponseItem { text: ec.serialize_to_xml(), }], end_turn: None, + phase: None, } } } diff --git a/codex-rs/core/src/event_mapping.rs b/codex-rs/core/src/event_mapping.rs index 35804fd47c..2ad19d3df5 100644 --- a/codex-rs/core/src/event_mapping.rs +++ b/codex-rs/core/src/event_mapping.rs @@ -177,6 +177,7 @@ mod tests { }, ], end_turn: None, + phase: None, }; let turn_item = parse_turn_item(&item).expect("expected user message turn item"); @@ -219,6 +220,7 @@ mod tests { }, ], end_turn: None, + phase: None, }; let turn_item = parse_turn_item(&item).expect("expected user message turn item"); @@ -260,6 +262,7 @@ mod tests { }, ], end_turn: None, + phase: None, }; let turn_item = parse_turn_item(&item).expect("expected user message turn item"); @@ -289,6 +292,7 @@ mod tests { text: "test_text".to_string(), }], end_turn: None, + phase: None, }, ResponseItem::Message { id: None, @@ -297,6 +301,7 @@ mod tests { text: "test_text".to_string(), }], end_turn: None, + phase: None, }, ResponseItem::Message { id: None, @@ -305,6 +310,7 @@ mod tests { text: "# AGENTS.md instructions for test_directory\n\n\ntest_text\n".to_string(), }], end_turn: None, + phase: None, }, ResponseItem::Message { id: None, @@ -314,6 +320,7 @@ mod tests { .to_string(), }], end_turn: None, + phase: None, }, ResponseItem::Message { id: None, @@ -322,6 +329,7 @@ mod tests { text: "echo 42".to_string(), }], end_turn: None, + phase: None, }, ]; @@ -340,6 +348,7 @@ mod tests { text: "Hello from Codex".to_string(), }], end_turn: None, + phase: None, }; let turn_item = parse_turn_item(&item).expect("expected agent message turn item"); diff --git a/codex-rs/core/src/instructions/user_instructions.rs b/codex-rs/core/src/instructions/user_instructions.rs index 611bf4cbf8..525834847e 100644 --- a/codex-rs/core/src/instructions/user_instructions.rs +++ b/codex-rs/core/src/instructions/user_instructions.rs @@ -39,6 +39,7 @@ impl From for ResponseItem { ), }], end_turn: None, + phase: None, } } } @@ -73,6 +74,7 @@ impl From for ResponseItem { ), }], end_turn: None, + phase: None, } } } diff --git a/codex-rs/core/src/models_manager/cache.rs b/codex-rs/core/src/models_manager/cache.rs index 07c5784f6c..e95b634b51 100644 --- a/codex-rs/core/src/models_manager/cache.rs +++ b/codex-rs/core/src/models_manager/cache.rs @@ -27,7 +27,7 @@ impl ModelsCacheManager { } /// Attempt to load a fresh cache entry. Returns `None` if the cache doesn't exist or is stale. - pub(crate) async fn load_fresh(&self) -> Option { + pub(crate) async fn load_fresh(&self, expected_version: &str) -> Option { let cache = match self.load().await { Ok(cache) => cache?, Err(err) => { @@ -35,6 +35,9 @@ impl ModelsCacheManager { return None; } }; + if cache.client_version.as_deref() != Some(expected_version) { + return None; + } if !cache.is_fresh(self.cache_ttl) { return None; } @@ -42,10 +45,16 @@ impl ModelsCacheManager { } /// Persist the cache to disk, creating parent directories as needed. - pub(crate) async fn persist_cache(&self, models: &[ModelInfo], etag: Option) { + pub(crate) async fn persist_cache( + &self, + models: &[ModelInfo], + etag: Option, + client_version: String, + ) { let cache = ModelsCache { fetched_at: Utc::now(), etag, + client_version: Some(client_version), models: models.to_vec(), }; if let Err(err) = self.save_internal(&cache).await { @@ -103,6 +112,20 @@ impl ModelsCacheManager { f(&mut cache.fetched_at); self.save_internal(&cache).await } + + #[cfg(test)] + /// Mutate the full cache contents for testing. + pub(crate) async fn mutate_cache_for_test(&self, f: F) -> io::Result<()> + where + F: FnOnce(&mut ModelsCache), + { + let mut cache = match self.load().await? { + Some(cache) => cache, + None => return Err(io::Error::new(ErrorKind::NotFound, "cache not found")), + }; + f(&mut cache); + self.save_internal(&cache).await + } } /// Serialized snapshot of models and metadata cached on disk. @@ -111,6 +134,8 @@ pub(crate) struct ModelsCache { pub(crate) fetched_at: DateTime, #[serde(default, skip_serializing_if = "Option::is_none")] pub(crate) etag: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub(crate) client_version: Option, pub(crate) models: Vec, } diff --git a/codex-rs/core/src/models_manager/manager.rs b/codex-rs/core/src/models_manager/manager.rs index 936e40d1bc..fffc280736 100644 --- a/codex-rs/core/src/models_manager/manager.rs +++ b/codex-rs/core/src/models_manager/manager.rs @@ -210,7 +210,7 @@ impl ModelsManager { let transport = ReqwestTransport::new(build_reqwest_client()); let client = ModelsClient::new(transport, api_provider, api_auth); - let client_version = format_client_version_to_whole(); + let client_version = crate::models_manager::client_version_to_whole(); let (models, etag) = timeout( MODELS_REFRESH_TIMEOUT, client.list_models(&client_version, HeaderMap::new()), @@ -221,7 +221,9 @@ impl ModelsManager { self.apply_remote_models(models.clone()).await; *self.etag.write().await = etag.clone(); - self.cache_manager.persist_cache(&models, etag).await; + self.cache_manager + .persist_cache(&models, etag, client_version) + .await; Ok(()) } @@ -255,7 +257,8 @@ impl ModelsManager { async fn try_load_cache(&self) -> bool { let _timer = codex_otel::start_global_timer("codex.remote_models.load_cache.duration_ms", &[]); - let cache = match self.cache_manager.load_fresh().await { + let client_version = crate::models_manager::client_version_to_whole(); + let cache = match self.cache_manager.load_fresh(&client_version).await { Some(cache) => cache, None => return false, }; @@ -350,16 +353,6 @@ impl ModelsManager { } } -/// Convert a client version string to a whole version string (e.g. "1.2.3-alpha.4" -> "1.2.3") -fn format_client_version_to_whole() -> String { - format!( - "{}.{}.{}", - env!("CARGO_PKG_VERSION_MAJOR"), - env!("CARGO_PKG_VERSION_MINOR"), - env!("CARGO_PKG_VERSION_PATCH") - ) -} - #[cfg(test)] mod tests { use super::*; @@ -613,6 +606,75 @@ mod tests { ); } + #[tokio::test] + async fn refresh_available_models_refetches_when_version_mismatch() { + let server = MockServer::start().await; + let initial_models = vec![remote_model("old", "Old", 1)]; + let initial_mock = mount_models_once( + &server, + ModelsResponse { + models: initial_models.clone(), + }, + ) + .await; + + let codex_home = tempdir().expect("temp dir"); + let mut config = ConfigBuilder::default() + .codex_home(codex_home.path().to_path_buf()) + .build() + .await + .expect("load default test config"); + config.features.enable(Feature::RemoteModels); + let auth_manager = Arc::new(AuthManager::new( + codex_home.path().to_path_buf(), + false, + AuthCredentialsStoreMode::File, + )); + let provider = provider_for(server.uri()); + let manager = + ModelsManager::with_provider(codex_home.path().to_path_buf(), auth_manager, provider); + + manager + .refresh_available_models(&config, RefreshStrategy::OnlineIfUncached) + .await + .expect("initial refresh succeeds"); + + manager + .cache_manager + .mutate_cache_for_test(|cache| { + let client_version = crate::models_manager::client_version_to_whole(); + cache.client_version = Some(format!("{client_version}-mismatch")); + }) + .await + .expect("cache mutation succeeds"); + + let updated_models = vec![remote_model("new", "New", 2)]; + server.reset().await; + let refreshed_mock = mount_models_once( + &server, + ModelsResponse { + models: updated_models.clone(), + }, + ) + .await; + + manager + .refresh_available_models(&config, RefreshStrategy::OnlineIfUncached) + .await + .expect("second refresh succeeds"); + assert_models_contain(&manager.get_remote_models(&config).await, &updated_models); + assert_eq!( + initial_mock.requests().len(), + 1, + "initial refresh should only hit /models once" + ); + assert_eq!( + refreshed_mock.requests().len(), + 1, + "version mismatch should fetch /models once" + ); + } + #[tokio::test] async fn refresh_available_models_drops_removed_remote_models() { let server = MockServer::start().await; diff --git a/codex-rs/core/src/models_manager/mod.rs b/codex-rs/core/src/models_manager/mod.rs index 3a649d5f14..9527466252 100644 --- a/codex-rs/core/src/models_manager/mod.rs +++ b/codex-rs/core/src/models_manager/mod.rs @@ -6,3 +6,13 @@ pub mod model_presets; #[cfg(any(test, feature = "test-support"))] pub use collaboration_mode_presets::test_builtin_collaboration_mode_presets; + +/// Convert the client version string to a whole version string (e.g. "1.2.3-alpha.4" -> "1.2.3"). +pub fn client_version_to_whole() -> String { + format!( + "{}.{}.{}", + env!("CARGO_PKG_VERSION_MAJOR"), + env!("CARGO_PKG_VERSION_MINOR"), + env!("CARGO_PKG_VERSION_PATCH") + ) +} diff --git a/codex-rs/core/src/models_manager/model_info.rs b/codex-rs/core/src/models_manager/model_info.rs index eaab160001..5cccefdd21 100644 --- a/codex-rs/core/src/models_manager/model_info.rs +++ b/codex-rs/core/src/models_manager/model_info.rs @@ -9,6 +9,7 @@ use codex_protocol::openai_models::ReasoningEffort; use codex_protocol::openai_models::ReasoningEffortPreset; use codex_protocol::openai_models::TruncationMode; use codex_protocol::openai_models::TruncationPolicyConfig; +use codex_protocol::openai_models::default_input_modalities; use crate::config::Config; use crate::features::Feature; @@ -66,6 +67,7 @@ macro_rules! model_info { auto_compact_token_limit: None, effective_context_window_percent: 95, experimental_supported_tools: Vec::new(), + input_modalities: default_input_modalities(), }; $( diff --git a/codex-rs/core/src/models_manager/model_presets.rs b/codex-rs/core/src/models_manager/model_presets.rs index f9105c6448..a597f7f922 100644 --- a/codex-rs/core/src/models_manager/model_presets.rs +++ b/codex-rs/core/src/models_manager/model_presets.rs @@ -3,6 +3,7 @@ use codex_protocol::openai_models::ModelPreset; use codex_protocol::openai_models::ModelUpgrade; use codex_protocol::openai_models::ReasoningEffort; use codex_protocol::openai_models::ReasoningEffortPreset; +use codex_protocol::openai_models::default_input_modalities; use indoc::indoc; use once_cell::sync::Lazy; @@ -41,6 +42,7 @@ static PRESETS: Lazy> = Lazy::new(|| { upgrade: None, show_in_picker: true, supported_in_api: true, + input_modalities: default_input_modalities(), }, ModelPreset { id: "gpt-5.1-codex-max".to_string(), @@ -71,6 +73,7 @@ static PRESETS: Lazy> = Lazy::new(|| { upgrade: Some(gpt_52_codex_upgrade()), show_in_picker: true, supported_in_api: true, + input_modalities: default_input_modalities(), }, ModelPreset { id: "gpt-5.1-codex-mini".to_string(), @@ -94,6 +97,7 @@ static PRESETS: Lazy> = Lazy::new(|| { upgrade: Some(gpt_52_codex_upgrade()), show_in_picker: true, supported_in_api: true, + input_modalities: default_input_modalities(), }, ModelPreset { id: "gpt-5.2".to_string(), @@ -124,6 +128,7 @@ static PRESETS: Lazy> = Lazy::new(|| { upgrade: Some(gpt_52_codex_upgrade()), show_in_picker: true, supported_in_api: true, + input_modalities: default_input_modalities(), }, ModelPreset { id: "bengalfox".to_string(), @@ -154,6 +159,7 @@ static PRESETS: Lazy> = Lazy::new(|| { upgrade: None, show_in_picker: false, supported_in_api: true, + input_modalities: default_input_modalities(), }, ModelPreset { id: "boomslang".to_string(), @@ -184,6 +190,7 @@ static PRESETS: Lazy> = Lazy::new(|| { upgrade: None, show_in_picker: false, supported_in_api: true, + input_modalities: default_input_modalities(), }, // Deprecated models. ModelPreset { @@ -211,6 +218,7 @@ static PRESETS: Lazy> = Lazy::new(|| { upgrade: Some(gpt_52_codex_upgrade()), show_in_picker: false, supported_in_api: true, + input_modalities: default_input_modalities(), }, ModelPreset { id: "gpt-5-codex-mini".to_string(), @@ -233,6 +241,7 @@ static PRESETS: Lazy> = Lazy::new(|| { upgrade: Some(gpt_52_codex_upgrade()), show_in_picker: false, supported_in_api: true, + input_modalities: default_input_modalities(), }, ModelPreset { id: "gpt-5.1-codex".to_string(), @@ -260,6 +269,7 @@ static PRESETS: Lazy> = Lazy::new(|| { upgrade: Some(gpt_52_codex_upgrade()), show_in_picker: false, supported_in_api: true, + input_modalities: default_input_modalities(), }, ModelPreset { id: "gpt-5".to_string(), @@ -290,6 +300,7 @@ static PRESETS: Lazy> = Lazy::new(|| { upgrade: Some(gpt_52_codex_upgrade()), show_in_picker: false, supported_in_api: true, + input_modalities: default_input_modalities(), }, ModelPreset { id: "gpt-5.1".to_string(), @@ -316,6 +327,7 @@ static PRESETS: Lazy> = Lazy::new(|| { upgrade: Some(gpt_52_codex_upgrade()), show_in_picker: false, supported_in_api: true, + input_modalities: default_input_modalities(), }, ] }); diff --git a/codex-rs/core/src/rollout/tests.rs b/codex-rs/core/src/rollout/tests.rs index cdf16291d3..ee750b126d 100644 --- a/codex-rs/core/src/rollout/tests.rs +++ b/codex-rs/core/src/rollout/tests.rs @@ -902,6 +902,7 @@ async fn test_updated_at_uses_file_mtime() -> Result<()> { text: format!("reply-{idx}"), }], end_turn: None, + phase: None, }), }; writeln!(file, "{}", serde_json::to_string(&response_line)?)?; diff --git a/codex-rs/core/src/rollout/truncation.rs b/codex-rs/core/src/rollout/truncation.rs index e6a84628fc..c50eacc48b 100644 --- a/codex-rs/core/src/rollout/truncation.rs +++ b/codex-rs/core/src/rollout/truncation.rs @@ -86,6 +86,7 @@ mod tests { text: text.to_string(), }], end_turn: None, + phase: None, } } @@ -97,6 +98,7 @@ mod tests { text: text.to_string(), }], end_turn: None, + phase: None, } } diff --git a/codex-rs/core/src/tasks/mod.rs b/codex-rs/core/src/tasks/mod.rs index a487041eba..d5d6f60582 100644 --- a/codex-rs/core/src/tasks/mod.rs +++ b/codex-rs/core/src/tasks/mod.rs @@ -265,6 +265,7 @@ impl Session { ), }], end_turn: None, + phase: None, }; self.record_into_history(std::slice::from_ref(&marker), task.turn_context.as_ref()) .await; diff --git a/codex-rs/core/src/tasks/review.rs b/codex-rs/core/src/tasks/review.rs index d156d3e0d5..cfc554f856 100644 --- a/codex-rs/core/src/tasks/review.rs +++ b/codex-rs/core/src/tasks/review.rs @@ -222,6 +222,7 @@ pub(crate) async fn exit_review_mode( role: "user".to_string(), content: vec![ContentItem::InputText { text: user_message }], end_turn: None, + phase: None, }], ) .await; @@ -241,6 +242,7 @@ pub(crate) async fn exit_review_mode( text: assistant_message, }], end_turn: None, + phase: None, }, ) .await; diff --git a/codex-rs/core/src/thread_manager.rs b/codex-rs/core/src/thread_manager.rs index 9b633ded37..37bd0efabc 100644 --- a/codex-rs/core/src/thread_manager.rs +++ b/codex-rs/core/src/thread_manager.rs @@ -462,6 +462,7 @@ mod tests { text: text.to_string(), }], end_turn: None, + phase: None, } } fn assistant_msg(text: &str) -> ResponseItem { @@ -472,6 +473,7 @@ mod tests { text: text.to_string(), }], end_turn: None, + phase: None, } } diff --git a/codex-rs/core/src/user_shell_command.rs b/codex-rs/core/src/user_shell_command.rs index 566f39958e..80128df006 100644 --- a/codex-rs/core/src/user_shell_command.rs +++ b/codex-rs/core/src/user_shell_command.rs @@ -63,6 +63,7 @@ pub fn user_shell_command_record_item( text: format_user_shell_command_record(command, exec_output, turn_context), }], end_turn: None, + phase: None, } } diff --git a/codex-rs/core/tests/chat_completions_payload.rs b/codex-rs/core/tests/chat_completions_payload.rs index 5406b44ad6..2772091441 100644 --- a/codex-rs/core/tests/chat_completions_payload.rs +++ b/codex-rs/core/tests/chat_completions_payload.rs @@ -139,6 +139,7 @@ fn user_message(text: &str) -> ResponseItem { text: text.to_string(), }], end_turn: None, + phase: None, } } @@ -150,6 +151,7 @@ fn assistant_message(text: &str) -> ResponseItem { text: text.to_string(), }], end_turn: None, + phase: None, } } diff --git a/codex-rs/core/tests/chat_completions_sse.rs b/codex-rs/core/tests/chat_completions_sse.rs index 8360237752..98e2fd9f0f 100644 --- a/codex-rs/core/tests/chat_completions_sse.rs +++ b/codex-rs/core/tests/chat_completions_sse.rs @@ -113,6 +113,7 @@ async fn run_stream_with_bytes(sse_body: &[u8]) -> Vec { text: "hello".to_string(), }], end_turn: None, + phase: None, }]; let mut stream = match client.stream(&prompt).await { diff --git a/codex-rs/core/tests/responses_headers.rs b/codex-rs/core/tests/responses_headers.rs index a40c92d02d..86e511c472 100644 --- a/codex-rs/core/tests/responses_headers.rs +++ b/codex-rs/core/tests/responses_headers.rs @@ -110,6 +110,7 @@ async fn responses_stream_includes_subagent_header_on_review() { text: "hello".into(), }], end_turn: None, + phase: None, }]; let mut stream = client_session.stream(&prompt).await.expect("stream failed"); @@ -209,6 +210,7 @@ async fn responses_stream_includes_subagent_header_on_other() { text: "hello".into(), }], end_turn: None, + phase: None, }]; let mut stream = client_session.stream(&prompt).await.expect("stream failed"); @@ -366,6 +368,7 @@ async fn responses_respects_model_info_overrides_from_config() { text: "hello".into(), }], end_turn: None, + phase: None, }]; let mut stream = client.stream(&prompt).await.expect("stream failed"); @@ -473,6 +476,7 @@ async fn responses_stream_includes_turn_metadata_header_for_git_workspace_e2e() text: "hello".into(), }], end_turn: None, + phase: None, }]; let first_request = responses::mount_sse_once(&server, response_body.clone()).await; diff --git a/codex-rs/core/tests/suite/client.rs b/codex-rs/core/tests/suite/client.rs index d79ef22b51..d0f2ba1ae7 100644 --- a/codex-rs/core/tests/suite/client.rs +++ b/codex-rs/core/tests/suite/client.rs @@ -29,6 +29,7 @@ use codex_protocol::config_types::ReasoningSummary; use codex_protocol::config_types::Settings; use codex_protocol::config_types::Verbosity; use codex_protocol::models::FunctionCallOutputPayload; +use codex_protocol::models::MessagePhase; use codex_protocol::models::ReasoningItemContent; use codex_protocol::models::ReasoningItemReasoningSummary; use codex_protocol::models::WebSearchAction; @@ -197,6 +198,7 @@ async fn resume_includes_initial_messages_and_sends_prior_items() { text: "resumed user message".to_string(), }], end_turn: None, + phase: None, }; let prior_user_json = serde_json::to_value(&prior_user).unwrap(); writeln!( @@ -218,6 +220,7 @@ async fn resume_includes_initial_messages_and_sends_prior_items() { text: "resumed system instruction".to_string(), }], end_turn: None, + phase: None, }; let prior_system_json = serde_json::to_value(&prior_system).unwrap(); writeln!( @@ -239,6 +242,7 @@ async fn resume_includes_initial_messages_and_sends_prior_items() { text: "resumed assistant message".to_string(), }], end_turn: None, + phase: Some(MessagePhase::Commentary), }; let prior_item_json = serde_json::to_value(&prior_item).unwrap(); writeln!( @@ -318,6 +322,25 @@ async fn resume_includes_initial_messages_and_sends_prior_items() { .iter() .position(|(role, text)| role == "assistant" && text == "resumed assistant message") .expect("prior assistant message"); + let prior_assistant = input + .iter() + .find(|item| { + item.get("role").and_then(|role| role.as_str()) == Some("assistant") + && item + .get("content") + .and_then(|content| content.as_array()) + .and_then(|content| content.first()) + .and_then(|entry| entry.get("text")) + .and_then(|text| text.as_str()) + == Some("resumed assistant message") + }) + .expect("resumed assistant message request item"); + assert_eq!( + prior_assistant + .get("phase") + .and_then(|phase| phase.as_str()), + Some("commentary") + ); let pos_permissions = messages .iter() .position(|(role, text)| role == "developer" && text.contains("")) @@ -1210,6 +1233,7 @@ async fn azure_responses_request_includes_store_and_reasoning_ids() { text: "message".into(), }], end_turn: None, + phase: None, }); prompt.input.push(ResponseItem::WebSearchCall { id: Some("web-search-id".into()), diff --git a/codex-rs/core/tests/suite/client_websockets.rs b/codex-rs/core/tests/suite/client_websockets.rs index 1acc3756d0..51c94edd7b 100644 --- a/codex-rs/core/tests/suite/client_websockets.rs +++ b/codex-rs/core/tests/suite/client_websockets.rs @@ -211,6 +211,7 @@ fn message_item(text: &str) -> ResponseItem { role: "user".into(), content: vec![ContentItem::InputText { text: text.into() }], end_turn: None, + phase: None, } } diff --git a/codex-rs/core/tests/suite/compact.rs b/codex-rs/core/tests/suite/compact.rs index fa07bf7f9b..ea9d2f92fa 100644 --- a/codex-rs/core/tests/suite/compact.rs +++ b/codex-rs/core/tests/suite/compact.rs @@ -1452,6 +1452,7 @@ async fn auto_compact_runs_after_resume_when_token_usage_is_over_limit() { text: remote_summary.to_string(), }], end_turn: None, + phase: None, }, codex_protocol::models::ResponseItem::Compaction { encrypted_content: "ENCRYPTED_COMPACTION_SUMMARY".to_string(), @@ -2275,6 +2276,7 @@ async fn auto_compact_counts_encrypted_reasoning_before_last_user() { text: "REMOTE_COMPACT_SUMMARY".to_string(), }], end_turn: None, + phase: None, }, codex_protocol::models::ResponseItem::Compaction { encrypted_content: "ENCRYPTED_COMPACTION_SUMMARY".to_string(), @@ -2395,6 +2397,7 @@ async fn auto_compact_runs_when_reasoning_header_clears_between_turns() { text: "REMOTE_COMPACT_SUMMARY".to_string(), }], end_turn: None, + phase: None, }, codex_protocol::models::ResponseItem::Compaction { encrypted_content: "ENCRYPTED_COMPACTION_SUMMARY".to_string(), diff --git a/codex-rs/core/tests/suite/compact_remote.rs b/codex-rs/core/tests/suite/compact_remote.rs index 563aff7826..55985c9b7a 100644 --- a/codex-rs/core/tests/suite/compact_remote.rs +++ b/codex-rs/core/tests/suite/compact_remote.rs @@ -62,6 +62,7 @@ async fn remote_compact_replaces_history_for_followups() -> Result<()> { text: "REMOTE_COMPACTED_SUMMARY".to_string(), }], end_turn: None, + phase: None, }, ResponseItem::Compaction { encrypted_content: "ENCRYPTED_COMPACTION_SUMMARY".to_string(), @@ -184,6 +185,7 @@ async fn remote_compact_runs_automatically() -> Result<()> { text: "REMOTE_COMPACTED_SUMMARY".to_string(), }], end_turn: None, + phase: None, }, ResponseItem::Compaction { encrypted_content: "ENCRYPTED_COMPACTION_SUMMARY".to_string(), @@ -220,6 +222,134 @@ async fn remote_compact_runs_automatically() -> Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn remote_compact_trims_function_call_history_to_fit_context_window() -> Result<()> { + skip_if_no_network!(Ok(())); + + let first_user_message = "turn with retained shell call"; + let second_user_message = "turn with trimmed shell call"; + let retained_call_id = "retained-call"; + let trimmed_call_id = "trimmed-call"; + let retained_command = "echo retained-shell-output"; + let trimmed_command = "yes x | head -n 3000"; + + let harness = TestCodexHarness::with_builder( + test_codex() + .with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing()) + .with_config(|config| { + config.features.enable(Feature::RemoteCompaction); + config.model_context_window = Some(2_000); + }), + ) + .await?; + let codex = harness.test().codex.clone(); + + let response_log = responses::mount_sse_sequence( + harness.server(), + vec![ + sse(vec![ + responses::ev_shell_command_call(retained_call_id, retained_command), + responses::ev_completed("retained-call-response"), + ]), + sse(vec![ + responses::ev_assistant_message("retained-assistant", "retained complete"), + responses::ev_completed("retained-final-response"), + ]), + sse(vec![ + responses::ev_shell_command_call(trimmed_call_id, trimmed_command), + responses::ev_completed("trimmed-call-response"), + ]), + sse(vec![responses::ev_completed("trimmed-final-response")]), + ], + ) + .await; + + codex + .submit(Op::UserInput { + items: vec![UserInput::Text { + text: first_user_message.into(), + text_elements: Vec::new(), + }], + final_output_json_schema: None, + }) + .await?; + wait_for_event(&codex, |event| matches!(event, EventMsg::TurnComplete(_))).await; + + codex + .submit(Op::UserInput { + items: vec![UserInput::Text { + text: second_user_message.into(), + text_elements: Vec::new(), + }], + final_output_json_schema: None, + }) + .await?; + wait_for_event(&codex, |event| matches!(event, EventMsg::TurnComplete(_))).await; + + let compact_mock = + responses::mount_compact_json_once(harness.server(), serde_json::json!({ "output": [] })) + .await; + + codex.submit(Op::Compact).await?; + wait_for_event(&codex, |event| matches!(event, EventMsg::TurnComplete(_))).await; + + assert!( + response_log + .function_call_output_text(retained_call_id) + .is_some(), + "expected retained shell call to produce function_call_output before compaction" + ); + assert!( + response_log + .function_call_output_text(trimmed_call_id) + .is_some(), + "expected trimmed shell call to produce function_call_output before compaction" + ); + + let compact_request = compact_mock.single_request(); + let user_messages = compact_request.message_input_texts("user"); + assert!( + user_messages + .iter() + .any(|message| message == first_user_message), + "expected compact request to retain earlier user history" + ); + assert!( + user_messages + .iter() + .any(|message| message == second_user_message), + "expected compact request to retain the user boundary message" + ); + + assert!( + compact_request.has_function_call(retained_call_id) + && compact_request + .function_call_output_text(retained_call_id) + .is_some(), + "expected compact request to keep the older function call/result pair" + ); + assert!( + !compact_request.has_function_call(trimmed_call_id) + && compact_request + .function_call_output_text(trimmed_call_id) + .is_none(), + "expected compact request to drop the trailing function call/result pair past the boundary" + ); + + assert_eq!( + compact_request.inputs_of_type("function_call").len(), + 1, + "expected exactly one function call after trimming" + ); + assert_eq!( + compact_request.inputs_of_type("function_call_output").len(), + 1, + "expected exactly one function call output after trimming" + ); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn remote_manual_compact_emits_context_compaction_items() -> Result<()> { skip_if_no_network!(Ok(())); @@ -251,6 +381,7 @@ async fn remote_manual_compact_emits_context_compaction_items() -> Result<()> { text: "REMOTE_COMPACTED_SUMMARY".to_string(), }], end_turn: None, + phase: None, }, ResponseItem::Compaction { encrypted_content: "ENCRYPTED_COMPACTION_SUMMARY".to_string(), @@ -352,6 +483,7 @@ async fn remote_compact_persists_replacement_history_in_rollout() -> Result<()> text: "COMPACTED_USER_SUMMARY".to_string(), }], end_turn: None, + phase: None, }, ResponseItem::Compaction { encrypted_content: "ENCRYPTED_COMPACTION_SUMMARY".to_string(), @@ -363,6 +495,7 @@ async fn remote_compact_persists_replacement_history_in_rollout() -> Result<()> text: "COMPACTED_ASSISTANT_NOTE".to_string(), }], end_turn: None, + phase: None, }, ]; let compact_mock = responses::mount_compact_json_once( diff --git a/codex-rs/core/tests/suite/image_rollout.rs b/codex-rs/core/tests/suite/image_rollout.rs index 6aebe6630d..691531e049 100644 --- a/codex-rs/core/tests/suite/image_rollout.rs +++ b/codex-rs/core/tests/suite/image_rollout.rs @@ -158,6 +158,7 @@ async fn copy_paste_local_image_persists_rollout_request_shape() -> anyhow::Resu }, ], end_turn: None, + phase: None, }; assert_eq!(actual, expected); @@ -239,6 +240,7 @@ async fn drag_drop_image_persists_rollout_request_shape() -> anyhow::Result<()> }, ], end_turn: None, + phase: None, }; assert_eq!(actual, expected); diff --git a/codex-rs/core/tests/suite/list_models.rs b/codex-rs/core/tests/suite/list_models.rs index f6db54af7b..aee3a60e0f 100644 --- a/codex-rs/core/tests/suite/list_models.rs +++ b/codex-rs/core/tests/suite/list_models.rs @@ -7,6 +7,7 @@ use codex_protocol::openai_models::ModelPreset; use codex_protocol::openai_models::ModelUpgrade; use codex_protocol::openai_models::ReasoningEffort; use codex_protocol::openai_models::ReasoningEffortPreset; +use codex_protocol::openai_models::default_input_modalities; use core_test_support::load_default_config_for_test; use indoc::indoc; use pretty_assertions::assert_eq; @@ -99,6 +100,7 @@ fn gpt_52_codex() -> ModelPreset { upgrade: None, show_in_picker: true, supported_in_api: true, + input_modalities: default_input_modalities(), } } @@ -142,6 +144,7 @@ fn gpt_5_1_codex_max() -> ModelPreset { )), show_in_picker: true, supported_in_api: true, + input_modalities: default_input_modalities(), } } @@ -177,6 +180,7 @@ fn gpt_5_1_codex_mini() -> ModelPreset { )), show_in_picker: true, supported_in_api: true, + input_modalities: default_input_modalities(), } } @@ -222,6 +226,7 @@ fn gpt_5_2() -> ModelPreset { )), show_in_picker: true, supported_in_api: true, + input_modalities: default_input_modalities(), } } @@ -255,6 +260,7 @@ fn bengalfox() -> ModelPreset { upgrade: None, show_in_picker: false, supported_in_api: true, + input_modalities: default_input_modalities(), } } @@ -288,6 +294,7 @@ fn boomslang() -> ModelPreset { upgrade: None, show_in_picker: false, supported_in_api: true, + input_modalities: default_input_modalities(), } } @@ -327,6 +334,7 @@ fn gpt_5_codex() -> ModelPreset { )), show_in_picker: false, supported_in_api: true, + input_modalities: default_input_modalities(), } } @@ -362,6 +370,7 @@ fn gpt_5_codex_mini() -> ModelPreset { )), show_in_picker: false, supported_in_api: true, + input_modalities: default_input_modalities(), } } @@ -401,6 +410,7 @@ fn gpt_5_1_codex() -> ModelPreset { )), show_in_picker: false, supported_in_api: true, + input_modalities: default_input_modalities(), } } @@ -444,6 +454,7 @@ fn gpt_5() -> ModelPreset { )), show_in_picker: false, supported_in_api: true, + input_modalities: default_input_modalities(), } } @@ -483,6 +494,7 @@ fn gpt_5_1() -> ModelPreset { )), show_in_picker: false, supported_in_api: true, + input_modalities: default_input_modalities(), } } diff --git a/codex-rs/core/tests/suite/models_cache_ttl.rs b/codex-rs/core/tests/suite/models_cache_ttl.rs index 5756f46a69..49d05b83e2 100644 --- a/codex-rs/core/tests/suite/models_cache_ttl.rs +++ b/codex-rs/core/tests/suite/models_cache_ttl.rs @@ -19,6 +19,7 @@ use codex_protocol::openai_models::ModelsResponse; use codex_protocol::openai_models::ReasoningEffort; use codex_protocol::openai_models::ReasoningEffortPreset; use codex_protocol::openai_models::TruncationPolicyConfig; +use codex_protocol::openai_models::default_input_modalities; use codex_protocol::user_input::UserInput; use core_test_support::responses; use core_test_support::responses::ev_assistant_message; @@ -36,6 +37,9 @@ use wiremock::MockServer; const ETAG: &str = "\"models-etag-ttl\""; const CACHE_FILE: &str = "models_cache.json"; const REMOTE_MODEL: &str = "codex-test-ttl"; +const VERSIONED_MODEL: &str = "codex-test-versioned"; +const MISSING_VERSION_MODEL: &str = "codex-test-missing-version"; +const DIFFERENT_VERSION_MODEL: &str = "codex-test-different-version"; #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn renews_cache_ttl_on_matching_models_etag() -> Result<()> { @@ -131,11 +135,157 @@ async fn renews_cache_ttl_on_matching_models_etag() -> Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn uses_cache_when_version_matches() -> Result<()> { + let server = MockServer::start().await; + let cached_model = test_remote_model(VERSIONED_MODEL, 1); + let models_mock = responses::mount_models_once( + &server, + ModelsResponse { + models: vec![test_remote_model("remote", 2)], + }, + ) + .await; + + let mut builder = test_codex().with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing()); + builder = builder + .with_pre_build_hook(move |home| { + let cache = ModelsCache { + fetched_at: Utc::now(), + etag: None, + client_version: Some(codex_core::models_manager::client_version_to_whole()), + models: vec![cached_model], + }; + let cache_path = home.join(CACHE_FILE); + write_cache_sync(&cache_path, &cache).expect("write cache"); + }) + .with_config(|config| { + config.features.enable(Feature::RemoteModels); + config.model_provider.request_max_retries = Some(0); + }); + + let test = builder.build(&server).await?; + let models_manager = test.thread_manager.get_models_manager(); + let models = models_manager + .list_models(&test.config, RefreshStrategy::OnlineIfUncached) + .await; + + assert!( + models.iter().any(|preset| preset.model == VERSIONED_MODEL), + "expected cached model" + ); + assert_eq!( + models_mock.requests().len(), + 0, + "/models should not be called when cache version matches" + ); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn refreshes_when_cache_version_missing() -> Result<()> { + let server = MockServer::start().await; + let cached_model = test_remote_model(MISSING_VERSION_MODEL, 1); + let models_mock = responses::mount_models_once( + &server, + ModelsResponse { + models: vec![test_remote_model("remote-missing", 2)], + }, + ) + .await; + + let mut builder = test_codex().with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing()); + builder = builder + .with_pre_build_hook(move |home| { + let cache = ModelsCache { + fetched_at: Utc::now(), + etag: None, + client_version: None, + models: vec![cached_model], + }; + let cache_path = home.join(CACHE_FILE); + write_cache_sync(&cache_path, &cache).expect("write cache"); + }) + .with_config(|config| { + config.features.enable(Feature::RemoteModels); + config.model_provider.request_max_retries = Some(0); + }); + + let test = builder.build(&server).await?; + let models_manager = test.thread_manager.get_models_manager(); + let models = models_manager + .list_models(&test.config, RefreshStrategy::OnlineIfUncached) + .await; + + assert!( + models.iter().any(|preset| preset.model == "remote-missing"), + "expected refreshed models" + ); + assert_eq!( + models_mock.requests().len(), + 1, + "/models should be called when cache version is missing" + ); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn refreshes_when_cache_version_differs() -> Result<()> { + let server = MockServer::start().await; + let cached_model = test_remote_model(DIFFERENT_VERSION_MODEL, 1); + let models_mock = responses::mount_models_once( + &server, + ModelsResponse { + models: vec![test_remote_model("remote-different", 2)], + }, + ) + .await; + + let mut builder = test_codex().with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing()); + builder = builder + .with_pre_build_hook(move |home| { + let client_version = codex_core::models_manager::client_version_to_whole(); + let cache = ModelsCache { + fetched_at: Utc::now(), + etag: None, + client_version: Some(format!("{client_version}-diff")), + models: vec![cached_model], + }; + let cache_path = home.join(CACHE_FILE); + write_cache_sync(&cache_path, &cache).expect("write cache"); + }) + .with_config(|config| { + config.features.enable(Feature::RemoteModels); + config.model_provider.request_max_retries = Some(0); + }); + + let test = builder.build(&server).await?; + let models_manager = test.thread_manager.get_models_manager(); + let models = models_manager + .list_models(&test.config, RefreshStrategy::OnlineIfUncached) + .await; + + assert!( + models + .iter() + .any(|preset| preset.model == "remote-different"), + "expected refreshed models" + ); + assert_eq!( + models_mock.requests().len(), + 1, + "/models should be called when cache version differs" + ); + + Ok(()) +} + async fn rewrite_cache_timestamp(path: &Path, fetched_at: DateTime) -> Result<()> { let mut cache = read_cache(path).await?; cache.fetched_at = fetched_at; - let contents = serde_json::to_vec_pretty(&cache)?; - tokio::fs::write(path, contents).await?; + write_cache(path, &cache).await?; Ok(()) } @@ -145,11 +295,25 @@ async fn read_cache(path: &Path) -> Result { Ok(cache) } +async fn write_cache(path: &Path, cache: &ModelsCache) -> Result<()> { + let contents = serde_json::to_vec_pretty(cache)?; + tokio::fs::write(path, contents).await?; + Ok(()) +} + +fn write_cache_sync(path: &Path, cache: &ModelsCache) -> Result<()> { + let contents = serde_json::to_vec_pretty(cache)?; + std::fs::write(path, contents)?; + Ok(()) +} + #[derive(Debug, Clone, Serialize, Deserialize)] struct ModelsCache { fetched_at: DateTime, #[serde(default)] etag: Option, + #[serde(default)] + client_version: Option, models: Vec, } @@ -186,5 +350,6 @@ fn test_remote_model(slug: &str, priority: i32) -> ModelInfo { auto_compact_token_limit: None, effective_context_window_percent: 95, experimental_supported_tools: Vec::new(), + input_modalities: default_input_modalities(), } } diff --git a/codex-rs/core/tests/suite/personality.rs b/codex-rs/core/tests/suite/personality.rs index 85cc25f4cd..87978ceb5a 100644 --- a/codex-rs/core/tests/suite/personality.rs +++ b/codex-rs/core/tests/suite/personality.rs @@ -16,6 +16,7 @@ use codex_protocol::openai_models::ModelsResponse; use codex_protocol::openai_models::ReasoningEffort; use codex_protocol::openai_models::ReasoningEffortPreset; use codex_protocol::openai_models::TruncationPolicyConfig; +use codex_protocol::openai_models::default_input_modalities; use codex_protocol::user_input::UserInput; use core_test_support::load_default_config_for_test; use core_test_support::responses::ev_completed; @@ -512,6 +513,7 @@ async fn ignores_remote_personality_if_remote_models_disabled() -> anyhow::Resul auto_compact_token_limit: None, effective_context_window_percent: 95, experimental_supported_tools: Vec::new(), + input_modalities: default_input_modalities(), }; let _models_mock = mount_models_once( @@ -627,6 +629,7 @@ async fn remote_model_friendly_personality_instructions_with_feature() -> anyhow auto_compact_token_limit: None, effective_context_window_percent: 95, experimental_supported_tools: Vec::new(), + input_modalities: default_input_modalities(), }; let _models_mock = mount_models_once( @@ -737,6 +740,7 @@ async fn user_turn_personality_remote_model_template_includes_update_message() - auto_compact_token_limit: None, effective_context_window_percent: 95, experimental_supported_tools: Vec::new(), + input_modalities: default_input_modalities(), }; let _models_mock = mount_models_once( diff --git a/codex-rs/core/tests/suite/remote_models.rs b/codex-rs/core/tests/suite/remote_models.rs index f59f73fd14..ed46855f9c 100644 --- a/codex-rs/core/tests/suite/remote_models.rs +++ b/codex-rs/core/tests/suite/remote_models.rs @@ -25,6 +25,7 @@ use codex_protocol::openai_models::ModelsResponse; use codex_protocol::openai_models::ReasoningEffort; use codex_protocol::openai_models::ReasoningEffortPreset; use codex_protocol::openai_models::TruncationPolicyConfig; +use codex_protocol::openai_models::default_input_modalities; use codex_protocol::user_input::UserInput; use core_test_support::load_default_config_for_test; use core_test_support::responses::ev_assistant_message; @@ -76,6 +77,7 @@ async fn remote_models_remote_model_uses_unified_exec() -> Result<()> { shell_type: ConfigShellToolType::UnifiedExec, visibility: ModelVisibility::List, supported_in_api: true, + input_modalities: default_input_modalities(), priority: 1, upgrade: None, base_instructions: "base instructions".to_string(), @@ -313,6 +315,7 @@ async fn remote_models_apply_remote_base_instructions() -> Result<()> { shell_type: ConfigShellToolType::ShellCommand, visibility: ModelVisibility::List, supported_in_api: true, + input_modalities: default_input_modalities(), priority: 1, upgrade: None, base_instructions: remote_base.to_string(), @@ -787,6 +790,7 @@ fn test_remote_model_with_policy( shell_type: ConfigShellToolType::ShellCommand, visibility, supported_in_api: true, + input_modalities: default_input_modalities(), priority, upgrade: None, base_instructions: "base instructions".to_string(), diff --git a/codex-rs/core/tests/suite/review.rs b/codex-rs/core/tests/suite/review.rs index cfc4a80488..1c9c3adf7b 100644 --- a/codex-rs/core/tests/suite/review.rs +++ b/codex-rs/core/tests/suite/review.rs @@ -530,6 +530,7 @@ async fn review_input_isolated_from_parent_history() { text: "parent: earlier user message".to_string(), }], end_turn: None, + phase: None, }; let user_json = serde_json::to_value(&user).unwrap(); let user_line = serde_json::json!({ @@ -549,6 +550,7 @@ async fn review_input_isolated_from_parent_history() { text: "parent: assistant reply".to_string(), }], end_turn: None, + phase: None, }; let assistant_json = serde_json::to_value(&assistant).unwrap(); let assistant_line = serde_json::json!({ diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index 07c8856fe6..74cc9a53f1 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -72,6 +72,13 @@ pub enum ContentItem { OutputText { text: String }, } +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, JsonSchema, TS)] +#[serde(rename_all = "snake_case")] +pub enum MessagePhase { + Commentary, + FinalAnswer, +} + #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, JsonSchema, TS)] #[serde(tag = "type", rename_all = "snake_case")] pub enum ResponseItem { @@ -85,6 +92,11 @@ pub enum ResponseItem { #[serde(default, skip_serializing_if = "Option::is_none")] #[ts(optional)] end_turn: Option, + // Optional output-message phase (for example: "commentary", "final_answer"). + // Do not use directly; availability can vary by provider and model. + #[serde(default, skip_serializing_if = "Option::is_none")] + #[ts(optional)] + phase: Option, }, Reasoning { #[serde(default, skip_serializing)] @@ -439,6 +451,7 @@ impl From for ResponseItem { text: di.into_text(), }], end_turn: None, + phase: None, } } } @@ -596,6 +609,7 @@ impl From for ResponseItem { content, id: None, end_turn: None, + phase: None, }, ResponseInputItem::FunctionCallOutput { call_id, output } => { Self::FunctionCallOutput { call_id, output } diff --git a/codex-rs/protocol/src/openai_models.rs b/codex-rs/protocol/src/openai_models.rs index 9a940539d5..90cf34f393 100644 --- a/codex-rs/protocol/src/openai_models.rs +++ b/codex-rs/protocol/src/openai_models.rs @@ -1,3 +1,8 @@ +//! Shared model metadata types exchanged between Codex services and clients. +//! +//! These types are serialized across core, TUI, app-server, and SDK boundaries, so field defaults +//! are used to preserve compatibility when older payloads omit newly introduced attributes. + use std::collections::HashMap; use std::collections::HashSet; @@ -43,6 +48,38 @@ pub enum ReasoningEffort { XHigh, } +/// Canonical user-input modality tags advertised by a model. +#[derive( + Debug, + Serialize, + Deserialize, + Clone, + Copy, + PartialEq, + Eq, + Display, + JsonSchema, + TS, + EnumIter, + Hash, +)] +#[serde(rename_all = "lowercase")] +#[strum(serialize_all = "lowercase")] +pub enum InputModality { + /// Plain text turns and tool payloads. + Text, + /// Image attachments included in user turns. + Image, +} + +/// Backward-compatible default when `input_modalities` is omitted on the wire. +/// +/// Legacy payloads predate modality metadata, so we conservatively assume both text and images are +/// accepted unless a preset explicitly narrows support. +pub fn default_input_modalities() -> Vec { + vec![InputModality::Text, InputModality::Image] +} + /// A reasoning effort option that can be surfaced for a model. #[derive(Debug, Clone, Deserialize, Serialize, TS, JsonSchema, PartialEq, Eq)] pub struct ReasoningEffortPreset { @@ -88,6 +125,9 @@ pub struct ModelPreset { pub show_in_picker: bool, /// whether this model is supported in the api pub supported_in_api: bool, + /// Input modalities accepted when composing user turns for this preset. + #[serde(default = "default_input_modalities")] + pub input_modalities: Vec, } /// Visibility of a model in the picker or APIs. @@ -206,6 +246,9 @@ pub struct ModelInfo { #[serde(default = "default_effective_context_window_percent")] pub effective_context_window_percent: i64, pub experimental_supported_tools: Vec, + /// Input modalities accepted by the backend for this model. + #[serde(default = "default_input_modalities")] + pub input_modalities: Vec, } impl ModelInfo { @@ -350,6 +393,7 @@ impl From for ModelPreset { }), show_in_picker: info.visibility == ModelVisibility::List, supported_in_api: info.supported_in_api, + input_modalities: info.input_modalities, } } } @@ -460,6 +504,7 @@ mod tests { auto_compact_token_limit: None, effective_context_window_percent: 95, experimental_supported_tools: vec![], + input_modalities: default_input_modalities(), } } diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 71a7973c6d..640d3c381e 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -589,13 +589,18 @@ impl SandboxPolicy { } subpaths.push(top_level_git); } - #[allow(clippy::expect_used)] - let top_level_codex = writable_root - .join(".codex") - .expect(".codex is a valid relative path"); - if top_level_codex.as_path().is_dir() { - subpaths.push(top_level_codex); + + // Make .agent/skills and .codex/config.toml and related + // files read-only to the agent, by default. + for subdir in &[".agent", ".codex"] { + #[allow(clippy::expect_used)] + let top_level_codex = + writable_root.join(subdir).expect("valid relative path"); + if top_level_codex.as_path().is_dir() { + subpaths.push(top_level_codex); + } } + WritableRoot { root: writable_root, read_only_subpaths: subpaths, @@ -1691,6 +1696,7 @@ impl From for ResponseItem { text: value.message, }], end_turn: None, + phase: None, } } } diff --git a/codex-rs/state/src/extract.rs b/codex-rs/state/src/extract.rs index 8d327a4d1a..ad40118086 100644 --- a/codex-rs/state/src/extract.rs +++ b/codex-rs/state/src/extract.rs @@ -151,6 +151,7 @@ mod tests { }, ], end_turn: None, + phase: None, }; let actual = extract_user_message_text(&item); assert_eq!(actual.as_deref(), Some("actual question")); diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index 068674b418..116e9c3abe 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -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) { 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, local_image_paths: Vec, + ) { + 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, + local_image_paths: Vec, + mention_paths: HashMap, ) { // 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); diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index 6d3a6755a4..cd5e2adb80 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -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) { 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, + local_image_paths: Vec, + mention_paths: HashMap, + ) { + 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, diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index cc99aef7cf..ce4af0d2ff 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -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 = 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, + local_images: Vec, + mention_paths: HashMap, + ) { + // 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 { diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index ea0cd43636..00141a0963 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -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); diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index a91b62a754..49ccc481bf 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -2147,6 +2147,7 @@ impl HistoryCell for FinalMessageSeparator { let mut label_parts = Vec::new(); if let Some(elapsed_seconds) = self .elapsed_seconds + .filter(|seconds| *seconds > 60) .map(super::status_indicator_widget::fmt_elapsed_compact) { label_parts.push(format!("Worked for {elapsed_seconds}")); @@ -2358,7 +2359,7 @@ mod tests { } #[test] - fn final_message_separator_includes_runtime_metrics() { + fn final_message_separator_hides_short_worked_label_and_includes_runtime_metrics() { let summary = RuntimeMetricsSummary { tool_calls: RuntimeMetricTotals { count: 3, @@ -2385,7 +2386,7 @@ mod tests { let rendered = render_lines(&cell.display_lines(200)); assert_eq!(rendered.len(), 1); - assert!(rendered[0].contains("Worked for 12s")); + assert!(!rendered[0].contains("Worked for")); assert!(rendered[0].contains("Local tools: 3 calls (2.5s)")); assert!(rendered[0].contains("Inference: 2 calls (1.2s)")); assert!(rendered[0].contains("WebSocket: 1 events send (700ms)")); @@ -2393,6 +2394,15 @@ mod tests { assert!(rendered[0].contains("4 events received (1.2s)")); } + #[test] + fn final_message_separator_includes_worked_label_after_one_minute() { + let cell = FinalMessageSeparator::new(Some(61), None); + let rendered = render_lines(&cell.display_lines(200)); + + assert_eq!(rendered.len(), 1); + assert!(rendered[0].contains("Worked for")); + } + #[test] fn ps_output_empty_snapshot() { let cell = new_unified_exec_processes_output(Vec::new()); diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index dc29030434..862f93eacf 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -114,7 +114,6 @@ mod wrapping; #[cfg(test)] pub mod test_backend; -use crate::onboarding::TrustDirectorySelection; use crate::onboarding::onboarding_screen::OnboardingScreenArgs; use crate::onboarding::onboarding_screen::run_onboarding_app; use crate::tui::Tui; @@ -494,12 +493,9 @@ async fn run_ratatui_app( exit_reason: ExitReason::UserRequested, }); } - // if the user acknowledged windows or made an explicit decision ato trust the directory, reload the config accordingly - if onboarding_result - .directory_trust_decision - .map(|d| d == TrustDirectorySelection::Trust) - .unwrap_or(false) - { + // If the user made an explicit trust decision, reload config so current + // process state reflects what was persisted to config.toml. + if onboarding_result.directory_trust_decision.is_some() { load_config_or_exit( cli_kv_overrides.clone(), overrides.clone(), diff --git a/codex-rs/tui/src/tooltips.rs b/codex-rs/tui/src/tooltips.rs index b59a478a4d..b62831e7e6 100644 --- a/codex-rs/tui/src/tooltips.rs +++ b/codex-rs/tui/src/tooltips.rs @@ -6,10 +6,9 @@ use rand::Rng; const ANNOUNCEMENT_TIP_URL: &str = "https://raw.githubusercontent.com/openai/codex/main/announcement_tip.toml"; -const PAID_TOOLTIP: &str = - "*New* Try the **Codex App** with 2x rate limits until *April 2nd*. https://chatgpt.com/codex"; +const PAID_TOOLTIP: &str = "*New* Try the **Codex App** with 2x rate limits until *April 2nd*. Run 'codex app' or visit https://chatgpt.com/codex"; const OTHER_TOOLTIP: &str = - "*New* Build faster with the **Codex App**. Try it now. https://chatgpt.com/codex"; + "*New* Build faster with the **Codex App**. Run 'codex app' or visit https://chatgpt.com/codex"; const FREE_GO_TOOLTIP: &str = "*New* Codex is included in your plan for free through *March 2nd* – let’s build together."; diff --git a/docs/contributing.md b/docs/contributing.md index d410123d2d..19b31073e9 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -41,6 +41,15 @@ If you are invited by a Codex team member to contribute a PR, here is the recomm 3. **Document behavior.** If your change affects user-facing behavior, update the README, inline help (`codex --help`), or relevant example projects. 4. **Keep commits atomic.** Each commit should compile and the tests should pass. This makes reviews and potential rollbacks easier. +### Model metadata updates + +When a change updates model catalogs or model metadata (`/models` payloads, presets, or fixtures): + +- Set `input_modalities` explicitly for any model that does not support images. +- Keep compatibility defaults in mind: omitted `input_modalities` currently implies text + image support. +- Ensure client surfaces that accept images (for example, TUI paste/attach) consume the same capability signal. +- Add/update tests that cover unsupported-image behavior and warning paths. + ### Opening a pull request (by invitation only) - Fill in the PR template (or include similar information) - **What? Why? How?** diff --git a/docs/tui-chat-composer.md b/docs/tui-chat-composer.md index 6211cbcfce..b927e2db44 100644 --- a/docs/tui-chat-composer.md +++ b/docs/tui-chat-composer.md @@ -84,6 +84,9 @@ Key effects when disabled: `prepare_submission_text`. - When `slash_commands_enabled` is `false`, slash-context paste-burst exceptions are disabled. - When `image_paste_enabled` is `false`, file-path paste image attachment is skipped. +- `ChatWidget` may toggle `image_paste_enabled` at runtime based on the selected model's + `input_modalities`; attach and submit paths also re-check support and emit a warning instead of + dropping the draft. Built-in slash command availability is centralized in `codex-rs/tui/src/bottom_pane/slash_commands.rs` and reused by both the composer and the command @@ -252,6 +255,11 @@ Non-char input must not leak burst state across unrelated actions: inserting, deleting, flushing a burst, applying a paste placeholder, etc. - Shortcut overlay toggling via `?` is gated on `!is_in_paste_burst()` so pastes cannot flip UI modes while streaming. +- Mention popup selection has two payloads: visible `$name` text and hidden + `mention_paths[name] -> canonical target` linkage. The generic + `set_text_content` path intentionally clears linkage for fresh drafts; restore + paths that rehydrate blocked/interrupted submissions must use the + mention-preserving setter so retry keeps the originally selected target. ## Tests that pin behavior