[App Server] Allow fetching or resuming a conversation summary from the conversation id (#5890)
This PR adds an option to app server to allow conversation summaries to be fetched from just the conversation id rather than rollout path for convenience at the cost of some latency to discover the rollout path. This convenience is non-trivial as it allows app servers to simply maintain conversation ids rather than rollout paths and the associated platform (Windows) handling associated with storing and encoding them correctly.
This commit is contained in:
@@ -340,12 +340,24 @@ pub struct ResumeConversationResponse {
|
|||||||
pub model: String,
|
pub model: String,
|
||||||
#[serde(skip_serializing_if = "Option::is_none")]
|
#[serde(skip_serializing_if = "Option::is_none")]
|
||||||
pub initial_messages: Option<Vec<EventMsg>>,
|
pub initial_messages: Option<Vec<EventMsg>>,
|
||||||
|
pub rollout_path: PathBuf,
|
||||||
}
|
}
|
||||||
|
|
||||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
|
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
|
||||||
#[serde(rename_all = "camelCase")]
|
#[serde(untagged)]
|
||||||
pub struct GetConversationSummaryParams {
|
pub enum GetConversationSummaryParams {
|
||||||
pub rollout_path: PathBuf,
|
/// Provide the absolute or CODEX_HOME‑relative rollout path directly.
|
||||||
|
RolloutPath {
|
||||||
|
#[serde(rename = "rolloutPath")]
|
||||||
|
rollout_path: PathBuf,
|
||||||
|
},
|
||||||
|
/// Provide a conversation id; the server will locate the rollout using the
|
||||||
|
/// same logic as `resumeConversation`. There will be extra latency compared to using the rollout path,
|
||||||
|
/// as the server needs to locate the rollout path first.
|
||||||
|
ConversationId {
|
||||||
|
#[serde(rename = "conversationId")]
|
||||||
|
conversation_id: ConversationId,
|
||||||
|
},
|
||||||
}
|
}
|
||||||
|
|
||||||
#[derive(Serialize, Deserialize, Debug, Clone, JsonSchema, TS)]
|
#[derive(Serialize, Deserialize, Debug, Clone, JsonSchema, TS)]
|
||||||
@@ -487,8 +499,12 @@ pub struct LogoutAccountResponse {}
|
|||||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
|
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
|
||||||
#[serde(rename_all = "camelCase")]
|
#[serde(rename_all = "camelCase")]
|
||||||
pub struct ResumeConversationParams {
|
pub struct ResumeConversationParams {
|
||||||
/// Absolute path to the rollout JSONL file.
|
/// Absolute path to the rollout JSONL file. If omitted, `conversationId` must be provided.
|
||||||
pub path: PathBuf,
|
#[serde(skip_serializing_if = "Option::is_none")]
|
||||||
|
pub path: Option<PathBuf>,
|
||||||
|
/// If the rollout path is not known, it can be discovered via the conversation id at at the cost of extra latency.
|
||||||
|
#[serde(skip_serializing_if = "Option::is_none")]
|
||||||
|
pub conversation_id: Option<ConversationId>,
|
||||||
/// Optional overrides to apply when spawning the resumed session.
|
/// Optional overrides to apply when spawning the resumed session.
|
||||||
#[serde(skip_serializing_if = "Option::is_none")]
|
#[serde(skip_serializing_if = "Option::is_none")]
|
||||||
pub overrides: Option<NewConversationParams>,
|
pub overrides: Option<NewConversationParams>,
|
||||||
|
|||||||
@@ -834,12 +834,37 @@ impl CodexMessageProcessor {
|
|||||||
request_id: RequestId,
|
request_id: RequestId,
|
||||||
params: GetConversationSummaryParams,
|
params: GetConversationSummaryParams,
|
||||||
) {
|
) {
|
||||||
let GetConversationSummaryParams { rollout_path } = params;
|
let path = match params {
|
||||||
let path = if rollout_path.is_relative() {
|
GetConversationSummaryParams::RolloutPath { rollout_path } => {
|
||||||
self.config.codex_home.join(&rollout_path)
|
if rollout_path.is_relative() {
|
||||||
} else {
|
self.config.codex_home.join(&rollout_path)
|
||||||
rollout_path.clone()
|
} else {
|
||||||
|
rollout_path
|
||||||
|
}
|
||||||
|
}
|
||||||
|
GetConversationSummaryParams::ConversationId { conversation_id } => {
|
||||||
|
match codex_core::find_conversation_path_by_id_str(
|
||||||
|
&self.config.codex_home,
|
||||||
|
&conversation_id.to_string(),
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
{
|
||||||
|
Ok(Some(p)) => p,
|
||||||
|
_ => {
|
||||||
|
let error = JSONRPCErrorError {
|
||||||
|
code: INVALID_REQUEST_ERROR_CODE,
|
||||||
|
message: format!(
|
||||||
|
"no rollout found for conversation id {conversation_id}"
|
||||||
|
),
|
||||||
|
data: None,
|
||||||
|
};
|
||||||
|
self.outgoing.send_error(request_id, error).await;
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
let fallback_provider = self.config.model_provider_id.as_str();
|
let fallback_provider = self.config.model_provider_id.as_str();
|
||||||
|
|
||||||
match read_summary_from_rollout(&path, fallback_provider).await {
|
match read_summary_from_rollout(&path, fallback_provider).await {
|
||||||
@@ -990,6 +1015,43 @@ impl CodexMessageProcessor {
|
|||||||
request_id: RequestId,
|
request_id: RequestId,
|
||||||
params: ResumeConversationParams,
|
params: ResumeConversationParams,
|
||||||
) {
|
) {
|
||||||
|
let path = match params {
|
||||||
|
ResumeConversationParams {
|
||||||
|
path: Some(path), ..
|
||||||
|
} => path,
|
||||||
|
ResumeConversationParams {
|
||||||
|
conversation_id: Some(conversation_id),
|
||||||
|
..
|
||||||
|
} => {
|
||||||
|
match codex_core::find_conversation_path_by_id_str(
|
||||||
|
&self.config.codex_home,
|
||||||
|
&conversation_id.to_string(),
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
{
|
||||||
|
Ok(Some(p)) => p,
|
||||||
|
_ => {
|
||||||
|
let error = JSONRPCErrorError {
|
||||||
|
code: INVALID_REQUEST_ERROR_CODE,
|
||||||
|
message: "unable to locate rollout path".to_string(),
|
||||||
|
data: None,
|
||||||
|
};
|
||||||
|
self.outgoing.send_error(request_id, error).await;
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
_ => {
|
||||||
|
let error = JSONRPCErrorError {
|
||||||
|
code: INVALID_REQUEST_ERROR_CODE,
|
||||||
|
message: "either path or conversation id must be provided".to_string(),
|
||||||
|
data: None,
|
||||||
|
};
|
||||||
|
self.outgoing.send_error(request_id, error).await;
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
// Derive a Config using the same logic as new conversation, honoring overrides if provided.
|
// Derive a Config using the same logic as new conversation, honoring overrides if provided.
|
||||||
let config = match params.overrides {
|
let config = match params.overrides {
|
||||||
Some(overrides) => {
|
Some(overrides) => {
|
||||||
@@ -1012,11 +1074,7 @@ impl CodexMessageProcessor {
|
|||||||
|
|
||||||
match self
|
match self
|
||||||
.conversation_manager
|
.conversation_manager
|
||||||
.resume_conversation_from_rollout(
|
.resume_conversation_from_rollout(config, path.clone(), self.auth_manager.clone())
|
||||||
config,
|
|
||||||
params.path.clone(),
|
|
||||||
self.auth_manager.clone(),
|
|
||||||
)
|
|
||||||
.await
|
.await
|
||||||
{
|
{
|
||||||
Ok(NewConversation {
|
Ok(NewConversation {
|
||||||
@@ -1046,6 +1104,7 @@ impl CodexMessageProcessor {
|
|||||||
conversation_id,
|
conversation_id,
|
||||||
model: session_configured.model.clone(),
|
model: session_configured.model.clone(),
|
||||||
initial_messages,
|
initial_messages,
|
||||||
|
rollout_path: session_configured.rollout_path.clone(),
|
||||||
};
|
};
|
||||||
self.outgoing.send_response(request_id, response).await;
|
self.outgoing.send_response(request_id, response).await;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -171,7 +171,8 @@ async fn test_list_and_resume_conversations() -> Result<()> {
|
|||||||
// Now resume one of the sessions and expect a SessionConfigured notification and response.
|
// Now resume one of the sessions and expect a SessionConfigured notification and response.
|
||||||
let resume_req_id = mcp
|
let resume_req_id = mcp
|
||||||
.send_resume_conversation_request(ResumeConversationParams {
|
.send_resume_conversation_request(ResumeConversationParams {
|
||||||
path: items[0].path.clone(),
|
path: Some(items[0].path.clone()),
|
||||||
|
conversation_id: None,
|
||||||
overrides: Some(NewConversationParams {
|
overrides: Some(NewConversationParams {
|
||||||
model: Some("o3".to_string()),
|
model: Some("o3".to_string()),
|
||||||
..Default::default()
|
..Default::default()
|
||||||
|
|||||||
@@ -40,6 +40,7 @@ pub struct FileMatch {
|
|||||||
pub indices: Option<Vec<u32>>, // Sorted & deduplicated when present
|
pub indices: Option<Vec<u32>>, // Sorted & deduplicated when present
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[derive(Debug)]
|
||||||
pub struct FileSearchResults {
|
pub struct FileSearchResults {
|
||||||
pub matches: Vec<FileMatch>,
|
pub matches: Vec<FileMatch>,
|
||||||
pub total_match_count: usize,
|
pub total_match_count: usize,
|
||||||
|
|||||||
Reference in New Issue
Block a user