mirror of
https://github.com/openai/codex.git
synced 2026-11-14 21:40:39 +00:00
Capture saved Daybreak preferences for native helpers (#60749)
## What changed Add the disabled-by-default `daybreak_program_propagation` feature to capture a parent's saved Daybreak choice when spawning native helpers. Retain the choice and account identity for helper turns and nested spawns, while preserving explicit `cyber_access_program` overrides. A helper's own saved preference takes precedence on later turns. Read pending metadata before persisted metadata so preferences survive thread materialization, with stored choices taking precedence. Share this resolution logic with `thread/read` and support refreshing the feature through user configuration. ## Testing Add coverage for saved and pending preferences, unset choices, explicit overrides, failed metadata reads, nested spawns, parent unloading, feature withdrawal, and preference changes between helper turns. GitOrigin-RevId: b3b349da8cb3d48154cddf058890587385efcda2
This commit is contained in:
@@ -81,6 +81,24 @@ pub enum ThreadStoreError {
|
||||
},
|
||||
}
|
||||
|
||||
impl ThreadStoreError {
|
||||
/// Whether a read found no persisted record for this specific thread.
|
||||
pub fn is_missing_for_read(&self, requested_thread_id: ThreadId) -> bool {
|
||||
match self {
|
||||
Self::ThreadNotFound { thread_id } => *thread_id == requested_thread_id,
|
||||
// The local store's strict read uses this legacy error for an unmaterialized rollout.
|
||||
Self::InvalidRequest { message } => {
|
||||
message == &format!("no rollout found for thread id {requested_thread_id}")
|
||||
}
|
||||
Self::Conflict { .. }
|
||||
| Self::Unsupported { .. }
|
||||
| Self::Retryable { .. }
|
||||
| Self::OutcomeUnknown { .. }
|
||||
| Self::Internal { .. } => false,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "error_tests.rs"]
|
||||
mod tests;
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
//! Verifies conservative classification at the shared storage error boundary.
|
||||
|
||||
use super::ThreadStoreError;
|
||||
use codex_protocol::ThreadId;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[test]
|
||||
@@ -21,3 +22,40 @@ fn unclassified_sqlx_failures_remain_internal() {
|
||||
assert_eq!(message, expected);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn missing_read_requires_the_requested_thread_and_a_known_store_error() {
|
||||
let requested = ThreadId::new();
|
||||
let other = ThreadId::new();
|
||||
let missing_rollout = format!("no rollout found for thread id {requested}");
|
||||
for error in [
|
||||
ThreadStoreError::ThreadNotFound {
|
||||
thread_id: requested,
|
||||
},
|
||||
ThreadStoreError::InvalidRequest {
|
||||
message: missing_rollout.clone(),
|
||||
},
|
||||
] {
|
||||
assert!(error.is_missing_for_read(requested));
|
||||
assert!(!error.is_missing_for_read(other));
|
||||
}
|
||||
for error in [
|
||||
ThreadStoreError::InvalidRequest {
|
||||
message: format!("{missing_rollout} (lookup failed)"),
|
||||
},
|
||||
ThreadStoreError::InvalidRequest {
|
||||
message: format!("thread {requested} is archived"),
|
||||
},
|
||||
ThreadStoreError::Retryable {
|
||||
message: missing_rollout.clone(),
|
||||
},
|
||||
ThreadStoreError::Internal {
|
||||
message: missing_rollout,
|
||||
},
|
||||
ThreadStoreError::Unsupported {
|
||||
operation: "read_thread",
|
||||
},
|
||||
] {
|
||||
assert!(!error.is_missing_for_read(requested), "{error:?}");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -61,6 +61,7 @@ pub use projects::UpdateProjectParams;
|
||||
pub use projects::UpdatedProject;
|
||||
pub use queue_store::LocalQueueStore;
|
||||
pub use queue_store::QueueStore;
|
||||
pub use store::PendingDaybreakPreference;
|
||||
pub use store::PersistContext;
|
||||
pub use store::ThreadStore;
|
||||
pub use store::ThreadStoreFuture;
|
||||
|
||||
@@ -232,6 +232,63 @@ async fn naming_empty_paginated_thread_propagates_rollout_persistence_failure()
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn saved_daybreak_preference_survives_materialization_and_prefers_the_stored_choice() {
|
||||
let (_home, store, _runtime) = store_with_runtime().await;
|
||||
for (staged, persisted) in [
|
||||
(Some(false), Some(true)),
|
||||
(Some(true), Some(false)),
|
||||
(None, None),
|
||||
] {
|
||||
let thread_id = ThreadId::new();
|
||||
if staged.is_some() {
|
||||
store
|
||||
.stage_pending_thread_metadata(
|
||||
thread_id,
|
||||
ThreadMetadataPatch {
|
||||
daybreak_enabled: staged,
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.await
|
||||
.expect("stage Daybreak preference");
|
||||
}
|
||||
let pending = crate::PendingDaybreakPreference::read(store.as_ref(), thread_id)
|
||||
.await
|
||||
.expect("read staged preference");
|
||||
let live = LiveThread::create(store.clone(), create_thread_params(thread_id))
|
||||
.await
|
||||
.expect("create thread");
|
||||
live.append_items(&[user_message_item("materialize the saved preference")])
|
||||
.await
|
||||
.expect("materialize thread");
|
||||
assert_pending_metadata_consumed(&store, thread_id).await;
|
||||
assert_eq!(pending.resolve(/*stored*/ None), staged);
|
||||
|
||||
if persisted.is_some() {
|
||||
store
|
||||
.update_thread_metadata(UpdateThreadMetadataParams {
|
||||
thread_id,
|
||||
patch: ThreadMetadataPatch {
|
||||
daybreak_enabled: persisted,
|
||||
..Default::default()
|
||||
},
|
||||
include_archived: false,
|
||||
})
|
||||
.await
|
||||
.expect("save new preference");
|
||||
}
|
||||
let row = live
|
||||
.read_thread(
|
||||
/*include_archived*/ false, /*include_history*/ false,
|
||||
)
|
||||
.await
|
||||
.expect("read stored preference");
|
||||
assert_eq!(row.daybreak_enabled, persisted);
|
||||
assert_eq!(pending.resolve(row.daybreak_enabled), persisted);
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn pending_thread_metadata_is_consumed_by_first_metadata_update() {
|
||||
let (_home, store, runtime) = store_with_runtime().await;
|
||||
|
||||
@@ -125,6 +125,22 @@ impl PersistContext {
|
||||
}
|
||||
}
|
||||
|
||||
/// A saved Daybreak choice staged for a thread before its stored row is materialized.
|
||||
pub struct PendingDaybreakPreference(Option<bool>);
|
||||
|
||||
impl PendingDaybreakPreference {
|
||||
/// Read staging before the stored row: materialization can consume the staged metadata.
|
||||
pub async fn read(store: &dyn ThreadStore, thread_id: ThreadId) -> ThreadStoreResult<Self> {
|
||||
let pending = store.read_pending_thread_metadata(thread_id).await?;
|
||||
Ok(Self(pending.and_then(|patch| patch.daybreak_enabled)))
|
||||
}
|
||||
|
||||
/// A stored choice, including explicit Off, takes precedence over the staged snapshot.
|
||||
pub fn resolve(&self, stored: Option<bool>) -> Option<bool> {
|
||||
stored.or(self.0)
|
||||
}
|
||||
}
|
||||
|
||||
/// Storage-neutral thread persistence boundary.
|
||||
pub trait ThreadStore: Any + Send + Sync {
|
||||
/// Return this store as [`Any`] for implementation-owned escape hatches.
|
||||
|
||||
Reference in New Issue
Block a user