Keep interrupts responsive during host service-tier updates (#60644)

## Why

Waiting for host acceptance of a service-tier update in the submission loop can block interrupts while the host receipt is pending.

## What changed

Move the host acceptance wait out of the submission loop while keeping explicit thread settings replies pending until acceptance completes. Report handoff failures to the caller while preserving the locally committed settings, and allow a subsequent explicit request for the same tier to retry a failed handoff.

## Testing

Add a WebSocket regression test covering interrupts with pending host receipts, failure reporting, same-tier retries, and continued thread operation after startup caller cancellation.

GitOrigin-RevId: 1b27d156eba3ebb47e717a987f41f4f595635be8
This commit is contained in:
rhan-oai
2026-11-10 19:13:55 +00:00
committed by copyberry
parent 3f797555cd
commit 30b42cb802
8 changed files with 302 additions and 33 deletions
+7 -4
View File
@@ -8,7 +8,7 @@ use futures::FutureExt;
use futures::future::BoxFuture;
use futures::future::Shared;
pub(crate) type ServiceTierAcceptance = Shared<BoxFuture<'static, ()>>;
pub(crate) type ServiceTierAcceptance = Shared<BoxFuture<'static, Result<(), String>>>;
/// The host's view of the service tier shared within an agent session.
///
@@ -27,9 +27,12 @@ pub trait AgentServiceTier: Send + Sync {
/// Hand off the committed tier in call order and return a receipt for local acceptance.
/// Core calls this method under its commit locks: the handoff must be fast and nonblocking.
/// Drive the update independently of the receipt: dropping the receipt only cancels the wait.
/// Core awaits it outside the state and registry locks. Explicit settings updates that
/// request the same committed tier share this receipt until the tier changes.
fn update(&self, service_tier: Option<String>) -> BoxFuture<'static, ()>;
/// Core waits outside the state and registry locks. `Ok(())` confirms local acceptance.
/// `Err` leaves acceptance unconfirmed and carries a caller-safe message. Core surfaces it
/// in an explicit thread settings reply. A later settings request with a reply can hand off
/// the same tier again after Core observes a failed receipt. The host must preserve commit
/// order and tolerate repeated handoffs.
fn update(&self, service_tier: Option<String>) -> BoxFuture<'static, Result<(), String>>;
}
pub(crate) fn host_child_uses_shared_service_tier(session: &Session) -> bool {
+8 -2
View File
@@ -826,7 +826,9 @@ impl CodexThread {
}
/// Queues settings for future turns and waits for Core to accept or reject them.
/// Rejections are returned to the caller instead of emitted as thread errors.
/// Rejections are returned to the caller instead of emitted as thread errors. A host-root
/// service-tier update also waits for local host acceptance outside Core's submission loop.
/// A handoff error leaves the settings committed locally. Callers can retry the same tier.
pub async fn update_thread_settings(
&self,
thread_settings: impl Into<WithTurnExtensionData<ThreadSettingsOverrides>>,
@@ -857,7 +859,11 @@ impl CodexThread {
settings: CodexThreadSettingsOverrides,
) -> ConstraintResult<()> {
let updates = self.thread_settings_update(settings);
self.session.update_settings(updates).await.map(|_| ())
let commit = self.session.update_settings(updates).await?;
if let Some(accepted) = commit.host_acceptance {
let _ = accepted.await;
}
Ok(())
}
/// Replaces tools for subsequent model steps and future turns, preserving in-flight calls.
+26 -6
View File
@@ -683,13 +683,33 @@ pub(super) async fn submission_loop(
request: thread_settings,
turn_extension_init: sub.turn_extension_init,
};
match thread_settings::update(&sess, thread_settings).await {
Ok(snapshot) => {
// Reply first: the caller may hold a lock its event consumer needs.
if let Some(reply) = reply {
let _ = reply.send(Ok(()));
match thread_settings::update(&sess, thread_settings, reply.is_some()).await {
Ok(commit) => {
match (reply, commit.host_acceptance) {
(Some(mut reply), Some(acceptance)) => {
// Leave the submission loop free to process interrupts.
tokio::spawn(async move {
tokio::select! {
_ = reply.closed() => {}
result = acceptance => {
let result = result.map_err(|error| {
CodexErr::from(std::io::Error::other(format!(
"service tier committed locally; host acceptance unconfirmed: {error}"
)))
});
let _ = reply.send(result);
}
}
});
}
(Some(reply), None) => {
// The caller may hold a lock its event consumer needs.
let _ = reply.send(Ok(()));
}
(None, _) => {}
}
thread_settings::emit_applied(&sess, sub.id.clone(), snapshot).await;
thread_settings::emit_applied(&sess, sub.id.clone(), commit.snapshot)
.await;
}
Err(error) => {
let message = format!("invalid thread settings override: {error}");
+19 -8
View File
@@ -2152,6 +2152,8 @@ impl Session {
///
/// The predicate must be fast and side-effect-free. It must not block, acquire
/// other locks, or call back into `Session`.
///
/// Returns any host receipt for the caller to await when it needs acknowledgment.
async fn update_settings_if(
&self,
updates: SessionSettingsUpdate,
@@ -2159,7 +2161,6 @@ impl Session {
) -> ConstraintResult<Option<SessionSettingsCommit>> {
let notify_config_contributors = !self.services.extensions.config_contributors().is_empty();
let windows_sandbox_changed;
let mut tier_acceptance = None;
let (
commit,
previous_config,
@@ -2223,7 +2224,17 @@ impl Session {
.update_configured_selections(&updated.environments);
}
state.session_configuration = updated;
if root_service_tier_changed && !state.shared_service_tier_pending {
let explicit_service_tier = updates.step_settings.service_tier.is_some();
let retry_failed_handoff = updates.host_acceptance_in_reply
&& explicit_service_tier
&& state
.service_tier_acceptance
.as_ref()
.and_then(futures::future::Shared::peek)
.is_some_and(Result::is_err);
if (root_service_tier_changed || retry_failed_handoff)
&& !state.shared_service_tier_pending
{
state.service_tier_acceptance = crate::agent::host::propagate_config_update(
self,
AgentConfigUpdate::ServiceTier(
@@ -2235,9 +2246,11 @@ impl Session {
),
);
}
if root_service_tier_changed || updates.step_settings.service_tier.is_some() {
tier_acceptance = state.service_tier_acceptance.clone();
}
let host_acceptance = if root_service_tier_changed || explicit_service_tier {
state.service_tier_acceptance.clone()
} else {
None
};
let new_config = notify_config_contributors
.then(|| self.build_effective_session_config(&state.session_configuration));
let commit = SessionSettingsCommit {
@@ -2245,6 +2258,7 @@ impl Session {
snapshot: state
.session_configuration
.thread_settings_snapshot(&state.session_configuration.environments),
host_acceptance,
};
(
commit,
@@ -2280,9 +2294,6 @@ impl Session {
if mcp_inputs_changed {
self.schedule_mcp_prewarm();
}
if let Some(accepted) = tier_acceptance {
accepted.await;
}
Ok(Some(commit))
}
+4
View File
@@ -666,6 +666,8 @@ impl SessionConfiguration {
pub(crate) struct SessionSettingsCommit {
pub(crate) configuration: SessionConfiguration,
pub(crate) snapshot: ThreadSettingsSnapshot,
/// Receipt for the host's local acceptance of this commit, when applicable.
pub(crate) host_acceptance: Option<Shared<BoxFuture<'static, Result<(), String>>>>,
}
#[derive(Default, Clone)]
@@ -682,6 +684,8 @@ pub(crate) struct SessionSettingsUpdate {
pub(crate) active_permission_profile: Option<ActivePermissionProfile>,
pub(crate) windows_sandbox_level: Option<WindowsSandboxLevel>,
pub(crate) service_tier_for_turn: Option<String>,
/// An explicit settings reply waits for host acceptance and can retry an observed failure.
pub(crate) host_acceptance_in_reply: bool,
pub(crate) app_server_client_name: Option<String>,
pub(crate) app_server_client_version: Option<String>,
pub(crate) disabled_plugin_ids: Option<Vec<String>>,
+6 -3
View File
@@ -2,6 +2,7 @@
//! with checkpoints written directly to storage.
use super::session::Session;
use super::session::SessionSettingsCommit;
use super::session::SessionSettingsUpdate;
use super::step_settings::StepSettingsUpdate;
use crate::WithTurnExtensionData;
@@ -77,8 +78,10 @@ impl Session {
pub(super) async fn update(
session: &Session,
overrides: impl Into<WithTurnExtensionData<ThreadSettingsOverrides>>,
) -> ConstraintResult<ThreadSettingsSnapshot> {
let updates = prepare_update(overrides, &session.services.selected_capability_roots);
reply_requested: bool,
) -> ConstraintResult<SessionSettingsCommit> {
let mut updates = prepare_update(overrides, &session.services.selected_capability_roots);
updates.host_acceptance_in_reply = reply_requested;
let commit = session.update_settings(updates).await?;
let active_turn = session.active_turn.lock().await;
// A registered running task retains its lineage so it can be recovered after a daemon
@@ -89,7 +92,7 @@ pub(super) async fn update(
{
session.state.lock().await.last_started_turn_id = None;
}
Ok(commit.snapshot)
Ok(commit)
}
/// Converts protocol overrides into the internal settings update shape.
+1 -1
View File
@@ -2980,7 +2980,7 @@ impl ThreadManagerState {
}
// Cancelling this wait must leave the registered runtime and the host handoff intact.
if let Some(accepted) = tier_acceptance {
accepted.await;
let _ = accepted.await;
}
Ok(new_thread)
}