From f09ff7b343a2440615e684174fb97964d33906ef Mon Sep 17 00:00:00 2001 From: Joe Florencio Date: Fri, 12 Jun 2026 16:49:15 -0700 Subject: [PATCH] Simplify cloud config publication lifecycle --- codex-rs/cloud-config/src/bundle_loader.rs | 4 +--- codex-rs/cloud-config/src/service.rs | 4 +--- codex-rs/cloud-config/src/service_tests.rs | 2 +- codex-rs/config/src/cloud_config_bundle.rs | 11 ++-------- .../config/src/cloud_config_bundle_tests.rs | 20 ++++++++----------- codex-rs/config/src/loader/tests.rs | 4 ++-- 6 files changed, 15 insertions(+), 30 deletions(-) diff --git a/codex-rs/cloud-config/src/bundle_loader.rs b/codex-rs/cloud-config/src/bundle_loader.rs index b09c05ec7a..3240c7e486 100644 --- a/codex-rs/cloud-config/src/bundle_loader.rs +++ b/codex-rs/cloud-config/src/bundle_loader.rs @@ -35,9 +35,7 @@ pub fn cloud_config_bundle_loader( Some(CLOUD_CONFIG_BUNDLE_CACHE_REFRESH_RETRY_INTERVAL), ), }; - if !publisher.publish(initial_bundle) { - return; - } + publisher.publish(initial_bundle); if let Some(refresh_in) = refresh_in { service .refresh_cache_in_background(refresh_in, publisher) diff --git a/codex-rs/cloud-config/src/service.rs b/codex-rs/cloud-config/src/service.rs index c9ffedf6a3..aebbe33f20 100644 --- a/codex-rs/cloud-config/src/service.rs +++ b/codex-rs/cloud-config/src/service.rs @@ -550,9 +550,7 @@ where match self.load_bundle(auth, "refresh").await { Ok(loaded) => { emit_load_metric("refresh", "success", loaded.bundle.as_ref()); - if !publisher.publish(Ok(loaded.bundle)) { - return CacheRefreshSchedule::Stop; - } + publisher.publish(Ok(loaded.bundle)); CacheRefreshSchedule::ContinueAfter(loaded.refresh_in) } Err(err) => { diff --git a/codex-rs/cloud-config/src/service_tests.rs b/codex-rs/cloud-config/src/service_tests.rs index 4c9d636e38..4f09ecccf3 100644 --- a/codex-rs/cloud-config/src/service_tests.rs +++ b/codex-rs/cloud-config/src/service_tests.rs @@ -211,7 +211,7 @@ fn test_bundle() -> CloudConfigBundle { fn pending_loader() -> (CloudConfigBundleLoader, CloudConfigBundlePublisher) { let (loader, publisher) = CloudConfigBundleLoader::pending(); - assert!(publisher.publish(Ok(None))); + publisher.publish(Ok(None)); (loader, publisher) } diff --git a/codex-rs/config/src/cloud_config_bundle.rs b/codex-rs/config/src/cloud_config_bundle.rs index c02407d44c..5b15b9c108 100644 --- a/codex-rs/config/src/cloud_config_bundle.rs +++ b/codex-rs/config/src/cloud_config_bundle.rs @@ -238,14 +238,8 @@ impl CloudConfigBundlePublisher { /// /// An error resolves a pending loader, but does not replace a result that /// was already published. Successful results always replace the current - /// value. Returns `false` if every associated loader was dropped. - pub fn publish( - &self, - result: Result, CloudConfigBundleLoadError>, - ) -> bool { - if self.state.is_closed() { - return false; - } + /// value. + pub fn publish(&self, result: Result, CloudConfigBundleLoadError>) { self.state.send_if_modified(move |state| { if matches!(state, CloudConfigBundleState::Ready(_)) && result.is_err() { return false; @@ -253,7 +247,6 @@ impl CloudConfigBundlePublisher { *state = CloudConfigBundleState::Ready(result); true }); - !self.state.is_closed() } } diff --git a/codex-rs/config/src/cloud_config_bundle_tests.rs b/codex-rs/config/src/cloud_config_bundle_tests.rs index f1b78547b3..70c65ad611 100644 --- a/codex-rs/config/src/cloud_config_bundle_tests.rs +++ b/codex-rs/config/src/cloud_config_bundle_tests.rs @@ -27,7 +27,7 @@ async fn pending_loader_receives_initial_result() { let load_task = tokio::spawn(async move { tokio::join!(loader.get(), cloned_loader.get()) }); tokio::task::yield_now().await; - assert!(publisher.publish(Ok(Some(bundle.clone())))); + publisher.publish(Ok(Some(bundle.clone()))); assert_eq!( tokio::time::timeout(std::time::Duration::from_secs(1), load_task) @@ -48,17 +48,17 @@ async fn successful_result_replaces_error_and_later_error_is_ignored() { ); let (loader, publisher) = CloudConfigBundleLoader::pending(); - assert!(publisher.publish(Err(initial_error.clone()))); + publisher.publish(Err(initial_error.clone())); assert_eq!(loader.get().await, Err(initial_error)); - assert!(publisher.publish(Ok(Some(bundle.clone())))); + publisher.publish(Ok(Some(bundle.clone()))); assert_eq!(loader.get().await, Ok(Some(bundle.clone()))); - assert!(publisher.publish(Err(CloudConfigBundleLoadError::new( + publisher.publish(Err(CloudConfigBundleLoadError::new( CloudConfigBundleLoadErrorCode::RequestFailed, /*status_code*/ None, "refresh failed", - )))); + ))); assert_eq!(loader.get().await, Ok(Some(bundle))); } @@ -70,22 +70,18 @@ async fn refresh_updates_only_loader_and_its_clones() { let (loader, publisher) = CloudConfigBundleLoader::pending(); let cloned_loader = loader.clone(); - assert!(publisher.publish(Ok(Some(initial_bundle.clone())))); + publisher.publish(Ok(Some(initial_bundle.clone()))); assert_eq!(loader.get().await, Ok(Some(initial_bundle))); - assert!(publisher.publish(Ok(Some(refreshed_bundle.clone())))); + publisher.publish(Ok(Some(refreshed_bundle.clone()))); assert_eq!(loader.get().await, Ok(Some(refreshed_bundle.clone()))); assert_eq!(cloned_loader.get().await, Ok(Some(refreshed_bundle))); assert_eq!(independent_loader.get().await, Ok(None)); - assert!(publisher.publish(Ok(None))); + publisher.publish(Ok(None)); assert_eq!(loader.get().await, Ok(None)); - - drop(cloned_loader); - drop(loader); - assert!(!publisher.publish(Ok(Some(CloudConfigBundle::default())))); } #[tokio::test] diff --git a/codex-rs/config/src/loader/tests.rs b/codex-rs/config/src/loader/tests.rs index c156251206..5cbba905ef 100644 --- a/codex-rs/config/src/loader/tests.rs +++ b/codex-rs/config/src/loader/tests.rs @@ -133,7 +133,7 @@ async fn refreshed_cloud_bundle_applies_to_future_layer_loads() { let initial_bundle = bundle("initial", "initial-model", "never"); let refreshed_bundle = bundle("refreshed", "refreshed-model", "on-request"); let (loader, publisher) = CloudConfigBundleLoader::pending(); - assert!(publisher.publish(Ok(Some(initial_bundle)))); + publisher.publish(Ok(Some(initial_bundle))); let mut loader_overrides = LoaderOverrides::without_managed_config_for_tests(); loader_overrides.managed_config_path = Some(tmp.path().join("managed_config.toml")); loader_overrides.system_config_path = Some(tmp.path().join("system-config.toml")); @@ -166,7 +166,7 @@ async fn refreshed_cloud_bundle_applies_to_future_layer_loads() { } ); - assert!(publisher.publish(Ok(Some(refreshed_bundle)))); + publisher.publish(Ok(Some(refreshed_bundle))); let refreshed_layers = load_layers().await.expect("load refreshed layers"); assert_eq!(