Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 21 additions & 5 deletions codex-rs/app-server-protocol/src/protocol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -340,12 +340,24 @@ pub struct ResumeConversationResponse {
pub model: String,
#[serde(skip_serializing_if = "Option::is_none")]
pub initial_messages: Option<Vec<EventMsg>>,
pub rollout_path: PathBuf,
}

#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
#[serde(rename_all = "camelCase")]
pub struct GetConversationSummaryParams {
pub rollout_path: PathBuf,
#[serde(untagged)]
pub enum GetConversationSummaryParams {
/// Provide the absolute or CODEX_HOME‑relative rollout path directly.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For a relative path, we should require that it is "normalized," i.e., no funny stuff with ../ to read paths outside $CODEX_HOME/sessions.

Come to think of it, we should probably resolve the path to a canonical (or at least absolute without hitting disk: https://crates.io/crates/path-absolutize) and then verify it is under $CODEX_HOME/sessions, which is simpler.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This just documents the existing behvaior, do you want me to update that as part of this PR?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gpeal It can be done in a separate PR, but I think it's important that it's done.

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)]
Expand Down Expand Up @@ -487,8 +499,12 @@ pub struct LogoutAccountResponse {}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
#[serde(rename_all = "camelCase")]
pub struct ResumeConversationParams {
/// Absolute path to the rollout JSONL file.
pub path: PathBuf,
/// Absolute path to the rollout JSONL file. If omitted, `conversationId` must be provided.
#[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.
#[serde(skip_serializing_if = "Option::is_none")]
pub overrides: Option<NewConversationParams>,
Expand Down
79 changes: 69 additions & 10 deletions codex-rs/app-server/src/codex_message_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -834,12 +834,37 @@ impl CodexMessageProcessor {
request_id: RequestId,
params: GetConversationSummaryParams,
) {
let GetConversationSummaryParams { rollout_path } = params;
let path = if rollout_path.is_relative() {
self.config.codex_home.join(&rollout_path)
} else {
rollout_path.clone()
let path = match params {
GetConversationSummaryParams::RolloutPath { rollout_path } => {
if rollout_path.is_relative() {
self.config.codex_home.join(&rollout_path)
} 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();

match read_summary_from_rollout(&path, fallback_provider).await {
Expand Down Expand Up @@ -990,6 +1015,43 @@ impl CodexMessageProcessor {
request_id: RequestId,
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.
let config = match params.overrides {
Some(overrides) => {
Expand All @@ -1012,11 +1074,7 @@ impl CodexMessageProcessor {

match self
.conversation_manager
.resume_conversation_from_rollout(
config,
params.path.clone(),
self.auth_manager.clone(),
)
.resume_conversation_from_rollout(config, path.clone(), self.auth_manager.clone())
.await
{
Ok(NewConversation {
Expand Down Expand Up @@ -1046,6 +1104,7 @@ impl CodexMessageProcessor {
conversation_id,
model: session_configured.model.clone(),
initial_messages,
rollout_path: session_configured.rollout_path.clone(),
};
self.outgoing.send_response(request_id, response).await;
}
Expand Down
3 changes: 2 additions & 1 deletion codex-rs/app-server/tests/suite/list_resume.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
let resume_req_id = mcp
.send_resume_conversation_request(ResumeConversationParams {
path: items[0].path.clone(),
path: Some(items[0].path.clone()),
conversation_id: None,
overrides: Some(NewConversationParams {
model: Some("o3".to_string()),
..Default::default()
Expand Down
1 change: 1 addition & 0 deletions codex-rs/file-search/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ pub struct FileMatch {
pub indices: Option<Vec<u32>>, // Sorted & deduplicated when present
}

#[derive(Debug)]
pub struct FileSearchResults {
pub matches: Vec<FileMatch>,
pub total_match_count: usize,
Expand Down
Loading