mirror of
https://github.com/openai/codex.git
synced 2026-10-19 16:17:49 +00:00
Pass thread persistence intent to attachment uploads (#51517)
## Why Attachment stores need to distinguish uploads from ephemeral threads, which intentionally skip durable persistence. ## What changed Add `ephemeral` to `UploadRequest` and propagate the turn configuration through image preparation for messages, tool outputs, and history replay. Include the flag in debug output while keeping attachment bytes redacted. ## Testing Extend the Guardian context-budget test to assert that a persistent Guardian uploads exactly one image with `ephemeral: false`. Update the attachment debug-output assertion to include the new field. GitOrigin-RevId: 42814c514a571ed6798e4dcf79a22028ee4efc28
This commit is contained in:
committed by
copyberry
parent
24edd7b890
commit
19c4793964
@@ -38,6 +38,8 @@ pub trait AttachmentStore: Send + Sync {
|
||||
pub struct UploadRequest {
|
||||
/// Thread receiving this attachment; storage backends may use it for placement.
|
||||
pub thread_id: String,
|
||||
/// Whether the originating thread intentionally skips durable persistence.
|
||||
pub ephemeral: bool,
|
||||
/// Optional name associated with the attachment.
|
||||
pub file_name: Option<String>,
|
||||
/// Attachment bytes to persist.
|
||||
@@ -49,6 +51,7 @@ impl fmt::Debug for UploadRequest {
|
||||
formatter
|
||||
.debug_struct("UploadRequest")
|
||||
.field("thread_id", &self.thread_id)
|
||||
.field("ephemeral", &self.ephemeral)
|
||||
.field("file_name", &self.file_name)
|
||||
.field("data", &"<redacted>")
|
||||
.finish()
|
||||
|
||||
@@ -7,6 +7,7 @@ use super::*;
|
||||
fn attachment_debug_output_redacts_bytes() {
|
||||
let request = UploadRequest {
|
||||
thread_id: "thread-1".to_string(),
|
||||
ephemeral: false,
|
||||
file_name: Some("image.png".to_string()),
|
||||
data: b"secret".to_vec(),
|
||||
};
|
||||
@@ -17,7 +18,7 @@ fn attachment_debug_output_redacts_bytes() {
|
||||
assert_eq!(
|
||||
(format!("{request:?}"), format!("{result:?}")),
|
||||
(
|
||||
r#"UploadRequest { thread_id: "thread-1", file_name: Some("image.png"), data: "<redacted>" }"#.to_string(),
|
||||
r#"UploadRequest { thread_id: "thread-1", ephemeral: false, file_name: Some("image.png"), data: "<redacted>" }"#.to_string(),
|
||||
r#"Inline { bytes: "<redacted>" }"#.to_string(),
|
||||
)
|
||||
);
|
||||
@@ -49,6 +50,7 @@ async fn inline_store_preserves_image_bytes() {
|
||||
let attachment = InlineAttachmentStore
|
||||
.upload(UploadRequest {
|
||||
thread_id: "thread-1".to_string(),
|
||||
ephemeral: false,
|
||||
file_name: Some(file_name.to_string()),
|
||||
data: data.to_vec(),
|
||||
})
|
||||
|
||||
@@ -53,6 +53,7 @@ pub(crate) fn unified_image_budget_enabled(
|
||||
#[derive(Clone, Copy, Debug)]
|
||||
struct ImageOrigin<'a> {
|
||||
thread_id: &'a str,
|
||||
ephemeral: bool,
|
||||
message_role: Option<&'a str>,
|
||||
item_id: Option<&'a str>,
|
||||
}
|
||||
@@ -120,6 +121,7 @@ impl ImagePreparationError {
|
||||
|
||||
pub(crate) async fn prepare_response_items(
|
||||
thread_id: &str,
|
||||
ephemeral: bool,
|
||||
items: &mut Vec<ResponseItem>,
|
||||
mode: ImagePreparationMode,
|
||||
resize_notice_mode: ImageResizeNoticeMode,
|
||||
@@ -138,6 +140,7 @@ pub(crate) async fn prepare_response_items(
|
||||
&mut content,
|
||||
ImageOrigin {
|
||||
thread_id,
|
||||
ephemeral,
|
||||
message_role: Some(role.as_str()),
|
||||
item_id: None,
|
||||
},
|
||||
@@ -161,6 +164,7 @@ pub(crate) async fn prepare_response_items(
|
||||
} => {
|
||||
prepare_tool_output(
|
||||
thread_id,
|
||||
ephemeral,
|
||||
output,
|
||||
call_id.as_deref(),
|
||||
resize_notice_mode,
|
||||
@@ -175,6 +179,7 @@ pub(crate) async fn prepare_response_items(
|
||||
} => {
|
||||
prepare_tool_output(
|
||||
thread_id,
|
||||
ephemeral,
|
||||
output,
|
||||
Some(call_id.as_str()),
|
||||
resize_notice_mode,
|
||||
@@ -209,8 +214,10 @@ pub(crate) async fn prepare_response_items(
|
||||
metadata
|
||||
}
|
||||
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
async fn prepare_tool_output(
|
||||
thread_id: &str,
|
||||
ephemeral: bool,
|
||||
output: &mut FunctionCallOutputPayload,
|
||||
item_id: Option<&str>,
|
||||
resize_notice_mode: ImageResizeNoticeMode,
|
||||
@@ -223,6 +230,7 @@ async fn prepare_tool_output(
|
||||
content,
|
||||
ImageOrigin {
|
||||
thread_id,
|
||||
ephemeral,
|
||||
message_role: None,
|
||||
item_id,
|
||||
},
|
||||
@@ -357,6 +365,7 @@ async fn prepare_image(
|
||||
let resize = prepared.resize;
|
||||
let request = UploadRequest {
|
||||
thread_id: origin.thread_id.to_owned(),
|
||||
ephemeral: origin.ephemeral,
|
||||
file_name: None,
|
||||
data: prepared.encoded.bytes.to_vec(),
|
||||
};
|
||||
|
||||
@@ -101,6 +101,7 @@ async fn preparation_preserves_small_image_bytes_and_replaces_remote_urls() {
|
||||
|
||||
prepare_response_items(
|
||||
"image-preparation-thread",
|
||||
/*ephemeral*/ false,
|
||||
&mut items,
|
||||
ImagePreparationMode::DetailBased,
|
||||
ImageResizeNoticeMode::Disabled,
|
||||
@@ -187,6 +188,7 @@ async fn detail_policies_apply_the_expected_budgets() {
|
||||
|
||||
let metadata = prepare_response_items(
|
||||
"image-preparation-thread",
|
||||
/*ephemeral*/ false,
|
||||
&mut items,
|
||||
ImagePreparationMode::DetailBased,
|
||||
ImageResizeNoticeMode::Disabled,
|
||||
@@ -241,6 +243,7 @@ async fn preparation_reports_tool_output_item_id() {
|
||||
}];
|
||||
let metadata = prepare_response_items(
|
||||
"image-preparation-thread",
|
||||
/*ephemeral*/ false,
|
||||
&mut items,
|
||||
ImagePreparationMode::DetailBased,
|
||||
ImageResizeNoticeMode::Disabled,
|
||||
@@ -278,6 +281,7 @@ async fn upload_failure_keeps_resized_image_inline() {
|
||||
|
||||
prepare_response_items(
|
||||
"image-preparation-thread",
|
||||
/*ephemeral*/ false,
|
||||
&mut items,
|
||||
ImagePreparationMode::DetailBased,
|
||||
ImageResizeNoticeMode::Disabled,
|
||||
@@ -379,6 +383,7 @@ async fn resize_notices_count_file_backed_images_and_skip_failed_images() {
|
||||
|
||||
prepare_response_items(
|
||||
"image-preparation-thread",
|
||||
/*ephemeral*/ false,
|
||||
&mut items,
|
||||
ImagePreparationMode::DetailBased,
|
||||
ImageResizeNoticeMode::Enabled,
|
||||
@@ -567,6 +572,7 @@ async fn preparation_replaces_only_failed_tool_images_and_preserves_metadata() {
|
||||
|
||||
prepare_response_items(
|
||||
"image-preparation-thread",
|
||||
/*ephemeral*/ false,
|
||||
&mut items,
|
||||
ImagePreparationMode::DetailBased,
|
||||
ImageResizeNoticeMode::Disabled,
|
||||
|
||||
@@ -1790,6 +1790,7 @@ impl Session {
|
||||
// Bound replay future size now that image preparation can await storage.
|
||||
let _ = Box::pin(prepare_image_response_items(
|
||||
&self.thread_id.to_string(),
|
||||
turn_context.config.ephemeral,
|
||||
&mut prepared_history,
|
||||
ImagePreparationMode::DetailBased,
|
||||
ImageResizeNoticeMode::Disabled,
|
||||
@@ -3365,6 +3366,7 @@ impl Session {
|
||||
// Keep nested image-upload futures out of every caller's future frame.
|
||||
let image_preparations = Box::pin(prepare_image_response_items(
|
||||
&self.thread_id.to_string(),
|
||||
turn_context.config.ephemeral,
|
||||
&mut items,
|
||||
image_preparation_mode,
|
||||
image_resize_notice_mode,
|
||||
|
||||
@@ -463,13 +463,16 @@ async fn review_respects_complete_context_budget(
|
||||
assert_eq!(compact_requests.len(), 1);
|
||||
let compact = &compact_requests[0];
|
||||
if matches!(reviewer_response, ReviewerResponse::FileImageContinuation) {
|
||||
assert_eq!(
|
||||
image_store
|
||||
.uploads
|
||||
.lock()
|
||||
.expect("image upload tracker lock is not poisoned")
|
||||
.len(),
|
||||
1
|
||||
let uploads = image_store
|
||||
.uploads
|
||||
.lock()
|
||||
.expect("image upload tracker lock is not poisoned");
|
||||
let [upload] = uploads.as_slice() else {
|
||||
panic!("persistent Guardian should upload one image");
|
||||
};
|
||||
assert!(
|
||||
!upload.ephemeral,
|
||||
"persistent Guardian should request durable attachment storage"
|
||||
);
|
||||
// The file reservation also protects the compaction request itself: an output
|
||||
// larger than its window is replaced before the summary request is sent.
|
||||
|
||||
Reference in New Issue
Block a user