mirror of
https://github.com/openai/codex.git
synced 2026-11-13 14:46:14 +00:00
Use goal preview metadata for goal-first threads (#21981)
Fixes #20792 ## Why `/goal`-first threads are valid resumable threads, but they can be missing from `codex resume` and app recents because discovery depends on metadata derived from a normal first user message. PR #21489 attempted to fix this by using the goal objective as `first_user_message`. Review feedback pointed out that `first_user_message` does more than provide visible text today: it gates listing, supplies preview text, and participates in deciding whether a later title should surface as a distinct thread name. Reusing it for the goal objective could leave a `/goal`-first thread with `first_user_message=<goal>` and `title=<later prompt>`, even though the goal should only provide the initial visible preview. This PR follows that feedback by and keeps the `first_user_message` as is but introduces a new `preview` field to separate concerns. The `preview` field is populated from the first user message or the goal objective. We can extend it in the future to include other sources. ## What Changed - Added internal thread `preview` metadata in `codex-state`, including a SQLite migration that backfills from `first_user_message` and from existing `thread_goals` objectives when needed. - Treated `ThreadGoalUpdated` as preview-bearing metadata so goal-first threads can be listed and searched without mutating `first_user_message`. - Updated rollout listing, state queries, thread-store conversion, and app-server mapping to use preview metadata while continuing to expose the existing public `preview` field. - Preserved title/name distinctness behavior around literal `first_user_message`, so a later normal prompt after `/goal` does not surface as a separate name just because the goal supplied the initial preview. - Preserved compatibility for older/internal metadata writes by deriving preview from `first_user_message` when explicit preview metadata is absent. ## Verification - Manually verified that a thread that starts with a `/goal <objective>` shows up in the resume picker.
This commit is contained in:
1 parent
96836e15ed
commit
f10ddc3f13
18 files changed
+499
-33
No files matched your search
@@ -33,7 +33,9 @@ pub fn apply_rollout_item(
|
||||
pub fn rollout_item_affects_thread_metadata(item: &RolloutItem) -> bool {
|
||||
match item {
|
||||
RolloutItem::SessionMeta(_) | RolloutItem::TurnContext(_) => true,
|
||||
RolloutItem::EventMsg(EventMsg::TokenCount(_) | EventMsg::UserMessage(_)) => true,
|
||||
RolloutItem::EventMsg(
|
||||
EventMsg::TokenCount(_) | EventMsg::UserMessage(_) | EventMsg::ThreadGoalUpdated(_),
|
||||
) => true,
|
||||
RolloutItem::EventMsg(_) | RolloutItem::ResponseItem(_) | RolloutItem::Compacted(_) => {
|
||||
false
|
||||
}
|
||||
@@ -86,9 +88,11 @@ fn apply_event_msg(metadata: &mut ThreadMetadata, event: &EventMsg) {
|
||||
}
|
||||
}
|
||||
EventMsg::UserMessage(user) => {
|
||||
let preview = user_message_preview(user);
|
||||
if metadata.first_user_message.is_none() {
|
||||
metadata.first_user_message = user_message_preview(user);
|
||||
metadata.first_user_message = preview.clone();
|
||||
}
|
||||
set_preview_if_empty(metadata, preview);
|
||||
if metadata.title.is_empty() {
|
||||
let title = strip_user_message_prefix(user.message.as_str());
|
||||
if !title.is_empty() {
|
||||
@@ -96,12 +100,24 @@ fn apply_event_msg(metadata: &mut ThreadMetadata, event: &EventMsg) {
|
||||
}
|
||||
}
|
||||
}
|
||||
EventMsg::ThreadGoalUpdated(event) => {
|
||||
let objective = event.goal.objective.trim();
|
||||
if !objective.is_empty() {
|
||||
set_preview_if_empty(metadata, Some(objective.to_string()));
|
||||
}
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
|
||||
fn apply_response_item(_metadata: &mut ThreadMetadata, _item: &ResponseItem) {}
|
||||
|
||||
fn set_preview_if_empty(metadata: &mut ThreadMetadata, preview: Option<String>) {
|
||||
if metadata.preview.is_none() {
|
||||
metadata.preview = preview;
|
||||
}
|
||||
}
|
||||
|
||||
fn strip_user_message_prefix(text: &str) -> &str {
|
||||
match text.find(USER_MESSAGE_BEGIN) {
|
||||
Some(idx) => text[idx + USER_MESSAGE_BEGIN.len()..].trim(),
|
||||
@@ -151,6 +167,9 @@ mod tests {
|
||||
use codex_protocol::protocol::SessionMeta;
|
||||
use codex_protocol::protocol::SessionMetaLine;
|
||||
use codex_protocol::protocol::SessionSource;
|
||||
use codex_protocol::protocol::ThreadGoal;
|
||||
use codex_protocol::protocol::ThreadGoalStatus;
|
||||
use codex_protocol::protocol::ThreadGoalUpdatedEvent;
|
||||
use codex_protocol::protocol::TurnContextItem;
|
||||
use codex_protocol::protocol::USER_MESSAGE_BEGIN;
|
||||
use codex_protocol::protocol::UserMessageEvent;
|
||||
@@ -174,6 +193,7 @@ mod tests {
|
||||
apply_rollout_item(&mut metadata, &item, "test-provider");
|
||||
|
||||
assert_eq!(metadata.first_user_message, None);
|
||||
assert_eq!(metadata.preview, None);
|
||||
assert_eq!(metadata.title, "");
|
||||
}
|
||||
|
||||
@@ -193,6 +213,7 @@ mod tests {
|
||||
metadata.first_user_message.as_deref(),
|
||||
Some("actual user request")
|
||||
);
|
||||
assert_eq!(metadata.preview.as_deref(), Some("actual user request"));
|
||||
assert_eq!(metadata.title, "actual user request");
|
||||
}
|
||||
|
||||
@@ -212,6 +233,10 @@ mod tests {
|
||||
metadata.first_user_message.as_deref(),
|
||||
Some(super::IMAGE_ONLY_USER_MESSAGE_PLACEHOLDER)
|
||||
);
|
||||
assert_eq!(
|
||||
metadata.preview.as_deref(),
|
||||
Some(super::IMAGE_ONLY_USER_MESSAGE_PLACEHOLDER)
|
||||
);
|
||||
assert_eq!(metadata.title, "");
|
||||
}
|
||||
|
||||
@@ -228,9 +253,52 @@ mod tests {
|
||||
apply_rollout_item(&mut metadata, &item, "test-provider");
|
||||
|
||||
assert_eq!(metadata.first_user_message, None);
|
||||
assert_eq!(metadata.preview, None);
|
||||
assert_eq!(metadata.title, "");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn event_msg_thread_goal_sets_preview_only_and_later_user_sets_message_title() {
|
||||
let mut metadata = metadata_for_test();
|
||||
let goal_item =
|
||||
RolloutItem::EventMsg(EventMsg::ThreadGoalUpdated(ThreadGoalUpdatedEvent {
|
||||
thread_id: metadata.id,
|
||||
turn_id: None,
|
||||
goal: ThreadGoal {
|
||||
thread_id: metadata.id,
|
||||
objective: "optimize the benchmark".to_string(),
|
||||
status: ThreadGoalStatus::Active,
|
||||
token_budget: None,
|
||||
tokens_used: 0,
|
||||
time_used_seconds: 0,
|
||||
created_at: 1,
|
||||
updated_at: 1,
|
||||
},
|
||||
}));
|
||||
|
||||
apply_rollout_item(&mut metadata, &goal_item, "test-provider");
|
||||
|
||||
assert_eq!(metadata.preview.as_deref(), Some("optimize the benchmark"));
|
||||
assert_eq!(metadata.first_user_message, None);
|
||||
assert_eq!(metadata.title, "");
|
||||
|
||||
let user_item = RolloutItem::EventMsg(EventMsg::UserMessage(UserMessageEvent {
|
||||
message: format!("{USER_MESSAGE_BEGIN} next normal prompt"),
|
||||
images: Some(vec![]),
|
||||
local_images: vec![],
|
||||
text_elements: vec![],
|
||||
}));
|
||||
|
||||
apply_rollout_item(&mut metadata, &user_item, "test-provider");
|
||||
|
||||
assert_eq!(metadata.preview.as_deref(), Some("optimize the benchmark"));
|
||||
assert_eq!(
|
||||
metadata.first_user_message.as_deref(),
|
||||
Some("next normal prompt")
|
||||
);
|
||||
assert_eq!(metadata.title, "next normal prompt");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn turn_context_does_not_override_session_cwd() {
|
||||
let mut metadata = metadata_for_test();
|
||||
@@ -421,6 +489,7 @@ mod tests {
|
||||
cwd: PathBuf::from("/tmp"),
|
||||
cli_version: "0.0.0".to_string(),
|
||||
title: String::new(),
|
||||
preview: None,
|
||||
sandbox_policy: "read-only".to_string(),
|
||||
approval_mode: "on-request".to_string(),
|
||||
tokens_used: 1,
|
||||
|
||||
Reference in new issue
Block a user