Skip to content

Commit 3f055d6

Browse files
authored
修复 Codex 图片生成 ACP 输出阻塞 (#295)
## 变更说明 - 在 ACP ToolCallUpdate 翻译边界清洗 Codex 图片生成的大型 inline base64,只保留 saved_path/image.path 等小型结构化字段。 - 图片已落盘时把工具状态归一为 completed,并同步 raw_output.status,避免会话一直显示执行中。 - 增加翻译层和 stream relay 回归测试,覆盖 base64 省略、状态完成、MIME 推断和持久化内容不再携带大字段。 - 更新中文架构文档和实施计划文档。 ## 关联 Issue Closes #297 ## 验证 - cargo fmt --check - cargo test -p aionui-ai-agent codex_image_tool_update -- --test-threads=1 - cargo test -p aionui-conversation run_acp_image_tool_call_update_persists_finish_without_base64 - git diff --check
1 parent eea941e commit 3f055d6

4 files changed

Lines changed: 389 additions & 2 deletions

File tree

ARCHITECTURE.zh-CN.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,19 @@ async fn handler(
256256
新增事件必须遵循上述两级 camelCase 规范,
257257
现有不一致的事件在相关模块迭代时逐步统一。
258258

259+
### ACP 工具输出清洗
260+
261+
ACP Agent 的工具调用事件在 `aionui-ai-agent` 翻译层进入统一
262+
`AgentStreamEvent`。该边界必须保证 WebSocket 和 SQLite 消息内容
263+
是可控大小,不能把工具返回的大型二进制或 inline base64 原样透传。
264+
265+
Codex 图片生成工具可能同时返回 `saved_path``raw_output.result`
266+
中的 PNG/JPEG/WebP base64。翻译层会在转发和持久化之前移除
267+
`result`,保留 `saved_path``image.path``result_omitted`
268+
`result_bytes` 等小型结构化字段;如果图片已经落盘,则把工具状态
269+
归一化为 `completed`。前端应通过文件路径按需加载图片,不应依赖
270+
inline base64。
271+
259272
## 数据层
260273

261274
### Repository Trait 模式

crates/aionui-ai-agent/src/protocol/events/mod.rs

Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,182 @@ mod tests {
328328
assert!(json["data"]["update"].get("rawInput").is_none());
329329
}
330330

331+
#[test]
332+
fn codex_image_tool_update_omits_base64_result() {
333+
let large_png_base64 = format!("iVBORw0KGgo{}", "A".repeat(128 * 1024));
334+
let notif = SessionNotification::new(
335+
"sess-1",
336+
SessionUpdate::ToolCallUpdate(SdkToolCallUpdate::new(
337+
"ig_test_image",
338+
ToolCallUpdateFields::new()
339+
.status(SdkToolCallStatus::InProgress)
340+
.raw_output(json!({
341+
"call_id": "ig_test_image",
342+
"status": "generating",
343+
"saved_path": "/Users/test/.codex/generated_images/session/ig_test_image.png",
344+
"revised_prompt": "一只小猫",
345+
"result": large_png_base64
346+
})),
347+
)),
348+
);
349+
350+
let events = session_notification_to_events(&notif);
351+
assert_eq!(events.len(), 1);
352+
let json = serde_json::to_value(&events[0]).unwrap();
353+
let raw_output = &json["data"]["update"]["rawOutput"];
354+
355+
assert_eq!(
356+
raw_output["saved_path"],
357+
"/Users/test/.codex/generated_images/session/ig_test_image.png"
358+
);
359+
assert_eq!(
360+
raw_output["image"]["path"],
361+
"/Users/test/.codex/generated_images/session/ig_test_image.png"
362+
);
363+
assert_eq!(raw_output["result_omitted"], true);
364+
assert!(raw_output.get("result").is_none());
365+
}
366+
367+
#[test]
368+
fn codex_image_tool_update_with_saved_path_is_completed() {
369+
let notif = SessionNotification::new(
370+
"sess-1",
371+
SessionUpdate::ToolCallUpdate(SdkToolCallUpdate::new(
372+
"ig_done_image",
373+
ToolCallUpdateFields::new()
374+
.status(SdkToolCallStatus::InProgress)
375+
.raw_output(json!({
376+
"call_id": "ig_done_image",
377+
"status": "generating",
378+
"saved_path": "/Users/test/.codex/generated_images/session/ig_done_image.png",
379+
"result": format!("iVBORw0KGgo{}", "A".repeat(128 * 1024))
380+
})),
381+
)),
382+
);
383+
384+
let events = session_notification_to_events(&notif);
385+
assert_eq!(events.len(), 1);
386+
let json = serde_json::to_value(&events[0]).unwrap();
387+
388+
assert_eq!(json["data"]["update"]["status"], "completed");
389+
assert_eq!(json["data"]["update"]["rawOutput"]["status"], "completed");
390+
}
391+
392+
#[test]
393+
fn codex_image_tool_update_keeps_small_text_result_in_progress() {
394+
let notif = SessionNotification::new(
395+
"sess-1",
396+
SessionUpdate::ToolCallUpdate(SdkToolCallUpdate::new(
397+
"small-result",
398+
ToolCallUpdateFields::new()
399+
.status(SdkToolCallStatus::InProgress)
400+
.raw_output(json!({
401+
"status": "generating",
402+
"saved_path": "/tmp/result.txt",
403+
"result": "not an inline image"
404+
})),
405+
)),
406+
);
407+
408+
let events = session_notification_to_events(&notif);
409+
let json = serde_json::to_value(&events[0]).unwrap();
410+
let raw_output = &json["data"]["update"]["rawOutput"];
411+
412+
assert_eq!(json["data"]["update"]["status"], "in_progress");
413+
assert_eq!(raw_output["result"], "not an inline image");
414+
assert!(raw_output.get("image").is_none());
415+
assert!(raw_output.get("result_omitted").is_none());
416+
}
417+
418+
#[test]
419+
fn codex_image_tool_update_detects_image_mime_types_from_saved_path() {
420+
let cases = [
421+
("photo.jpg", "image/jpeg", "/9j/"),
422+
("photo.webp", "image/webp", "UklGR"),
423+
("photo.gif", "image/gif", "data:image/gif;base64,"),
424+
("photo.bin", "image/png", "iVBORw0KGgo"),
425+
];
426+
427+
for (file_name, expected_mime, prefix) in cases {
428+
let saved_path = format!("/Users/test/.codex/generated_images/session/{file_name}");
429+
let notif = SessionNotification::new(
430+
"sess-1",
431+
SessionUpdate::ToolCallUpdate(SdkToolCallUpdate::new(
432+
"ig_mime",
433+
ToolCallUpdateFields::new().raw_output(json!({
434+
"saved_path": saved_path,
435+
"result": format!("{prefix}{}", "A".repeat(128 * 1024))
436+
})),
437+
)),
438+
);
439+
440+
let events = session_notification_to_events(&notif);
441+
let json = serde_json::to_value(&events[0]).unwrap();
442+
443+
assert_eq!(json["data"]["update"]["rawOutput"]["image"]["mime_type"], expected_mime);
444+
assert!(json["data"]["update"]["rawOutput"].get("result").is_none());
445+
}
446+
}
447+
448+
#[test]
449+
fn codex_image_tool_update_omits_base64_without_saved_path() {
450+
let large_png_base64 = format!("iVBORw0KGgo{}", "A".repeat(128 * 1024));
451+
let notif = SessionNotification::new(
452+
"sess-1",
453+
SessionUpdate::ToolCallUpdate(SdkToolCallUpdate::new(
454+
"ig_no_path",
455+
ToolCallUpdateFields::new()
456+
.status(SdkToolCallStatus::InProgress)
457+
.raw_output(json!({
458+
"call_id": "ig_no_path",
459+
"status": "generating",
460+
"result": large_png_base64
461+
})),
462+
)),
463+
);
464+
465+
let events = session_notification_to_events(&notif);
466+
assert_eq!(events.len(), 1);
467+
let json = serde_json::to_value(&events[0]).unwrap();
468+
let raw_output = &json["data"]["update"]["rawOutput"];
469+
470+
// Oversized base64 must be stripped even though Codex did not save the file.
471+
assert!(raw_output.get("result").is_none());
472+
assert_eq!(raw_output["result_omitted"], true);
473+
// No saved_path means we cannot offer a path-based preview, so no image object.
474+
assert!(raw_output.get("image").is_none());
475+
// Without a saved image the status must pass through unchanged.
476+
assert_eq!(json["data"]["update"]["status"], "in_progress");
477+
}
478+
479+
#[test]
480+
fn codex_image_tool_update_preserves_failed_status() {
481+
let notif = SessionNotification::new(
482+
"sess-1",
483+
SessionUpdate::ToolCallUpdate(SdkToolCallUpdate::new(
484+
"ig_failed_image",
485+
ToolCallUpdateFields::new()
486+
.status(SdkToolCallStatus::Failed)
487+
.raw_output(json!({
488+
"call_id": "ig_failed_image",
489+
"status": "failed",
490+
"saved_path": "/Users/test/.codex/generated_images/session/ig_failed_image.png",
491+
"result": format!("iVBORw0KGgo{}", "A".repeat(128 * 1024))
492+
})),
493+
)),
494+
);
495+
496+
let events = session_notification_to_events(&notif);
497+
assert_eq!(events.len(), 1);
498+
let json = serde_json::to_value(&events[0]).unwrap();
499+
500+
// A terminal `failed` status must never be rewritten to `completed`.
501+
assert_eq!(json["data"]["update"]["status"], "failed");
502+
assert_eq!(json["data"]["update"]["rawOutput"]["status"], "failed");
503+
// The base64 payload is still stripped regardless of the failure.
504+
assert!(json["data"]["update"]["rawOutput"].get("result").is_none());
505+
}
506+
331507
#[test]
332508
fn permission_request_maps_to_snake_case_event_data() {
333509
let request = RequestPermissionRequest::new(

crates/aionui-ai-agent/src/protocol/events/translate.rs

Lines changed: 112 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,16 +63,20 @@ pub(crate) fn session_notification_to_events(notif: &SessionNotification) -> Vec
6363
}
6464

6565
SessionUpdate::ToolCallUpdate(tcu) => {
66+
let mut raw_output = sanitize_raw_output(tcu.fields.raw_output.clone());
67+
let status = normalize_tool_status(tcu.fields.status.as_ref(), raw_output.as_ref());
68+
normalize_raw_output_status(&mut raw_output, status.as_ref());
69+
6670
events.push(AgentStreamEvent::AcpToolCall(AcpToolCallEventData {
6771
session_id,
6872
update: AcpToolCallUpdateData {
6973
session_update: AcpToolCallSessionUpdateKind::ToolCallUpdate,
7074
tool_call_id: tcu.tool_call_id.to_string(),
71-
status: tcu.fields.status.as_ref().map(map_sdk_tool_status),
75+
status,
7276
title: tcu.fields.title.clone(),
7377
kind: tcu.fields.kind.as_ref().map(map_sdk_tool_kind),
7478
raw_input: tcu.fields.raw_input.clone(),
75-
raw_output: tcu.fields.raw_output.clone(),
79+
raw_output,
7680
content: tcu
7781
.fields
7882
.content
@@ -157,6 +161,112 @@ fn map_sdk_tool_status(sdk: &SdkToolCallStatus) -> AcpToolCallStatus {
157161
}
158162
}
159163

164+
const ACP_RAW_OUTPUT_INLINE_IMAGE_LIMIT: usize = 64 * 1024;
165+
166+
fn sanitize_raw_output(raw_output: Option<serde_json::Value>) -> Option<serde_json::Value> {
167+
let mut value = raw_output?;
168+
sanitize_inline_image_result(&mut value);
169+
Some(value)
170+
}
171+
172+
fn sanitize_inline_image_result(value: &mut serde_json::Value) {
173+
let Some(obj) = value.as_object_mut() else {
174+
return;
175+
};
176+
177+
let saved_path = obj.get("saved_path").and_then(|v| v.as_str()).map(str::to_owned);
178+
// Strip any oversized inline-image `result` regardless of whether the image was
179+
// saved to disk. Older codex versions and interrupted/failed generations may emit
180+
// the multi-MB base64 without a `saved_path`; that payload must never reach the
181+
// WebSocket broadcast or SQLite either. `saved_path` only decides whether we attach
182+
// the structured `image { path, mime_type, source }` object below.
183+
let should_omit = obj
184+
.get("result")
185+
.and_then(|v| v.as_str())
186+
.map(is_probably_inline_image_result)
187+
.unwrap_or(false);
188+
189+
if !should_omit {
190+
return;
191+
}
192+
193+
let result_len = obj.get("result").and_then(|v| v.as_str()).map(str::len).unwrap_or(0);
194+
obj.remove("result");
195+
obj.insert("result_omitted".to_owned(), serde_json::Value::Bool(true));
196+
obj.insert(
197+
"result_omitted_reason".to_owned(),
198+
serde_json::Value::String("image_base64".to_owned()),
199+
);
200+
obj.insert(
201+
"result_bytes".to_owned(),
202+
serde_json::Value::Number(serde_json::Number::from(result_len)),
203+
);
204+
205+
if let Some(path) = saved_path {
206+
let mime_type = mime_type_from_image_path(&path);
207+
obj.insert(
208+
"image".to_owned(),
209+
serde_json::json!({
210+
"path": path,
211+
"mime_type": mime_type,
212+
"source": "codex_image_generation"
213+
}),
214+
);
215+
}
216+
}
217+
218+
fn is_probably_inline_image_result(value: &str) -> bool {
219+
value.len() > ACP_RAW_OUTPUT_INLINE_IMAGE_LIMIT
220+
&& (value.starts_with("iVBORw0KGgo")
221+
|| value.starts_with("/9j/")
222+
|| value.starts_with("UklGR")
223+
|| value.starts_with("data:image/"))
224+
}
225+
226+
fn mime_type_from_image_path(path: &str) -> &'static str {
227+
let lower = path.to_ascii_lowercase();
228+
if lower.ends_with(".jpg") || lower.ends_with(".jpeg") {
229+
"image/jpeg"
230+
} else if lower.ends_with(".webp") {
231+
"image/webp"
232+
} else if lower.ends_with(".gif") {
233+
"image/gif"
234+
} else {
235+
"image/png"
236+
}
237+
}
238+
239+
fn normalize_tool_status(
240+
sdk_status: Option<&SdkToolCallStatus>,
241+
raw_output: Option<&serde_json::Value>,
242+
) -> Option<AcpToolCallStatus> {
243+
let image_saved = raw_output
244+
.and_then(|v| v.get("image"))
245+
.and_then(|v| v.get("path"))
246+
.and_then(|v| v.as_str())
247+
.is_some();
248+
249+
// Only force `completed` when the image is on disk AND the agent did not already
250+
// report a terminal status. Codex stalls by leaving the final event as
251+
// `generating`/`in_progress`, but a genuine `failed` must be preserved as-is.
252+
match (image_saved, sdk_status.map(map_sdk_tool_status)) {
253+
(true, None | Some(AcpToolCallStatus::Pending | AcpToolCallStatus::InProgress)) => {
254+
Some(AcpToolCallStatus::Completed)
255+
}
256+
(_, status) => status,
257+
}
258+
}
259+
260+
fn normalize_raw_output_status(raw_output: &mut Option<serde_json::Value>, status: Option<&AcpToolCallStatus>) {
261+
let Some(AcpToolCallStatus::Completed) = status else {
262+
return;
263+
};
264+
let Some(obj) = raw_output.as_mut().and_then(|v| v.as_object_mut()) else {
265+
return;
266+
};
267+
obj.insert("status".to_owned(), serde_json::Value::String("completed".to_owned()));
268+
}
269+
160270
fn map_sdk_tool_kind(kind: &SdkToolKind) -> AcpToolCallKind {
161271
match kind {
162272
SdkToolKind::Read | SdkToolKind::Search => AcpToolCallKind::Read,

0 commit comments

Comments
 (0)