mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Route standalone image generation through host finalization md (#25176)
## Why Standalone image-generation extensions emitted turn items through the low-level event path, bypassing host-owned finalization such as image persistence and contributor processing. At the same time, the generated-image save-path hint must remain visible to the model through the extension tool's `FunctionCallOutput`, rather than the legacy built-in developer-message path. ## What changed - Extended `ExtensionTurnItem` to support image-generation items while keeping the extension-facing emitter API limited to `emit_started` and `emit_completed`. - Routed extension completion through core `finalize_turn_item`, so standalone image-generation items receive host-owned processing and persisted `saved_path` values before publication. - Kept legacy built-in image generation on its existing developer-message hint path, while standalone image generation returns its deterministic saved-path hint in `FunctionCallOutput`. - Shared the image artifact path and output-hint formatting used by core and the image-generation extension. - Passed thread identity through extension tool calls so standalone image generation can construct the same intended artifact path as core. - Added an app-server integration test covering real standalone image generation, saved artifact publication, model-visible output hint wiring, and absence of the legacy developer-message hint. ## Validation - `just fmt` - `just test -p codex-image-generation-extension` - `just test -p codex-web-search-extension` - `just test -p codex-goal-extension` - `just test -p codex-memories-extension` - Targeted `codex-core` tests for image save history, extension completion finalization, and contributor execution - `just test -p codex-app-server standalone_image_generation_returns_saved_path_hint_to_model` - `just fix -p codex-core` - `just fix -p codex-image-generation-extension` - `just bazel-lock-update` - `just bazel-lock-check`
This commit is contained in:
@@ -131,6 +131,7 @@ pub(crate) async fn persist_image_generation_item(
|
||||
turn_context: &TurnContext,
|
||||
image_item: &mut ImageGenerationItem,
|
||||
) -> Option<AbsolutePathBuf> {
|
||||
image_item.saved_path = None;
|
||||
let session_id = sess.conversation_id.to_string();
|
||||
match save_image_generation_result(
|
||||
&turn_context.config.codex_home,
|
||||
@@ -163,15 +164,12 @@ pub(crate) async fn persist_image_generation_item(
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) async fn finalize_image_generation_item(
|
||||
async fn record_image_generation_instructions(
|
||||
sess: &Session,
|
||||
turn_context: &TurnContext,
|
||||
image_item: &mut ImageGenerationItem,
|
||||
image_item: &ImageGenerationItem,
|
||||
) {
|
||||
if persist_image_generation_item(sess, turn_context, image_item)
|
||||
.await
|
||||
.is_none()
|
||||
{
|
||||
if image_item.saved_path.is_none() {
|
||||
return;
|
||||
}
|
||||
let session_id = sess.conversation_id.to_string();
|
||||
@@ -530,27 +528,16 @@ pub(crate) async fn handle_non_tool_response_item(
|
||||
| ResponseItem::WebSearchCall { .. }
|
||||
| ResponseItem::ImageGenerationCall { .. } => {
|
||||
let mut turn_item = parse_turn_item(item)?;
|
||||
if let TurnItemContributorPolicy::Run(turn_store) = contributor_policy {
|
||||
apply_turn_item_contributors(sess, turn_store, &mut turn_item).await;
|
||||
}
|
||||
if let TurnItem::AgentMessage(agent_message) = &mut turn_item {
|
||||
let combined = agent_message
|
||||
.content
|
||||
.iter()
|
||||
.map(|entry| match entry {
|
||||
codex_protocol::items::AgentMessageContent::Text { text } => text.as_str(),
|
||||
})
|
||||
.collect::<String>();
|
||||
let (stripped, memory_citation) =
|
||||
strip_hidden_assistant_markup_and_parse_memory_citation(&combined, plan_mode);
|
||||
agent_message.content =
|
||||
vec![codex_protocol::items::AgentMessageContent::Text { text: stripped }];
|
||||
if agent_message.memory_citation.is_none() {
|
||||
agent_message.memory_citation = memory_citation;
|
||||
}
|
||||
}
|
||||
if let TurnItem::ImageGeneration(image_item) = &mut turn_item {
|
||||
finalize_image_generation_item(sess, turn_context, image_item).await;
|
||||
finalize_turn_item(
|
||||
sess,
|
||||
turn_context,
|
||||
contributor_policy,
|
||||
&mut turn_item,
|
||||
plan_mode,
|
||||
)
|
||||
.await;
|
||||
if let TurnItem::ImageGeneration(image_item) = &turn_item {
|
||||
record_image_generation_instructions(sess, turn_context, image_item).await;
|
||||
}
|
||||
Some(turn_item)
|
||||
}
|
||||
@@ -564,6 +551,37 @@ pub(crate) async fn handle_non_tool_response_item(
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) async fn finalize_turn_item(
|
||||
sess: &Session,
|
||||
turn_context: &TurnContext,
|
||||
contributor_policy: TurnItemContributorPolicy<'_>,
|
||||
turn_item: &mut TurnItem,
|
||||
plan_mode: bool,
|
||||
) {
|
||||
if let TurnItemContributorPolicy::Run(turn_store) = contributor_policy {
|
||||
apply_turn_item_contributors(sess, turn_store, turn_item).await;
|
||||
}
|
||||
if let TurnItem::AgentMessage(agent_message) = &mut *turn_item {
|
||||
let combined = agent_message
|
||||
.content
|
||||
.iter()
|
||||
.map(|entry| match entry {
|
||||
codex_protocol::items::AgentMessageContent::Text { text } => text.as_str(),
|
||||
})
|
||||
.collect::<String>();
|
||||
let (stripped, memory_citation) =
|
||||
strip_hidden_assistant_markup_and_parse_memory_citation(&combined, plan_mode);
|
||||
agent_message.content =
|
||||
vec![codex_protocol::items::AgentMessageContent::Text { text: stripped }];
|
||||
if agent_message.memory_citation.is_none() {
|
||||
agent_message.memory_citation = memory_citation;
|
||||
}
|
||||
}
|
||||
if let TurnItem::ImageGeneration(image_item) = &mut *turn_item {
|
||||
persist_image_generation_item(sess, turn_context, image_item).await;
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn last_assistant_message_from_item(
|
||||
item: &ResponseItem,
|
||||
plan_mode: bool,
|
||||
|
||||
@@ -4,7 +4,6 @@ use std::sync::Weak;
|
||||
use codex_protocol::items::TurnItem;
|
||||
use codex_tools::ConversationHistory;
|
||||
use codex_tools::ExtensionTurnItem;
|
||||
use codex_tools::ImageGenerationCompletionFuture;
|
||||
use codex_tools::ToolCall as ExtensionToolCall;
|
||||
use codex_tools::ToolName;
|
||||
use codex_tools::ToolSearchInfo;
|
||||
@@ -12,12 +11,11 @@ use codex_tools::ToolSpec;
|
||||
use codex_tools::TurnItemEmissionFuture;
|
||||
use codex_tools::TurnItemEmitter;
|
||||
|
||||
use crate::context::ContextualUserFragment;
|
||||
use crate::context::ImageGenerationInstructions;
|
||||
use crate::function_tool::FunctionCallError;
|
||||
use crate::session::session::Session;
|
||||
use crate::session::turn_context::TurnContext;
|
||||
use crate::stream_events_utils::persist_image_generation_item;
|
||||
use crate::stream_events_utils::TurnItemContributorPolicy;
|
||||
use crate::stream_events_utils::finalize_turn_item;
|
||||
use crate::tools::context::ToolInvocation;
|
||||
use crate::tools::context::ToolOutput;
|
||||
use crate::tools::context::ToolPayload;
|
||||
@@ -76,6 +74,10 @@ struct CoreTurnItemEmitter {
|
||||
fn extension_turn_item(item: ExtensionTurnItem) -> TurnItem {
|
||||
match item {
|
||||
ExtensionTurnItem::WebSearch(item) => TurnItem::WebSearch(item),
|
||||
ExtensionTurnItem::ImageGeneration(mut item) => {
|
||||
item.saved_path = None;
|
||||
TurnItem::ImageGeneration(item)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -85,8 +87,9 @@ impl TurnItemEmitter for CoreTurnItemEmitter {
|
||||
let (Some(session), Some(turn)) = (self.session.upgrade(), self.turn.upgrade()) else {
|
||||
return;
|
||||
};
|
||||
let item = extension_turn_item(item);
|
||||
session.emit_turn_item_started(turn.as_ref(), &item).await;
|
||||
session
|
||||
.emit_turn_item_started(turn.as_ref(), &extension_turn_item(item))
|
||||
.await;
|
||||
})
|
||||
}
|
||||
|
||||
@@ -95,54 +98,18 @@ impl TurnItemEmitter for CoreTurnItemEmitter {
|
||||
let (Some(session), Some(turn)) = (self.session.upgrade(), self.turn.upgrade()) else {
|
||||
return;
|
||||
};
|
||||
let item = extension_turn_item(item);
|
||||
let mut item = extension_turn_item(item);
|
||||
finalize_turn_item(
|
||||
session.as_ref(),
|
||||
turn.as_ref(),
|
||||
TurnItemContributorPolicy::Run(turn.extension_data.as_ref()),
|
||||
&mut item,
|
||||
turn.collaboration_mode.mode == codex_protocol::config_types::ModeKind::Plan,
|
||||
)
|
||||
.await;
|
||||
session.emit_turn_item_completed(turn.as_ref(), item).await;
|
||||
})
|
||||
}
|
||||
|
||||
fn image_generation_completed<'a>(
|
||||
&'a self,
|
||||
call_id: String,
|
||||
prompt: String,
|
||||
result: String,
|
||||
) -> ImageGenerationCompletionFuture<'a> {
|
||||
Box::pin(async move {
|
||||
let (Some(session), Some(turn)) = (self.session.upgrade(), self.turn.upgrade()) else {
|
||||
return None;
|
||||
};
|
||||
let mut item = codex_protocol::items::ImageGenerationItem {
|
||||
id: call_id,
|
||||
status: "completed".to_string(),
|
||||
revised_prompt: Some(prompt),
|
||||
result,
|
||||
saved_path: None,
|
||||
};
|
||||
let output_hint =
|
||||
persist_image_generation_item(session.as_ref(), turn.as_ref(), &mut item)
|
||||
.await
|
||||
.map(|saved_path| {
|
||||
let output_dir = saved_path
|
||||
.parent()
|
||||
.unwrap_or_else(|| turn.config.codex_home.clone());
|
||||
ImageGenerationInstructions::new(output_dir.display(), saved_path.display())
|
||||
.body()
|
||||
});
|
||||
let started_item = codex_protocol::items::ImageGenerationItem {
|
||||
id: item.id.clone(),
|
||||
status: "in_progress".to_string(),
|
||||
revised_prompt: None,
|
||||
result: String::new(),
|
||||
saved_path: None,
|
||||
};
|
||||
session
|
||||
.emit_turn_item_started(turn.as_ref(), &TurnItem::ImageGeneration(started_item))
|
||||
.await;
|
||||
session
|
||||
.emit_turn_item_completed(turn.as_ref(), TurnItem::ImageGeneration(item))
|
||||
.await;
|
||||
output_hint
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
async fn to_extension_call(invocation: &ToolInvocation) -> ExtensionToolCall {
|
||||
@@ -167,6 +134,8 @@ async fn to_extension_call(invocation: &ToolInvocation) -> ExtensionToolCall {
|
||||
mod tests {
|
||||
use std::sync::Arc;
|
||||
|
||||
use codex_extension_api::ExtensionData;
|
||||
use codex_extension_api::TurnItemContributor;
|
||||
use codex_protocol::items::TurnItem;
|
||||
use codex_protocol::items::WebSearchItem;
|
||||
use codex_protocol::models::ContentItem;
|
||||
@@ -174,10 +143,13 @@ mod tests {
|
||||
use codex_protocol::models::WebSearchAction;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_tools::ExtensionTurnItem;
|
||||
use codex_utils_absolute_path::test_support::PathExt;
|
||||
use codex_utils_absolute_path::test_support::test_path_buf;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
use tokio::sync::Mutex;
|
||||
|
||||
use super::CoreTurnItemEmitter;
|
||||
use super::ExtensionToolAdapter;
|
||||
use crate::tools::context::ToolCallSource;
|
||||
use crate::tools::context::ToolInvocation;
|
||||
@@ -409,8 +381,54 @@ mod tests {
|
||||
assert_eq!(end.action, expected.action);
|
||||
}
|
||||
|
||||
struct ImageGenerationExtensionExecutor {
|
||||
output_hint: Arc<Mutex<Option<String>>>,
|
||||
struct ImageGenerationExtensionExecutor;
|
||||
|
||||
#[derive(Debug)]
|
||||
struct ExtensionTurnItemContributorRan;
|
||||
|
||||
struct RecordExtensionTurnItemContributor;
|
||||
|
||||
#[async_trait::async_trait]
|
||||
impl TurnItemContributor for RecordExtensionTurnItemContributor {
|
||||
async fn contribute(
|
||||
&self,
|
||||
_thread_store: &ExtensionData,
|
||||
turn_store: &ExtensionData,
|
||||
_item: &mut TurnItem,
|
||||
) -> Result<(), String> {
|
||||
turn_store.insert(ExtensionTurnItemContributorRan);
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn extension_completion_runs_turn_item_contributors() {
|
||||
let (mut session, turn) = crate::session::tests::make_session_and_context().await;
|
||||
let mut builder = codex_extension_api::ExtensionRegistryBuilder::new();
|
||||
builder.turn_item_contributor(Arc::new(RecordExtensionTurnItemContributor));
|
||||
session.services.extensions = Arc::new(builder.build());
|
||||
let session = Arc::new(session);
|
||||
let turn = Arc::new(turn);
|
||||
let emitter = CoreTurnItemEmitter {
|
||||
session: Arc::downgrade(&session),
|
||||
turn: Arc::downgrade(&turn),
|
||||
};
|
||||
|
||||
codex_tools::TurnItemEmitter::emit_completed(
|
||||
&emitter,
|
||||
ExtensionTurnItem::WebSearch(WebSearchItem {
|
||||
id: "search-1".to_string(),
|
||||
query: "contributors".to_string(),
|
||||
action: WebSearchAction::Other,
|
||||
}),
|
||||
)
|
||||
.await;
|
||||
|
||||
assert!(
|
||||
turn.extension_data
|
||||
.get::<ExtensionTurnItemContributorRan>()
|
||||
.is_some()
|
||||
);
|
||||
}
|
||||
|
||||
#[async_trait::async_trait]
|
||||
@@ -434,15 +452,28 @@ mod tests {
|
||||
&self,
|
||||
call: codex_tools::ToolCall,
|
||||
) -> Result<Box<dyn codex_tools::ToolOutput>, codex_tools::FunctionCallError> {
|
||||
let output_hint = call
|
||||
.turn_item_emitter
|
||||
.image_generation_completed(
|
||||
call.call_id,
|
||||
"A tiny blue square".to_string(),
|
||||
"cG5n".to_string(),
|
||||
)
|
||||
call.turn_item_emitter
|
||||
.emit_started(ExtensionTurnItem::ImageGeneration(
|
||||
codex_protocol::items::ImageGenerationItem {
|
||||
id: call.call_id.clone(),
|
||||
status: "in_progress".to_string(),
|
||||
revised_prompt: None,
|
||||
result: String::new(),
|
||||
saved_path: None,
|
||||
},
|
||||
))
|
||||
.await;
|
||||
call.turn_item_emitter
|
||||
.emit_completed(ExtensionTurnItem::ImageGeneration(
|
||||
codex_protocol::items::ImageGenerationItem {
|
||||
id: call.call_id,
|
||||
status: "completed".to_string(),
|
||||
revised_prompt: Some("A tiny blue square".to_string()),
|
||||
result: "cG5n".to_string(),
|
||||
saved_path: Some(test_path_buf("/tmp/extension-claimed.png").abs()),
|
||||
},
|
||||
))
|
||||
.await;
|
||||
*self.output_hint.lock().await = output_hint;
|
||||
Ok(Box::new(codex_tools::JsonToolOutput::new(
|
||||
json!({ "ok": true }),
|
||||
)))
|
||||
@@ -451,10 +482,7 @@ mod tests {
|
||||
|
||||
#[tokio::test]
|
||||
async fn image_generation_publication_is_finalized_by_core() {
|
||||
let output_hint = Arc::new(Mutex::new(None));
|
||||
let handler = ExtensionToolAdapter::new(Arc::new(ImageGenerationExtensionExecutor {
|
||||
output_hint: Arc::clone(&output_hint),
|
||||
}));
|
||||
let handler = ExtensionToolAdapter::new(Arc::new(ImageGenerationExtensionExecutor));
|
||||
let (session, turn, rx) = crate::session::tests::make_session_and_context_with_rx().await;
|
||||
let expected_path = crate::stream_events_utils::image_generation_artifact_path(
|
||||
&turn.config.codex_home,
|
||||
@@ -521,17 +549,5 @@ mod tests {
|
||||
std::fs::read(&expected_path).expect("generated artifact should be saved"),
|
||||
b"png"
|
||||
);
|
||||
assert_eq!(
|
||||
*output_hint.lock().await,
|
||||
Some(format!(
|
||||
"Generated images are saved to {} as {} by default.\n\
|
||||
If you need to use a generated image at another path, copy it and leave the original in place unless the user explicitly asks you to delete it.",
|
||||
expected_path
|
||||
.parent()
|
||||
.expect("generated image path should have a parent")
|
||||
.display(),
|
||||
expected_path.display(),
|
||||
))
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user