openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-08-23 13:09:46 +00:00
Code Issues Packages Projects Releases Wiki Activity

Route pet asset downloads through the shared HTTP client (#36008)

Browse Source 
## Why

Built-in pet downloads used a standalone blocking client, so CDN redirects did
not use Codex's configured route handling.

## What changed

- Download pet spritesheets asynchronously with a reusable
  `RouteAwareClientPool`.
- Keep cache validation, asset installation, and pet decoding off the async
  runtime while sharing one asset-and-load path across startup, previews, and
  selection.
- Preserve the download size limit while consuming streamed response chunks,
  and remove the TUI's direct `reqwest` dependency.

## Testing

Add coverage for oversized response chunks, cached built-in assets, pet loads
without an existing Tokio runtime, and stale preview and selection completions.

GitOrigin-RevId: 725e76f3ddb7e84b7212769892f28dc0add15741
This commit is contained in:
Celia Chen
2026-07-29 18:34:20 +00:00
committed by copyberry
parent ad6fc66b6d
commit 09cf609218
11 changed files with 293 additions and 95 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
codex-rs/Cargo.lock generated
View File

@@ -4268,6 +4268,7 @@ dependencies = [
"dirs",
"dunce",
"futures",
"http 1.4.0",
"image",
"insta",
"itertools 0.14.0",
@@ -4280,7 +4281,6 @@ dependencies = [
"ratatui",
"ratatui-macros",
"regex-lite",
"reqwest 0.12.28",
"rmcp",
"serde",
"serde_json",

1
codex-rs/deny.toml
View File

@@ -245,7 +245,6 @@ deny = [
"codex-otel",
"codex-protocol",
"codex-responses-api-proxy",
"codex-tui",
# Third-party crates that own their reqwest integration. These are not part of the
# first-party migration count above.
"oauth2",

2
codex-rs/tui/Cargo.toml
View File

@@ -87,7 +87,6 @@ ratatui = { workspace = true, features = [
] }
ratatui-macros = { workspace = true }
regex-lite = { workspace = true }
reqwest = { workspace = true, features = ["blocking", "json"] }
rmcp = { workspace = true }
serde = { workspace = true, features = ["derive"] }
serde_json = { workspace = true, features = ["preserve_order"] }
@@ -156,6 +155,7 @@ codex-utils-cargo-bin = { workspace = true }
assert_matches = { workspace = true }
chrono = { workspace = true, features = ["serde"] }
futures = { workspace = true }
http = { workspace = true }
insta = { workspace = true }
pretty_assertions = { workspace = true }
rand = { workspace = true }

24
codex-rs/tui/src/app/pets.rs
View File

@@ -82,18 +82,18 @@ impl App {
let frame_requester = tui.frame_requester();
let animations_enabled = self.config.animations;
let tx = self.app_event_tx.clone();
std::mem::drop(tokio::task::spawn_blocking(move || {
let result = crate::pets::ensure_builtin_pack_for_pet(&pet_id, &codex_home)
.and_then(|()| {
crate::pets::AmbientPet::load(
Some(&pet_id),
&codex_home,
frame_requester,
animations_enabled,
)
})
.map(Some)
.map_err(|err| err.to_string());
let pet_http_client = self.chat_widget.pet_http_client.clone();
std::mem::drop(tokio::spawn(async move {
let result = crate::pets::load_pet_with_assets(
pet_id.clone(),
codex_home,
frame_requester,
animations_enabled,
&pet_http_client,
)
.await
.map(Some)
.map_err(|err| err.to_string());
tx.send(AppEvent::PetSelectionLoaded {
request_id,
pet_id,

2
codex-rs/tui/src/chatwidget.rs
View File

@@ -637,6 +637,8 @@ pub(crate) struct ChatWidget {
review: ReviewState,
// Active hook runs render in a dedicated live cell so they can run alongside tools.
active_hook_cell: Option<HookCell>,
// Reused for built-in pet CDN requests so redirects remain route-aware.
pub(crate) pet_http_client: codex_http_client::RouteAwareClientPool,
// Ambient companion rendered over the transcript area, never inside the footer rows.
ambient_pet: Option<crate::pets::AmbientPet>,
pet_picker_preview_state: crate::pets::PetPickerPreviewState,

6
codex-rs/tui/src/chatwidget/constructor.rs
View File

@@ -85,11 +85,16 @@ impl ChatWidget {
&chat_keymap.edit_queued_message,
current_terminal_info,
);
let pet_http_client = codex_http_client::RouteAwareClientPool::new(
config.http_client_factory(),
codex_http_client::ClientRouteClass::Other,
);
pets::start_configured_pet_load_if_needed(
&config,
/*ambient_pet_missing*/ true,
frame_requester.clone(),
app_event_tx.clone(),
pet_http_client.clone(),
);
let mut widget = Self {
app_event_tx: app_event_tx.clone(),
@@ -184,6 +189,7 @@ impl ChatWidget {
status_state: StatusState::default(),
review: ReviewState::default(),
active_hook_cell: None,
pet_http_client,
ambient_pet: None,
pet_picker_preview_state: crate::pets::PetPickerPreviewState::default(),
pet_picker_preview_pet: None,

92
codex-rs/tui/src/chatwidget/pets.rs
View File

@@ -26,6 +26,7 @@ pub(super) fn start_configured_pet_load_if_needed(
ambient_pet_missing: bool,
frame_requester: FrameRequester,
app_event_tx: AppEventSender,
pet_http_client: codex_http_client::RouteAwareClientPool,
) {
let Some(pet_id) = config.tui_pet.clone() else {
return;
@@ -36,20 +37,26 @@ pub(super) fn start_configured_pet_load_if_needed(
let codex_home = config.codex_home.clone();
let animations_enabled = config.animations;
spawn_pet_load(move || {
let result = crate::pets::ensure_builtin_pack_for_pet(&pet_id, &codex_home)
.and_then(|()| {
crate::pets::AmbientPet::load(
Some(&pet_id),
&codex_home,
frame_requester,
animations_enabled,
)
})
let event_pet_id = pet_id.clone();
spawn_pet_load(
async move {
crate::pets::load_pet_with_assets(
pet_id,
codex_home,
frame_requester,
animations_enabled,
&pet_http_client,
)
.await
.map(Some)
.map_err(|err| err.to_string());
app_event_tx.send(AppEvent::ConfiguredPetLoaded { pet_id, result });
});
.map_err(|err| err.to_string())
},
app_event_tx,
move |result| AppEvent::ConfiguredPetLoaded {
pet_id: event_pet_id,
result,
},
);
}
impl ChatWidget {
@@ -234,19 +241,22 @@ impl ChatWidget {
let codex_home = self.config.codex_home.clone();
let frame_requester = self.frame_requester.clone();
let tx = self.app_event_tx.clone();
spawn_pet_load(move || {
let result = crate::pets::ensure_builtin_pack_for_pet(&pet_id, &codex_home)
.and_then(|()| {
crate::pets::AmbientPet::load(
Some(&pet_id),
&codex_home,
frame_requester,
/*animations_enabled*/ false,
)
})
.map_err(|err| err.to_string());
tx.send(AppEvent::PetPreviewLoaded { request_id, result });
});
let pet_http_client = self.pet_http_client.clone();
spawn_pet_load(
async move {
crate::pets::load_pet_with_assets(
pet_id,
codex_home,
frame_requester,
/*animations_enabled*/ false,
&pet_http_client,
)
.await
.map_err(|err| err.to_string())
},
tx,
move |result| AppEvent::PetPreviewLoaded { request_id, result },
);
}
pub(crate) fn finish_pet_picker_preview_load(
@@ -326,10 +336,34 @@ impl ChatWidget {
}
}
fn spawn_pet_load(f: impl FnOnce() + Send + 'static) {
fn spawn_pet_load<T>(
future: impl std::future::Future<Output = Result<T, String>> + Send + 'static,
app_event_tx: AppEventSender,
completion_event: impl FnOnce(Result<T, String>) -> AppEvent + Send + 'static,
) where
T: Send + 'static,
{
if let Ok(handle) = tokio::runtime::Handle::try_current() {
std::mem::drop(handle.spawn_blocking(f));
std::mem::drop(handle.spawn(async move {
app_event_tx.send(completion_event(future.await));
}));
} else {
let _ = std::thread::spawn(f);
let _ = std::thread::spawn(move || {
let result = match tokio::runtime::Builder::new_current_thread()
.enable_all()
.build()
{
Ok(runtime) => runtime.block_on(future),
Err(err) => {
tracing::warn!(error = %err, "failed to start pet load runtime");
Err(format!("failed to start pet load runtime: {err}"))
}
};
app_event_tx.send(completion_event(result));
});
}
}
#[cfg(test)]
#[path = "pets_tests.rs"]
mod tests;

86
codex-rs/tui/src/chatwidget/pets_tests.rs Normal file
View File

@@ -0,0 +1,86 @@
use super::*;
use codex_utils_absolute_path::AbsolutePathBuf;
use pretty_assertions::assert_eq;
#[test]
fn pet_load_without_runtime_sends_completion_event() {
let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel();
let app_event_tx = AppEventSender::new(tx);
spawn_pet_load(
async { Ok::<Option<crate::pets::AmbientPet>, String>(None) },
app_event_tx,
|result| AppEvent::ConfiguredPetLoaded {
pet_id: crate::pets::DEFAULT_PET_ID.to_string(),
result,
},
);
match rx.blocking_recv().expect("pet load completion event") {
AppEvent::ConfiguredPetLoaded { pet_id, result } => {
assert_eq!(pet_id, crate::pets::DEFAULT_PET_ID);
assert!(result.expect("successful pet load").is_none());
}
event => panic!("expected configured pet completion, got {event:?}"),
}
}
#[tokio::test]
async fn shared_pet_load_uses_cached_builtin_assets() {
let (chat, _tx, _rx, _op_rx) =
crate::chatwidget::tests::make_chatwidget_manual_with_sender().await;
let codex_home = tempfile::tempdir().unwrap();
crate::pets::write_test_pack(codex_home.path());
crate::pets::load_pet_with_assets(
crate::pets::DEFAULT_PET_ID.to_string(),
AbsolutePathBuf::from_absolute_path(codex_home.path()).expect("absolute temporary path"),
chat.frame_requester.clone(),
/*animations_enabled*/ false,
&chat.pet_http_client,
)
.await
.expect("load cached built-in pet");
assert!(
codex_home
.path()
.join("cache")
.join("tui-pets")
.join("frame-cache")
.join(crate::pets::DEFAULT_PET_ID)
.is_dir()
);
}
#[tokio::test]
async fn stale_pet_preview_completion_keeps_current_preview() {
let (mut chat, _tx, _rx, _op_rx) =
crate::chatwidget::tests::make_chatwidget_manual_with_sender().await;
chat.pet_picker_preview_request_id = 2;
chat.pet_picker_preview_pet = Some(crate::pets::test_ambient_pet(
chat.frame_requester.clone(),
/*animations_enabled*/ false,
));
chat.finish_pet_picker_preview_load(/*request_id*/ 1, Err("stale preview".to_string()));
assert!(chat.pet_picker_preview_pet.is_some());
assert_eq!(chat.pet_picker_preview_request_id, 2);
}
#[tokio::test]
async fn stale_pet_selection_completion_keeps_current_loading_popup() {
let (mut chat, _tx, _rx, _op_rx) =
crate::chatwidget::tests::make_chatwidget_manual_with_sender().await;
let current_request_id = chat.show_pet_selection_loading_popup();
let stale_request_id = current_request_id.wrapping_sub(/*rhs*/ 1);
assert!(!chat.finish_pet_selection_loading_popup(stale_request_id));
assert_eq!(
chat.bottom_pane.active_view_id(),
Some(crate::chatwidget::PET_SELECTION_LOADING_VIEW_ID)
);
assert!(chat.finish_pet_selection_loading_popup(current_request_id));
assert_eq!(chat.bottom_pane.active_view_id(), None);
}

2
codex-rs/tui/src/history_cell/tests.rs
View File

@@ -20,10 +20,10 @@ use codex_protocol::account::PlanType;
use codex_protocol::error::UnexpectedResponseError;
use codex_protocol::parse_command::ParsedCommand;
use dirs::home_dir;
use http::StatusCode;
use pretty_assertions::assert_eq;
use ratatui::buffer::Buffer;
use ratatui::layout::Rect;
use reqwest::StatusCode;
use serde_json::json;
use std::collections::HashMap;
use std::path::PathBuf;

128
codex-rs/tui/src/pets/asset_pack.rs
View File

@@ -11,7 +11,6 @@
//! built-in pet is safe to persist to config.
use std::fs;
use std::io::Read;
use std::path::Path;
use std::path::PathBuf;
use std::time::Duration;
@@ -19,6 +18,7 @@ use std::time::Duration;
use anyhow::Context;
use anyhow::Result;
use anyhow::bail;
use codex_http_client::RouteAwareClientPool;
use url::Url;
use uuid::Uuid;
@@ -42,44 +42,58 @@ pub(crate) fn builtin_spritesheet_path(codex_home: &Path, file: &str) -> PathBuf
/// validates the decoded image dimensions, and installs it atomically. Callers
/// should treat any error here as "the asset is unavailable", not as a partial
/// install they can safely ignore.
pub(crate) fn ensure_builtin_pet(codex_home: &Path, pet: catalog::BuiltinPet) -> Result<()> {
pub(crate) async fn ensure_builtin_pet(
codex_home: &Path,
pet: catalog::BuiltinPet,
http_client: &RouteAwareClientPool,
) -> Result<()> {
let destination = builtin_spritesheet_path(codex_home, pet.spritesheet_file);
if validate_cached_spritesheet(&destination).is_ok() {
let cache_destination = destination.clone();
let cache_valid = tokio::task::spawn_blocking(move || {
validate_cached_spritesheet(&cache_destination).is_ok()
})
.await
.context("join pet spritesheet cache validation task")?;
if cache_valid {
return Ok(());
}
let url = builtin_pet_url(pet)?;
let bytes = download_bytes_with_limit(&url, PET_MAX_DOWNLOAD_BYTES)?;
let parent = destination
.parent()
.context("pet spritesheet path should include an assets directory")?;
fs::create_dir_all(parent).with_context(|| format!("create {}", parent.display()))?;
let bytes = download_bytes_with_limit(http_client, &url, PET_MAX_DOWNLOAD_BYTES).await?;
tokio::task::spawn_blocking(move || {
let parent = destination
.parent()
.context("pet spritesheet path should include an assets directory")?;
fs::create_dir_all(parent).with_context(|| format!("create {}", parent.display()))?;
let staging = destination.with_file_name(format!(
".{}.download-{}.webp",
pet.spritesheet_file,
Uuid::new_v4()
));
fs::write(&staging, &bytes).with_context(|| format!("write {}", staging.display()))?;
if let Err(err) = validate_cached_spritesheet(&staging) {
let _ = fs::remove_file(&staging);
return Err(err);
}
let staging = destination.with_file_name(format!(
".{}.download-{}.webp",
pet.spritesheet_file,
Uuid::new_v4()
));
fs::write(&staging, &bytes).with_context(|| format!("write {}", staging.display()))?;
if let Err(err) = validate_cached_spritesheet(&staging) {
let _ = fs::remove_file(&staging);
return Err(err);
}
if install_downloaded_spritesheet(&staging, &destination).is_ok() {
return Ok(());
}
if install_downloaded_spritesheet(&staging, &destination).is_ok() {
return Ok(());
}
if validate_cached_spritesheet(&destination).is_ok() {
let _ = fs::remove_file(&staging);
return Ok(());
}
if validate_cached_spritesheet(&destination).is_ok() {
let _ = fs::remove_file(&staging);
return Ok(());
}
if destination.exists() {
fs::remove_file(&destination)
.with_context(|| format!("remove {}", destination.display()))?;
}
install_downloaded_spritesheet(&staging, &destination)
if destination.exists() {
fs::remove_file(&destination)
.with_context(|| format!("remove {}", destination.display()))?;
}
install_downloaded_spritesheet(&staging, &destination)
})
.await
.context("join pet spritesheet install task")?
}
fn builtin_pet_url(pet: catalog::BuiltinPet) -> Result<String> {
@@ -92,14 +106,17 @@ fn pack_dir(codex_home: &Path) -> PathBuf {
codex_home.join(PET_PACK_DIR).join(PET_PACK_VERSION)
}
fn download_bytes_with_limit(url: &str, max_bytes: u64) -> Result<Vec<u8>> {
async fn download_bytes_with_limit(
http_client: &RouteAwareClientPool,
url: &str,
max_bytes: u64,
) -> Result<Vec<u8>> {
validate_download_url(url)?;
let response = reqwest::blocking::Client::builder()
.timeout(PET_DOWNLOAD_TIMEOUT)
.build()
.context("build pet asset download client")?
let mut response = http_client
.get(url)
.timeout(PET_DOWNLOAD_TIMEOUT)
.send()
.await
.with_context(|| format!("download pet asset from {url}"))?
.error_for_status()
.with_context(|| format!("download pet asset from {url}"))?;
@@ -110,16 +127,29 @@ fn download_bytes_with_limit(url: &str, max_bytes: u64) -> Result<Vec<u8>> {
}
let mut bytes = Vec::new();
response
.take(max_bytes.saturating_add(/*rhs*/ 1))
.read_to_end(&mut bytes)
.with_context(|| format!("read pet asset download from {url}"))?;
if bytes.len() as u64 > max_bytes {
bail!("pet asset download from {url} exceeded {max_bytes} bytes");
while let Some(chunk) = response
.chunk()
.await
.with_context(|| format!("read pet asset download from {url}"))?
{
append_download_chunk(&mut bytes, &chunk, max_bytes, url)?;
}
Ok(bytes)
}
fn append_download_chunk(
bytes: &mut Vec<u8>,
chunk: &[u8],
max_bytes: u64,
url: &str,
) -> Result<()> {
if (bytes.len() as u64).saturating_add(chunk.len() as u64) > max_bytes {
bail!("pet asset download from {url} exceeded {max_bytes} bytes");
}
bytes.extend_from_slice(chunk);
Ok(())
}
fn install_downloaded_spritesheet(staging: &Path, destination: &Path) -> Result<()> {
fs::rename(staging, destination).with_context(|| format!("install {}", destination.display()))
}
@@ -175,6 +205,22 @@ mod tests {
);
}
#[test]
fn oversized_download_chunk_is_rejected() {
let url = "https://example.com/pet.webp";
let mut bytes = Vec::new();
append_download_chunk(&mut bytes, b"1234", /*max_bytes*/ 8, url).unwrap();
let error = append_download_chunk(&mut bytes, b"56789", /*max_bytes*/ 8, url)
.expect_err("chunk should exceed the download limit");
assert_eq!(
error.to_string(),
"pet asset download from https://example.com/pet.webp exceeded 8 bytes"
);
assert_eq!(bytes, b"1234");
}
#[test]
fn write_test_pack_installs_all_builtins() {
let dir = tempfile::tempdir().unwrap();

43
codex-rs/tui/src/pets/mod.rs
View File

@@ -9,9 +9,9 @@
//! resolving a selected pet id, preparing frames for terminal image protocols,
//! rendering the ambient sprite and picker preview, and preserving enough
//! metadata for `/pets` to behave like a first-class configuration surface.
//! It does not own config persistence or popup orchestration; callers must
//! ensure a built-in asset exists before loading it and must persist the final
//! selection only after the load succeeds.
//! It prepares built-in assets before loading pets, but does not own config
//! persistence or popup orchestration; callers must persist the final selection
//! only after the load succeeds.
use std::io::Write;
@@ -27,6 +27,10 @@ mod sixel;
use anyhow::Context;
use anyhow::Result;
use codex_http_client::RouteAwareClientPool;
use codex_utils_absolute_path::AbsolutePathBuf;
use crate::tui::FrameRequester;
pub(crate) use ambient::AmbientPet;
pub(crate) use ambient::AmbientPetDraw;
@@ -53,20 +57,41 @@ pub(crate) const DISABLED_PET_ID: &str = "disabled";
/// Ensure that a selected built-in pet has a locally cached spritesheet.
///
/// Custom pets are intentionally a no-op here because their source of truth is
/// already local. Callers should invoke this before loading a built-in pet for
/// preview or selection; skipping it would make first-use preview and
/// persistence failures depend on deeper image-loading errors instead of the
/// asset-fetch boundary.
pub(crate) fn ensure_builtin_pack_for_pet(
/// already local. Preparing this before loading keeps first-use preview and
/// persistence failures at the asset-fetch boundary rather than surfacing as
/// deeper image-loading errors.
async fn ensure_builtin_pack_for_pet(
pet_id: &str,
codex_home: &std::path::Path,
http_client: &RouteAwareClientPool,
) -> Result<()> {
if let Some(pet) = catalog::builtin_pet(pet_id) {
asset_pack::ensure_builtin_pet(codex_home, pet)?;
asset_pack::ensure_builtin_pet(codex_home, pet, http_client).await?;
}
Ok(())
}
/// Prepare a pet's built-in assets and load its synchronous state off the runtime.
pub(crate) async fn load_pet_with_assets(
pet_id: String,
codex_home: AbsolutePathBuf,
frame_requester: FrameRequester,
animations_enabled: bool,
http_client: &RouteAwareClientPool,
) -> Result<AmbientPet> {
ensure_builtin_pack_for_pet(&pet_id, &codex_home, http_client).await?;
tokio::task::spawn_blocking(move || {
AmbientPet::load(
Some(&pet_id),
&codex_home,
frame_requester,
animations_enabled,
)
})
.await
.context("join pet load task")?
}
#[derive(Debug)]
pub(crate) enum PetImageRenderError {
Terminal(std::io::Error),