mirror of
https://github.com/openai/codex.git
synced 2026-10-27 18:48:37 +00:00
Add persisted sections for organizing threads (#35722)
## What changed - Replace the `isPinned` thread metadata and filters with an optional persisted `section` and `sectionId`. - Add the paginated `threadSection/list` app-server method so clients can discover sections even when they contain no threads. - Seed a stable `Pinned` section, validate section assignments, and support filtering for a specific section or for unsectioned threads. ## Testing - Cover section protocol serialization, listing and pagination, metadata updates, filtering, persistence, migration compatibility, and operation without SQLite state. GitOrigin-RevId: 7972b5471d29317b9387bfd90aa9f573f691ad4c
This commit is contained in:
committed by
copyberry
parent
9291a123fb
commit
85c6da1c79
@@ -633,6 +633,11 @@ client_request_definitions! {
|
||||
serialization: None,
|
||||
response: v2::ThreadListResponse,
|
||||
},
|
||||
ThreadSectionList => "threadSection/list" {
|
||||
params: v2::ThreadSectionListParams,
|
||||
serialization: None,
|
||||
response: v2::ThreadSectionListResponse,
|
||||
},
|
||||
#[experimental("thread/search")]
|
||||
ThreadSearch => "thread/search" {
|
||||
params: v2::ThreadSearchParams,
|
||||
@@ -2730,7 +2735,7 @@ mod tests {
|
||||
parent_thread_id: None,
|
||||
preview: "first prompt".to_string(),
|
||||
ephemeral: true,
|
||||
is_pinned: false,
|
||||
section: None,
|
||||
history_mode: Default::default(),
|
||||
model_provider: "openai".to_string(),
|
||||
created_at: 1,
|
||||
@@ -2783,7 +2788,7 @@ mod tests {
|
||||
"parentThreadId": null,
|
||||
"preview": "first prompt",
|
||||
"ephemeral": true,
|
||||
"isPinned": false,
|
||||
"section": null,
|
||||
"historyMode": "legacy",
|
||||
"modelProvider": "openai",
|
||||
"createdAt": 1,
|
||||
|
||||
@@ -185,7 +185,10 @@ fn thread_resume_response_round_trips_initial_turns_page() {
|
||||
parent_thread_id: None,
|
||||
preview: String::new(),
|
||||
ephemeral: false,
|
||||
is_pinned: true,
|
||||
section: Some(ThreadSection {
|
||||
id: "01984de2-8f74-7c91-a3b2-5c5e937cf318".to_string(),
|
||||
name: "Pinned".to_string(),
|
||||
}),
|
||||
history_mode: Default::default(),
|
||||
model_provider: "openai".to_string(),
|
||||
created_at: 1,
|
||||
@@ -226,16 +229,22 @@ fn thread_resume_response_round_trips_initial_turns_page() {
|
||||
};
|
||||
|
||||
let value = serde_json::to_value(&response).expect("serialize thread resume response");
|
||||
assert_eq!(value["thread"]["isPinned"], json!(true));
|
||||
assert_eq!(
|
||||
value["thread"]["section"],
|
||||
json!({
|
||||
"id": "01984de2-8f74-7c91-a3b2-5c5e937cf318",
|
||||
"name": "Pinned",
|
||||
})
|
||||
);
|
||||
|
||||
let mut legacy_thread = value["thread"].clone();
|
||||
legacy_thread
|
||||
.as_object_mut()
|
||||
.expect("serialized thread should be an object")
|
||||
.remove("isPinned");
|
||||
.remove("section");
|
||||
let legacy_thread =
|
||||
serde_json::from_value::<Thread>(legacy_thread).expect("deserialize legacy thread");
|
||||
assert!(!legacy_thread.is_pinned);
|
||||
assert_eq!(legacy_thread.section, None);
|
||||
|
||||
assert_eq!(
|
||||
value.get("initialTurnsPage"),
|
||||
@@ -369,39 +378,122 @@ fn thread_list_params_accepts_state_db_only_flag() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn thread_list_params_accepts_pinned_filter() {
|
||||
for is_pinned in [true, false] {
|
||||
fn thread_list_params_accepts_section_id_filter() {
|
||||
for section_id in [
|
||||
"01984de2-8f74-7c91-a3b2-5c5e937cf318",
|
||||
"01984de2-8f74-7c91-a3b2-5c5e937cf319",
|
||||
] {
|
||||
let params = serde_json::from_value::<ThreadListParams>(json!({
|
||||
"isPinned": is_pinned,
|
||||
"sectionId": section_id,
|
||||
}))
|
||||
.expect("pinned filter should deserialize");
|
||||
.expect("section ID filter should deserialize");
|
||||
|
||||
assert_eq!(params.is_pinned, Some(is_pinned));
|
||||
assert_eq!(
|
||||
params.section_id.as_ref().map(|section| section.as_deref()),
|
||||
Some(Some(section_id))
|
||||
);
|
||||
assert_eq!(
|
||||
serde_json::to_value(¶ms)
|
||||
.expect("section ID filter should serialize")
|
||||
.get("sectionId"),
|
||||
Some(&json!(section_id))
|
||||
);
|
||||
}
|
||||
|
||||
let params = serde_json::from_value::<ThreadListParams>(json!({
|
||||
"sectionId": null,
|
||||
}))
|
||||
.expect("unsectioned thread filter should deserialize");
|
||||
assert_eq!(params.section_id, Some(None));
|
||||
assert_eq!(
|
||||
serde_json::to_value(¶ms)
|
||||
.expect("unsectioned thread filter should serialize")
|
||||
.get("sectionId"),
|
||||
Some(&json!(null))
|
||||
);
|
||||
|
||||
let params = serde_json::from_value::<ThreadListParams>(json!({}))
|
||||
.expect("omitted pinned filter should deserialize");
|
||||
assert_eq!(params.is_pinned, None);
|
||||
.expect("omitted section ID filter should deserialize");
|
||||
assert_eq!(params.section_id, None);
|
||||
assert!(
|
||||
serde_json::to_value(¶ms)
|
||||
.expect("omitted section ID filter should serialize")
|
||||
.get("sectionId")
|
||||
.is_none()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn thread_metadata_update_params_accepts_pinned_patch() {
|
||||
for is_pinned in [true, false] {
|
||||
fn thread_metadata_update_params_distinguish_section_id_set_clear_and_omission() {
|
||||
for section_id in [
|
||||
"01984de2-8f74-7c91-a3b2-5c5e937cf318",
|
||||
"01984de2-8f74-7c91-a3b2-5c5e937cf319",
|
||||
] {
|
||||
let params = serde_json::from_value::<ThreadMetadataUpdateParams>(json!({
|
||||
"threadId": "thr_123",
|
||||
"isPinned": is_pinned,
|
||||
"sectionId": section_id,
|
||||
}))
|
||||
.expect("pinned metadata patch should deserialize");
|
||||
.expect("section ID metadata patch should deserialize");
|
||||
|
||||
assert_eq!(params.is_pinned, Some(is_pinned));
|
||||
assert_eq!(
|
||||
params.section_id.as_ref().map(|section| section.as_deref()),
|
||||
Some(Some(section_id))
|
||||
);
|
||||
assert_eq!(params.git_info, None);
|
||||
}
|
||||
|
||||
let params = serde_json::from_value::<ThreadMetadataUpdateParams>(json!({
|
||||
"threadId": "thr_123",
|
||||
"sectionId": null,
|
||||
}))
|
||||
.expect("omitted pinned metadata patch should deserialize");
|
||||
assert_eq!(params.is_pinned, None);
|
||||
.expect("cleared section ID metadata patch should deserialize");
|
||||
assert_eq!(params.section_id, Some(None));
|
||||
|
||||
let params = serde_json::from_value::<ThreadMetadataUpdateParams>(json!({
|
||||
"threadId": "thr_123",
|
||||
}))
|
||||
.expect("omitted section ID metadata patch should deserialize");
|
||||
assert_eq!(params.section_id, None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn thread_section_list_params_and_response_round_trip() {
|
||||
let params = serde_json::from_value::<ThreadSectionListParams>(json!({
|
||||
"cursor": "section-cursor",
|
||||
"limit": 25,
|
||||
}))
|
||||
.expect("section list parameters should deserialize");
|
||||
assert_eq!(
|
||||
params,
|
||||
ThreadSectionListParams {
|
||||
cursor: Some("section-cursor".to_string()),
|
||||
limit: Some(25),
|
||||
}
|
||||
);
|
||||
|
||||
let response = ThreadSectionListResponse {
|
||||
data: vec![ThreadSection {
|
||||
id: "01984de2-8f74-7c91-a3b2-5c5e937cf318".to_string(),
|
||||
name: "Pinned".to_string(),
|
||||
}],
|
||||
next_cursor: None,
|
||||
};
|
||||
let value = serde_json::to_value(&response).expect("section list should serialize");
|
||||
assert_eq!(
|
||||
value,
|
||||
json!({
|
||||
"data": [{
|
||||
"id": "01984de2-8f74-7c91-a3b2-5c5e937cf318",
|
||||
"name": "Pinned",
|
||||
}],
|
||||
"nextCursor": null,
|
||||
})
|
||||
);
|
||||
assert_eq!(
|
||||
serde_json::from_value::<ThreadSectionListResponse>(value)
|
||||
.expect("section list should deserialize"),
|
||||
response
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -6,6 +6,7 @@ use super::SandboxPolicy;
|
||||
use super::Thread;
|
||||
use super::ThreadHistoryMode;
|
||||
use super::ThreadItem;
|
||||
use super::ThreadSection;
|
||||
use super::ThreadSource;
|
||||
use super::Turn;
|
||||
use super::TurnEnvironmentParams;
|
||||
@@ -865,9 +866,15 @@ pub struct ThreadMetadataUpdateParams {
|
||||
/// provide a string to replace the stored value.
|
||||
#[ts(optional = nullable)]
|
||||
pub git_info: Option<ThreadMetadataGitInfoUpdateParams>,
|
||||
/// Patch whether this thread is pinned. Omit to leave the stored value unchanged.
|
||||
#[ts(optional = nullable)]
|
||||
pub is_pinned: Option<bool>,
|
||||
/// Omit to leave the section unchanged, set to `null` to clear it, or provide a section ID.
|
||||
#[serde(
|
||||
default,
|
||||
skip_serializing_if = "Option::is_none",
|
||||
serialize_with = "crate::protocol::serde_helpers::serialize_double_option",
|
||||
deserialize_with = "crate::protocol::serde_helpers::deserialize_double_option"
|
||||
)]
|
||||
#[ts(optional = nullable, type = "string | null")]
|
||||
pub section_id: Option<Option<String>>,
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
|
||||
@@ -1093,6 +1100,29 @@ pub struct ThreadRollbackResponse {
|
||||
pub thread: Thread,
|
||||
}
|
||||
|
||||
/// Parameters for listing independently persisted thread sections.
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq, Eq, JsonSchema, TS)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[ts(export_to = "v2/")]
|
||||
pub struct ThreadSectionListParams {
|
||||
/// Opaque pagination cursor returned by a previous call.
|
||||
#[ts(optional = nullable)]
|
||||
pub cursor: Option<String>,
|
||||
/// Maximum number of sections to return.
|
||||
#[ts(optional = nullable)]
|
||||
pub limit: Option<u32>,
|
||||
}
|
||||
|
||||
/// One page of independently persisted thread sections.
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[ts(export_to = "v2/")]
|
||||
pub struct ThreadSectionListResponse {
|
||||
pub data: Vec<ThreadSection>,
|
||||
/// Opaque cursor for the next page, or `null` when no sections remain.
|
||||
pub next_cursor: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS, ExperimentalApi)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[ts(export_to = "v2/")]
|
||||
@@ -1121,9 +1151,16 @@ pub struct ThreadListParams {
|
||||
/// If false or null, only non-archived threads are returned.
|
||||
#[ts(optional = nullable)]
|
||||
pub archived: Option<bool>,
|
||||
/// Optional pinned filter; when set, only threads matching this value are returned.
|
||||
#[ts(optional = nullable)]
|
||||
pub is_pinned: Option<bool>,
|
||||
/// Omit to include every section, set to `null` for unsectioned threads,
|
||||
/// or provide a section ID to return only threads in that section.
|
||||
#[serde(
|
||||
default,
|
||||
skip_serializing_if = "Option::is_none",
|
||||
serialize_with = "crate::protocol::serde_helpers::serialize_double_option",
|
||||
deserialize_with = "crate::protocol::serde_helpers::deserialize_double_option"
|
||||
)]
|
||||
#[ts(optional = nullable, type = "string | null")]
|
||||
pub section_id: Option<Option<String>>,
|
||||
/// Optional cwd filter or filters; when set, only threads whose session cwd
|
||||
/// exactly matches one of these paths are returned.
|
||||
#[ts(optional = nullable, type = "string | Array<string> | null")]
|
||||
|
||||
@@ -164,6 +164,17 @@ pub struct GitInfo {
|
||||
pub origin_url: Option<String>,
|
||||
}
|
||||
|
||||
/// An independently persisted, user-visible thread section.
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[ts(export_to = "v2/")]
|
||||
pub struct ThreadSection {
|
||||
/// Opaque UUIDv7 identity that remains stable when the section is renamed.
|
||||
pub id: String,
|
||||
/// The current user-visible section name.
|
||||
pub name: String,
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS, ExperimentalApi)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[ts(export_to = "v2/")]
|
||||
@@ -183,9 +194,9 @@ pub struct Thread {
|
||||
pub preview: String,
|
||||
/// Whether the thread is ephemeral and should not be materialized on disk.
|
||||
pub ephemeral: bool,
|
||||
/// Whether the thread has been pinned by the user.
|
||||
/// The independently persisted section selected for this thread, if any.
|
||||
#[serde(default)]
|
||||
pub is_pinned: bool,
|
||||
pub section: Option<ThreadSection>,
|
||||
/// Persisted thread history contract selected when this thread was created.
|
||||
#[experimental("thread.historyMode")]
|
||||
#[serde(default)]
|
||||
|
||||
Reference in New Issue
Block a user