mirror of
https://github.com/openai/codex.git
synced 2026-10-16 16:58:46 +00:00
Populate thread previews from delegated task inputs (#50462)
## Why Delegated turns can begin without a user message, leaving threads without a task preview. Populate the preview from delegated input so these threads are discoverable before a user follow-up. ## What changed - Extract previews from completed `create_thread` and `send_message_to_thread` outputs in the `codex_app` and `codex_tui` namespaces. - Unwrap recognized `<codex_delegation>` inputs and decode their escaped task text for display, preserving model content. Keep plain or unrecognized wrappers as text and ignore blank inputs. - Fill missing previews in state metadata extraction and live thread metadata updates without replacing existing previews or setting the title or `first_user_message`. ## Testing Add tests for delegation decoding and tool filtering, preview-only metadata updates, preservation of later user-message metadata, and database listing before a user follow-up in both legacy and paginated history modes. GitOrigin-RevId: f7b3c60ef75e08c76b4084d043e3967bf2bade22
This commit is contained in:
committed by
copyberry
parent
c6049ebd96
commit
09bced5ad9
@@ -0,0 +1,32 @@
|
||||
//! Display-only previews for recognized delegated inputs; model content is never rewritten.
|
||||
|
||||
use codex_protocol::items::FunctionCallOutputItem;
|
||||
|
||||
/// Extract the task text from a delegated input, ignoring unrelated tool outputs.
|
||||
pub fn delegated_output_preview(output: &FunctionCallOutputItem) -> Option<String> {
|
||||
if !matches!(output.namespace.as_deref(), Some("codex_app" | "codex_tui"))
|
||||
|| !matches!(
|
||||
output.name.as_str(),
|
||||
"create_thread" | "send_message_to_thread"
|
||||
)
|
||||
{
|
||||
return None;
|
||||
}
|
||||
let text = output.output.to_text()?;
|
||||
let preview = text
|
||||
.strip_prefix("<codex_delegation>\n <source_thread_id>")
|
||||
.and_then(|delegation| delegation.split_once("</source_thread_id>\n <input>"))
|
||||
.and_then(|(_, input)| input.strip_suffix("</input>\n</codex_delegation>"))
|
||||
.map(|input| {
|
||||
input
|
||||
.replace("<", "<")
|
||||
.replace(">", ">")
|
||||
.replace("&", "&")
|
||||
})
|
||||
.unwrap_or(text);
|
||||
(!preview.trim().is_empty()).then_some(preview)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "delegated_preview_tests.rs"]
|
||||
mod tests;
|
||||
@@ -0,0 +1,49 @@
|
||||
//! Verify delegated preview decoding and tool identity filtering.
|
||||
|
||||
use super::delegated_output_preview;
|
||||
use codex_protocol::items::FunctionCallOutputItem;
|
||||
use codex_protocol::models::FunctionCallOutputBody;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[test]
|
||||
fn delegated_previews_unwrap_only_recognized_inputs() {
|
||||
let wrapped = "<codex_delegation>\n <source_thread_id>source</source_thread_id>\n <input>Check <main> & &lt;literal&gt;</input>\n</codex_delegation>";
|
||||
for (namespace, name, text, expected) in [
|
||||
(
|
||||
Some("codex_app"),
|
||||
"create_thread",
|
||||
wrapped,
|
||||
Some("Check <main> & <literal>"),
|
||||
),
|
||||
(
|
||||
Some("codex_tui"),
|
||||
"send_message_to_thread",
|
||||
wrapped,
|
||||
Some("Check <main> & <literal>"),
|
||||
),
|
||||
(
|
||||
Some("codex_app"),
|
||||
"create_thread",
|
||||
"plain <text>",
|
||||
Some("plain <text>"),
|
||||
),
|
||||
(Some("codex_tui"), "create_thread", " ", None),
|
||||
(Some("other"), "create_thread", wrapped, None),
|
||||
(None, "create_thread", wrapped, None),
|
||||
(Some("codex_app"), "shell", wrapped, None),
|
||||
(
|
||||
Some("codex_app"),
|
||||
"create_thread",
|
||||
"<codex_delegation>incomplete",
|
||||
Some("<codex_delegation>incomplete"),
|
||||
),
|
||||
] {
|
||||
let output = FunctionCallOutputItem {
|
||||
id: "output".to_string(),
|
||||
name: name.to_string(),
|
||||
namespace: namespace.map(str::to_string),
|
||||
output: FunctionCallOutputBody::Text(text.to_string()),
|
||||
};
|
||||
assert_eq!(delegated_output_preview(&output).as_deref(), expected);
|
||||
}
|
||||
}
|
||||
@@ -59,7 +59,10 @@ pub fn rollout_item_affects_thread_metadata(item: &RolloutItem) -> bool {
|
||||
| EventMsg::ThreadSettingsApplied(_),
|
||||
) => true,
|
||||
RolloutItem::EventMsg(EventMsg::ItemCompleted(event))
|
||||
if matches!(event.item, TurnItem::UserMessage(_)) =>
|
||||
if matches!(
|
||||
event.item,
|
||||
TurnItem::UserMessage(_) | TurnItem::FunctionCallOutput(_)
|
||||
) =>
|
||||
{
|
||||
true
|
||||
}
|
||||
@@ -142,6 +145,12 @@ fn apply_event_msg(metadata: &mut ThreadMetadata, event: &EventMsg) {
|
||||
&& !metadata_is_guardian_review(metadata)
|
||||
{
|
||||
apply_user_message(metadata, &user.as_legacy_user_message_event());
|
||||
} else if let TurnItem::FunctionCallOutput(output) = &event.item
|
||||
&& !metadata_is_guardian_review(metadata)
|
||||
{
|
||||
// Delegated turns have no user message, but must still be discoverable.
|
||||
let preview = crate::delegated_output_preview(output);
|
||||
set_preview_if_empty(metadata, preview);
|
||||
}
|
||||
}
|
||||
EventMsg::ThreadGoalUpdated(event) if !metadata_is_guardian_review(metadata) => {
|
||||
@@ -289,6 +298,40 @@ mod tests {
|
||||
assert_eq!(metadata.title, "actual user request");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn delegated_turn_preview_preserves_user_message_metadata() {
|
||||
let mut metadata = metadata_for_test();
|
||||
let mut expected = metadata.clone();
|
||||
for text in [" ", "delegated task", "later output"] {
|
||||
let item = RolloutItem::EventMsg(EventMsg::ItemCompleted(ItemCompletedEvent {
|
||||
thread_id: metadata.id,
|
||||
turn_id: "turn-1".to_string(),
|
||||
item: TurnItem::FunctionCallOutput(codex_protocol::items::FunctionCallOutputItem {
|
||||
id: "output-1".to_string(),
|
||||
name: "create_thread".to_string(),
|
||||
namespace: Some("codex_app".to_string()),
|
||||
output: codex_protocol::models::FunctionCallOutputBody::Text(text.to_string()),
|
||||
}),
|
||||
started_at_ms: Some(0),
|
||||
completed_at_ms: 0,
|
||||
}));
|
||||
assert!(rollout_item_affects_thread_metadata(&item));
|
||||
apply_rollout_item(&mut metadata, &item, "test-provider");
|
||||
if text == "delegated task" {
|
||||
expected.preview = Some(text.to_string());
|
||||
}
|
||||
assert_eq!(metadata, expected);
|
||||
}
|
||||
let user_item = RolloutItem::EventMsg(EventMsg::UserMessage(UserMessageEvent {
|
||||
message: "actual user follow-up".to_string(),
|
||||
..Default::default()
|
||||
}));
|
||||
apply_rollout_item(&mut metadata, &user_item, "test-provider");
|
||||
expected.first_user_message = Some("actual user follow-up".to_string());
|
||||
expected.title = "actual user follow-up".to_string();
|
||||
assert_eq!(metadata, expected);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn completed_user_message_items_set_title_and_first_user_message() {
|
||||
let mut metadata = metadata_for_test();
|
||||
|
||||
@@ -10,6 +10,7 @@ const _: () = assert!(
|
||||
);
|
||||
|
||||
mod audit;
|
||||
mod delegated_preview;
|
||||
mod extract;
|
||||
pub mod log_db;
|
||||
mod migrations;
|
||||
@@ -39,6 +40,7 @@ pub use sqlite::SqliteConfig;
|
||||
|
||||
pub use audit::ThreadStateAuditRow;
|
||||
pub use audit::read_thread_state_audit_rows;
|
||||
pub use delegated_preview::delegated_output_preview;
|
||||
pub use extract::GUARDIAN_THREAD_PREVIEW;
|
||||
pub use extract::GUARDIAN_THREAD_TITLE;
|
||||
/// Low-level storage engine: useful for focused tests.
|
||||
|
||||
@@ -813,6 +813,8 @@ impl ThreadStore for LocalThreadStore {
|
||||
mod tests {
|
||||
#[path = "acquisition_tests.rs"]
|
||||
mod acquisition_tests;
|
||||
#[path = "delegated_preview_tests.rs"]
|
||||
mod delegated_preview_tests;
|
||||
use std::sync::Arc;
|
||||
|
||||
use codex_protocol::ThreadId;
|
||||
|
||||
@@ -0,0 +1,83 @@
|
||||
//! Delegated tasks are discoverable through database listing before any user message.
|
||||
|
||||
use super::*;
|
||||
use crate::SortDirection;
|
||||
use crate::ThreadSortKey;
|
||||
use codex_protocol::items::FunctionCallOutputItem;
|
||||
use codex_protocol::models::FunctionCallOutputBody;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[tokio::test]
|
||||
async fn delegated_thread_is_listed_before_user_followup() {
|
||||
for history_mode in [ThreadHistoryMode::Legacy, ThreadHistoryMode::Paginated] {
|
||||
let home = TempDir::new().expect("temp dir");
|
||||
let config = test_config(home.path());
|
||||
let runtime = codex_state::StateRuntime::init(
|
||||
config.sqlite.clone(),
|
||||
config.default_model_provider_id.clone(),
|
||||
)
|
||||
.await
|
||||
.expect("initialize state database");
|
||||
let store = Arc::new(LocalThreadStore::new(config, Some(runtime)));
|
||||
let thread_id = ThreadId::new();
|
||||
let mut params = create_thread_params(thread_id);
|
||||
params.history_mode = history_mode;
|
||||
let live_thread = LiveThread::create(store.clone(), params)
|
||||
.await
|
||||
.expect("create live thread");
|
||||
let preview = "Inspect <main> & report findings.";
|
||||
live_thread
|
||||
.append_items(&[RolloutItem::EventMsg(EventMsg::ItemCompleted(
|
||||
ItemCompletedEvent {
|
||||
thread_id,
|
||||
turn_id: "delegated-turn".to_string(),
|
||||
item: TurnItem::FunctionCallOutput(FunctionCallOutputItem {
|
||||
id: "delegated-output".to_string(),
|
||||
name: "create_thread".to_string(),
|
||||
namespace: Some("codex_app".to_string()),
|
||||
output: FunctionCallOutputBody::Text(
|
||||
"<codex_delegation>\n <source_thread_id>source</source_thread_id>\n <input>Inspect <main> & report findings.</input>\n</codex_delegation>".to_string(),
|
||||
),
|
||||
}),
|
||||
started_at_ms: Some(0),
|
||||
completed_at_ms: 0,
|
||||
},
|
||||
))])
|
||||
.await
|
||||
.expect("append delegated input");
|
||||
live_thread.flush().await.expect("persist delegated input");
|
||||
|
||||
let listed = store
|
||||
.list_threads(ListThreadsParams {
|
||||
page_size: 10,
|
||||
cursor: None,
|
||||
sort_key: ThreadSortKey::CreatedAt,
|
||||
sort_direction: SortDirection::Desc,
|
||||
allowed_sources: Vec::new(),
|
||||
model_providers: None,
|
||||
cwd_filters: None,
|
||||
section: None,
|
||||
project_id: None,
|
||||
archived: false,
|
||||
search_term: None,
|
||||
relation_filter: None,
|
||||
use_state_db_only: true,
|
||||
})
|
||||
.await
|
||||
.expect("list database threads");
|
||||
assert_eq!(
|
||||
listed
|
||||
.items
|
||||
.iter()
|
||||
.map(|thread| (
|
||||
thread.thread_id,
|
||||
thread.preview.as_str(),
|
||||
thread.first_user_message.as_deref(),
|
||||
))
|
||||
.collect::<Vec<_>>(),
|
||||
vec![(thread_id, preview, None)],
|
||||
"history mode: {history_mode:?}",
|
||||
);
|
||||
live_thread.shutdown().await.expect("shutdown live thread");
|
||||
}
|
||||
}
|
||||
@@ -27,6 +27,10 @@ use crate::types::canonical_history_mode_from_rollout_items;
|
||||
|
||||
const THREAD_UPDATED_AT_TOUCH_INTERVAL: Duration = Duration::from_secs(5);
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "thread_metadata_sync_preview_tests.rs"]
|
||||
mod preview_tests;
|
||||
|
||||
/// Live-thread helper that derives metadata updates from appended rollout items.
|
||||
///
|
||||
/// Stores receive raw rollout items plus explicit metadata patches. This helper
|
||||
@@ -302,6 +306,12 @@ impl ThreadMetadataSync {
|
||||
&user.as_legacy_user_message_event(),
|
||||
&mut update,
|
||||
);
|
||||
} else if let TurnItem::FunctionCallOutput(output) = &event.item
|
||||
&& !self.preview_seen
|
||||
&& let Some(preview) = codex_state::delegated_output_preview(output)
|
||||
{
|
||||
self.preview_seen = true;
|
||||
update.preview = Some(preview);
|
||||
}
|
||||
}
|
||||
RolloutItem::EventMsg(EventMsg::TokenCount(token_count)) => {
|
||||
|
||||
@@ -0,0 +1,60 @@
|
||||
//! Verify delegated inputs emit a preview-only live metadata patch exactly once.
|
||||
|
||||
use super::*;
|
||||
use crate::ThreadPersistenceMetadata;
|
||||
use codex_protocol::items::FunctionCallOutputItem;
|
||||
use codex_protocol::models::FunctionCallOutputBody;
|
||||
use codex_protocol::protocol::ItemCompletedEvent;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[test]
|
||||
fn delegated_output_emits_only_first_preview_in_live_patch() {
|
||||
let thread_id = ThreadId::new();
|
||||
let mut sync = ThreadMetadataSync::for_resume(
|
||||
&ResumeThreadParams {
|
||||
thread_id,
|
||||
rollout_path: None,
|
||||
history: None,
|
||||
history_revision: None,
|
||||
include_archived: false,
|
||||
metadata: ThreadPersistenceMetadata {
|
||||
cwd: None,
|
||||
model_provider: "test-provider".to_string(),
|
||||
memory_mode: ThreadMemoryMode::Enabled,
|
||||
},
|
||||
},
|
||||
/*metadata*/ None,
|
||||
);
|
||||
for (index, text) in ["first task", "later task"].into_iter().enumerate() {
|
||||
let item = RolloutItem::EventMsg(EventMsg::ItemCompleted(ItemCompletedEvent {
|
||||
thread_id,
|
||||
turn_id: "turn".to_string(),
|
||||
item: TurnItem::FunctionCallOutput(FunctionCallOutputItem {
|
||||
id: format!("output-{index}"),
|
||||
name: "create_thread".to_string(),
|
||||
namespace: Some("codex_tui".to_string()),
|
||||
output: FunctionCallOutputBody::Text(format!(
|
||||
"<codex_delegation>\n <source_thread_id>source</source_thread_id>\n <input>{text}</input>\n</codex_delegation>"
|
||||
)),
|
||||
}),
|
||||
started_at_ms: Some(0),
|
||||
completed_at_ms: 0,
|
||||
}));
|
||||
let update = sync.observe_appended_items(&[item]);
|
||||
if index == 0 {
|
||||
let update = update.expect("first delegated preview patch");
|
||||
let expected = ThreadMetadataPatch {
|
||||
preview: Some(text.to_string()),
|
||||
updated_at: update.patch.updated_at,
|
||||
..Default::default()
|
||||
};
|
||||
assert_eq!(
|
||||
serde_json::to_value(&update.patch).unwrap(),
|
||||
serde_json::to_value(expected).unwrap()
|
||||
);
|
||||
sync.mark_pending_update_applied(&update);
|
||||
} else {
|
||||
assert_eq!(update.and_then(|update| update.patch.preview), None);
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user