Switch to uuid_v7 and tighten ConversationId usage (#3819)
Make sure conversations have a timestamp.
This commit is contained in:
@@ -4,9 +4,9 @@ name = "codex-core"
|
|||||||
version = { workspace = true }
|
version = { workspace = true }
|
||||||
|
|
||||||
[lib]
|
[lib]
|
||||||
|
doctest = false
|
||||||
name = "codex_core"
|
name = "codex_core"
|
||||||
path = "src/lib.rs"
|
path = "src/lib.rs"
|
||||||
doctest = false
|
|
||||||
|
|
||||||
[lints]
|
[lints]
|
||||||
workspace = true
|
workspace = true
|
||||||
@@ -41,7 +41,12 @@ similar = "2.7.0"
|
|||||||
strum_macros = "0.27.2"
|
strum_macros = "0.27.2"
|
||||||
tempfile = "3"
|
tempfile = "3"
|
||||||
thiserror = "2.0.16"
|
thiserror = "2.0.16"
|
||||||
time = { version = "0.3", features = ["formatting", "parsing", "local-offset", "macros"] }
|
time = { version = "0.3", features = [
|
||||||
|
"formatting",
|
||||||
|
"parsing",
|
||||||
|
"local-offset",
|
||||||
|
"macros",
|
||||||
|
] }
|
||||||
tokio = { version = "1", features = [
|
tokio = { version = "1", features = [
|
||||||
"io-std",
|
"io-std",
|
||||||
"macros",
|
"macros",
|
||||||
|
|||||||
@@ -36,7 +36,7 @@ tokio = { version = "1", features = [
|
|||||||
toml = "0.9"
|
toml = "0.9"
|
||||||
tracing = { version = "0.1.41", features = ["log"] }
|
tracing = { version = "0.1.41", features = ["log"] }
|
||||||
tracing-subscriber = { version = "0.3", features = ["env-filter", "fmt"] }
|
tracing-subscriber = { version = "0.3", features = ["env-filter", "fmt"] }
|
||||||
uuid = { version = "1", features = ["serde", "v4"] }
|
uuid = { version = "1", features = ["serde", "v7"] }
|
||||||
|
|
||||||
[dev-dependencies]
|
[dev-dependencies]
|
||||||
assert_cmd = "2"
|
assert_cmd = "2"
|
||||||
|
|||||||
@@ -814,7 +814,7 @@ impl CodexMessageProcessor {
|
|||||||
return;
|
return;
|
||||||
};
|
};
|
||||||
|
|
||||||
let required_suffix = format!("{}.jsonl", conversation_id.0);
|
let required_suffix = format!("{conversation_id}.jsonl");
|
||||||
let Some(file_name) = canonical_rollout_path.file_name().map(OsStr::to_owned) else {
|
let Some(file_name) = canonical_rollout_path.file_name().map(OsStr::to_owned) else {
|
||||||
let error = JSONRPCErrorError {
|
let error = JSONRPCErrorError {
|
||||||
code: INVALID_REQUEST_ERROR_CODE,
|
code: INVALID_REQUEST_ERROR_CODE,
|
||||||
@@ -1414,13 +1414,13 @@ mod tests {
|
|||||||
#[test]
|
#[test]
|
||||||
fn extract_conversation_summary_prefers_plain_user_messages() {
|
fn extract_conversation_summary_prefers_plain_user_messages() {
|
||||||
let conversation_id =
|
let conversation_id =
|
||||||
ConversationId(Uuid::parse_str("3f941c35-29b3-493b-b0a4-e25800d9aeb0").unwrap());
|
ConversationId::from_string("3f941c35-29b3-493b-b0a4-e25800d9aeb0").unwrap();
|
||||||
let timestamp = Some("2025-09-05T16:53:11.850Z".to_string());
|
let timestamp = Some("2025-09-05T16:53:11.850Z".to_string());
|
||||||
let path = PathBuf::from("rollout.jsonl");
|
let path = PathBuf::from("rollout.jsonl");
|
||||||
|
|
||||||
let head = vec![
|
let head = vec![
|
||||||
json!({
|
json!({
|
||||||
"id": conversation_id.0,
|
"id": conversation_id.to_string(),
|
||||||
"timestamp": timestamp,
|
"timestamp": timestamp,
|
||||||
"cwd": "/",
|
"cwd": "/",
|
||||||
"originator": "codex",
|
"originator": "codex",
|
||||||
|
|||||||
@@ -36,7 +36,6 @@ use serde_json::json;
|
|||||||
use std::sync::Arc;
|
use std::sync::Arc;
|
||||||
use tokio::sync::Mutex;
|
use tokio::sync::Mutex;
|
||||||
use tokio::task;
|
use tokio::task;
|
||||||
use uuid::Uuid;
|
|
||||||
|
|
||||||
pub(crate) struct MessageProcessor {
|
pub(crate) struct MessageProcessor {
|
||||||
codex_message_processor: CodexMessageProcessor,
|
codex_message_processor: CodexMessageProcessor,
|
||||||
@@ -484,8 +483,8 @@ impl MessageProcessor {
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
let conversation_id = match Uuid::parse_str(&conversation_id) {
|
let conversation_id = match ConversationId::from_string(&conversation_id) {
|
||||||
Ok(id) => ConversationId::from(id),
|
Ok(id) => id,
|
||||||
Err(e) => {
|
Err(e) => {
|
||||||
tracing::error!("Failed to parse conversation_id: {e}");
|
tracing::error!("Failed to parse conversation_id: {e}");
|
||||||
let result = CallToolResult {
|
let result = CallToolResult {
|
||||||
|
|||||||
@@ -142,7 +142,7 @@ async fn test_list_and_resume_conversations() {
|
|||||||
} = to_response::<ResumeConversationResponse>(resume_resp)
|
} = to_response::<ResumeConversationResponse>(resume_resp)
|
||||||
.expect("deserialize resumeConversation response");
|
.expect("deserialize resumeConversation response");
|
||||||
// conversation id should be a valid UUID
|
// conversation id should be a valid UUID
|
||||||
let _: uuid::Uuid = conversation_id.into();
|
assert!(!conversation_id.to_string().is_empty());
|
||||||
}
|
}
|
||||||
|
|
||||||
fn create_fake_rollout(codex_home: &Path, filename_ts: &str, meta_rfc3339: &str, preview: &str) {
|
fn create_fake_rollout(codex_home: &Path, filename_ts: &str, meta_rfc3339: &str, preview: &str) {
|
||||||
|
|||||||
@@ -136,7 +136,7 @@ async fn test_send_message_session_not_found() {
|
|||||||
.expect("timeout")
|
.expect("timeout")
|
||||||
.expect("init");
|
.expect("init");
|
||||||
|
|
||||||
let unknown = ConversationId(uuid::Uuid::new_v4());
|
let unknown = ConversationId::new();
|
||||||
let req_id = mcp
|
let req_id = mcp
|
||||||
.send_send_user_message_request(SendUserMessageParams {
|
.send_send_user_message_request(SendUserMessageParams {
|
||||||
conversation_id: unknown,
|
conversation_id: unknown,
|
||||||
|
|||||||
@@ -23,8 +23,12 @@ strum = "0.27.2"
|
|||||||
strum_macros = "0.27.2"
|
strum_macros = "0.27.2"
|
||||||
sys-locale = "0.3.2"
|
sys-locale = "0.3.2"
|
||||||
tracing = "0.1.41"
|
tracing = "0.1.41"
|
||||||
ts-rs = { version = "11", features = ["uuid-impl", "serde-json-impl", "no-serde-warnings"] }
|
ts-rs = { version = "11", features = [
|
||||||
uuid = { version = "1", features = ["serde", "v4"] }
|
"uuid-impl",
|
||||||
|
"serde-json-impl",
|
||||||
|
"no-serde-warnings",
|
||||||
|
] }
|
||||||
|
uuid = { version = "1", features = ["serde", "v7"] }
|
||||||
|
|
||||||
[dev-dependencies]
|
[dev-dependencies]
|
||||||
pretty_assertions = "1.4.1"
|
pretty_assertions = "1.4.1"
|
||||||
|
|||||||
@@ -19,13 +19,23 @@ use strum_macros::Display;
|
|||||||
use ts_rs::TS;
|
use ts_rs::TS;
|
||||||
use uuid::Uuid;
|
use uuid::Uuid;
|
||||||
|
|
||||||
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, TS, Hash)]
|
#[derive(Debug, Clone, Copy, PartialEq, Eq, TS, Hash)]
|
||||||
#[ts(type = "string")]
|
#[ts(type = "string")]
|
||||||
pub struct ConversationId(pub Uuid);
|
pub struct ConversationId {
|
||||||
|
uuid: Uuid,
|
||||||
|
}
|
||||||
|
|
||||||
impl ConversationId {
|
impl ConversationId {
|
||||||
pub fn new() -> Self {
|
pub fn new() -> Self {
|
||||||
Self(Uuid::new_v4())
|
Self {
|
||||||
|
uuid: Uuid::now_v7(),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
pub fn from_string(s: &str) -> Result<Self, uuid::Error> {
|
||||||
|
Ok(Self {
|
||||||
|
uuid: Uuid::parse_str(s)?,
|
||||||
|
})
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -37,19 +47,27 @@ impl Default for ConversationId {
|
|||||||
|
|
||||||
impl Display for ConversationId {
|
impl Display for ConversationId {
|
||||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||||
write!(f, "{}", self.0)
|
write!(f, "{}", self.uuid)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
impl From<Uuid> for ConversationId {
|
impl Serialize for ConversationId {
|
||||||
fn from(value: Uuid) -> Self {
|
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
|
||||||
Self(value)
|
where
|
||||||
|
S: serde::Serializer,
|
||||||
|
{
|
||||||
|
serializer.collect_str(&self.uuid)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
impl From<ConversationId> for Uuid {
|
impl<'de> Deserialize<'de> for ConversationId {
|
||||||
fn from(value: ConversationId) -> Self {
|
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
|
||||||
value.0
|
where
|
||||||
|
D: serde::Deserializer<'de>,
|
||||||
|
{
|
||||||
|
let value = String::deserialize(deserializer)?;
|
||||||
|
let uuid = Uuid::parse_str(&value).map_err(serde::de::Error::custom)?;
|
||||||
|
Ok(Self { uuid })
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -719,6 +737,27 @@ mod tests {
|
|||||||
#[test]
|
#[test]
|
||||||
fn test_conversation_id_default_is_not_zeroes() {
|
fn test_conversation_id_default_is_not_zeroes() {
|
||||||
let id = ConversationId::default();
|
let id = ConversationId::default();
|
||||||
assert_ne!(id.0, Uuid::nil());
|
assert_ne!(id.uuid, Uuid::nil());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn conversation_id_serializes_as_plain_string() {
|
||||||
|
let id = ConversationId::from_string("67e55044-10b1-426f-9247-bb680e5fe0c8").unwrap();
|
||||||
|
|
||||||
|
assert_eq!(
|
||||||
|
json!("67e55044-10b1-426f-9247-bb680e5fe0c8"),
|
||||||
|
serde_json::to_value(id).unwrap()
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn conversation_id_deserializes_from_plain_string() {
|
||||||
|
let id: ConversationId =
|
||||||
|
serde_json::from_value(json!("67e55044-10b1-426f-9247-bb680e5fe0c8")).unwrap();
|
||||||
|
|
||||||
|
assert_eq!(
|
||||||
|
ConversationId::from_string("67e55044-10b1-426f-9247-bb680e5fe0c8").unwrap(),
|
||||||
|
id,
|
||||||
|
);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1252,7 +1252,8 @@ mod tests {
|
|||||||
/// amount of nesting.
|
/// amount of nesting.
|
||||||
#[test]
|
#[test]
|
||||||
fn serialize_event() {
|
fn serialize_event() {
|
||||||
let conversation_id = ConversationId(uuid::uuid!("67e55044-10b1-426f-9247-bb680e5fe0c8"));
|
let conversation_id =
|
||||||
|
ConversationId::from_string("67e55044-10b1-426f-9247-bb680e5fe0c8").unwrap();
|
||||||
let rollout_file = NamedTempFile::new().unwrap();
|
let rollout_file = NamedTempFile::new().unwrap();
|
||||||
let event = Event {
|
let event = Event {
|
||||||
id: "1234".to_string(),
|
id: "1234".to_string(),
|
||||||
|
|||||||
Reference in New Issue
Block a user