Skip to content

Commit 14a1796

Browse files
committed
Cleanup
1 parent 4bdf21d commit 14a1796

File tree

7 files changed

+114
-95
lines changed

7 files changed

+114
-95
lines changed

codex-rs/core/src/client.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@ impl ModelClient {
233233
&& auth.mode == AuthMode::ChatGPT
234234
&& let Some(account_id) = auth.get_account_id()
235235
{
236-
req_builder = req_builder.header("chatgpt-account-id", &account_id);
236+
req_builder = req_builder.header("chatgpt-account-id", account_id);
237237
}
238238

239239
let res = req_builder.send().await;

codex-rs/core/src/codex.rs

Lines changed: 34 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,6 @@ use crate::protocol::ExecCommandBeginEvent;
8989
use crate::protocol::ExecCommandEndEvent;
9090
use crate::protocol::FileChange;
9191
use crate::protocol::InputItem;
92-
use crate::protocol::InputMessageKind;
9392
use crate::protocol::ListCustomPromptsResponseEvent;
9493
use crate::protocol::Op;
9594
use crate::protocol::PatchApplyBeginEvent;
@@ -169,9 +168,7 @@ impl Codex {
169168
pub async fn spawn(
170169
config: Config,
171170
auth_manager: Arc<AuthManager>,
172-
conversation_id: Option<ConversationId>,
173171
conversation_history: InitialHistory,
174-
rollout_path: Option<PathBuf>,
175172
) -> CodexResult<CodexSpawnOk> {
176173
let (tx_sub, rx_sub) = async_channel::bounded(SUBMISSION_CHANNEL_CAPACITY);
177174
let (tx_event, rx_event) = async_channel::unbounded();
@@ -199,9 +196,7 @@ impl Codex {
199196
config.clone(),
200197
auth_manager.clone(),
201198
tx_event.clone(),
202-
conversation_id,
203199
conversation_history.clone(),
204-
rollout_path,
205200
)
206201
.await
207202
.map_err(|e| {
@@ -365,11 +360,8 @@ impl Session {
365360
config: Arc<Config>,
366361
auth_manager: Arc<AuthManager>,
367362
tx_event: Sender<Event>,
368-
conversation_id: Option<ConversationId>,
369363
initial_history: InitialHistory,
370-
rollout_path: Option<PathBuf>,
371364
) -> anyhow::Result<(Arc<Self>, TurnContext)> {
372-
let conversation_id = conversation_id.unwrap_or_else(ConversationId::new);
373365
let ConfigureSession {
374366
provider,
375367
model,
@@ -387,6 +379,20 @@ impl Session {
387379
return Err(anyhow::anyhow!("cwd is not absolute: {cwd:?}"));
388380
}
389381

382+
let (conversation_id, rollout_params) = match &initial_history {
383+
InitialHistory::New | InitialHistory::Forked(_) => {
384+
let conversation_id = ConversationId::default();
385+
(
386+
conversation_id,
387+
RolloutRecorderParams::new(conversation_id, user_instructions.clone()),
388+
)
389+
}
390+
InitialHistory::Resumed(resumed_history) => (
391+
resumed_history.conversation_id,
392+
RolloutRecorderParams::resume(resumed_history.rollout_path.clone()),
393+
),
394+
};
395+
390396
// Error messages to dispatch after SessionConfigured is sent.
391397
let mut post_session_configured_error_events = Vec::<Event>::new();
392398

@@ -396,10 +402,6 @@ impl Session {
396402
// - spin up MCP connection manager
397403
// - perform default shell discovery
398404
// - load history metadata
399-
let rollout_params = match rollout_path {
400-
Some(path) => RolloutRecorderParams::resume(path),
401-
None => RolloutRecorderParams::new(conversation_id, user_instructions.clone()),
402-
};
403405
let rollout_fut = RolloutRecorder::new(&config, rollout_params);
404406

405407
let mcp_fut = McpConnectionManager::new(config.mcp_servers.clone());
@@ -492,7 +494,10 @@ impl Session {
492494
// If resuming, include converted initial messages in the payload so UIs can render them immediately.
493495
let initial_messages = match &initial_history {
494496
InitialHistory::New => None,
495-
InitialHistory::Resumed(items) => Some(sess.build_initial_messages(items)),
497+
InitialHistory::Forked(items) => Some(sess.build_initial_messages(items)),
498+
InitialHistory::Resumed(resumed_history) => {
499+
Some(sess.build_initial_messages(&resumed_history.history))
500+
}
496501
};
497502

498503
let events = std::iter::once(Event {
@@ -541,8 +546,12 @@ impl Session {
541546
InitialHistory::New => {
542547
self.record_initial_history_new(turn_context).await;
543548
}
544-
InitialHistory::Resumed(items) => {
545-
self.record_initial_history_resumed(items).await;
549+
InitialHistory::Forked(items) => {
550+
self.record_initial_history_from_items(items).await;
551+
}
552+
InitialHistory::Resumed(resumed_history) => {
553+
self.record_initial_history_from_items(resumed_history.history)
554+
.await;
546555
}
547556
}
548557
}
@@ -564,8 +573,8 @@ impl Session {
564573
self.record_conversation_items(&conversation_items).await;
565574
}
566575

567-
async fn record_initial_history_resumed(&self, items: Vec<ResponseItem>) {
568-
self.record_conversation_items(&items).await;
576+
async fn record_initial_history_from_items(&self, items: Vec<ResponseItem>) {
577+
self.record_conversation_items_internal(&items, false).await;
569578
}
570579

571580
/// build the initial messages vector for SessionConfigured by converting
@@ -576,12 +585,6 @@ impl Session {
576585
.flat_map(|item| {
577586
map_response_item_to_event_messages(item, self.show_raw_agent_reasoning)
578587
})
579-
.filter(|event| {
580-
if let EventMsg::UserMessage(user_message) = event {
581-
return matches!(user_message.kind, Some(InputMessageKind::Plain));
582-
}
583-
true
584-
})
585588
.collect()
586589
}
587590

@@ -680,8 +683,14 @@ impl Session {
680683
/// Records items to both the rollout and the chat completions/ZDR
681684
/// transcript, if enabled.
682685
async fn record_conversation_items(&self, items: &[ResponseItem]) {
686+
self.record_conversation_items_internal(items, true).await;
687+
}
688+
689+
async fn record_conversation_items_internal(&self, items: &[ResponseItem], persist: bool) {
683690
debug!("Recording items for conversation: {items:?}");
684-
self.record_state_snapshot(items).await;
691+
if persist {
692+
self.record_state_snapshot(items).await;
693+
}
685694

686695
self.state.lock_unchecked().history.record_items(items);
687696
}
@@ -1267,7 +1276,7 @@ async fn submission_loop(
12671276
log_id,
12681277
entry: entry_opt.map(|e| {
12691278
codex_protocol::message_history::HistoryEntry {
1270-
conversation_id: e.conversation_id,
1279+
conversation_id: e.session_id,
12711280
ts: e.ts,
12721281
text: e.text,
12731282
}

codex-rs/core/src/conversation_manager.rs

Lines changed: 18 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,18 @@ use std::path::PathBuf;
1818
use std::sync::Arc;
1919
use tokio::sync::RwLock;
2020

21+
#[derive(Debug, Clone, PartialEq)]
22+
pub struct ResumedHistory {
23+
pub conversation_id: ConversationId,
24+
pub history: Vec<ResponseItem>,
25+
pub rollout_path: PathBuf,
26+
}
27+
2128
#[derive(Debug, Clone, PartialEq)]
2229
pub enum InitialHistory {
2330
New,
24-
Resumed(Vec<ResponseItem>),
31+
Resumed(ResumedHistory),
32+
Forked(Vec<ResponseItem>),
2533
}
2634

2735
/// Represents a newly created Codex conversation, including the first event
@@ -65,25 +73,17 @@ impl ConversationManager {
6573
) -> CodexResult<NewConversation> {
6674
// TO BE REFACTORED: use the config experimental_resume field until we have a mainstream way.
6775
if let Some(resume_path) = config.experimental_resume.clone() {
68-
let (conversation_id, initial_history) =
69-
RolloutRecorder::get_rollout_history(&resume_path).await?;
76+
let initial_history = RolloutRecorder::get_rollout_history(&resume_path).await?;
7077
let CodexSpawnOk {
7178
codex,
7279
conversation_id,
73-
} = Codex::spawn(
74-
config,
75-
auth_manager,
76-
conversation_id,
77-
initial_history,
78-
Some(resume_path.clone()),
79-
)
80-
.await?;
80+
} = Codex::spawn(config, auth_manager, initial_history).await?;
8181
self.finalize_spawn(codex, conversation_id).await
8282
} else {
8383
let CodexSpawnOk {
8484
codex,
8585
conversation_id,
86-
} = Codex::spawn(config, auth_manager, None, InitialHistory::New, None).await?;
86+
} = Codex::spawn(config, auth_manager, InitialHistory::New).await?;
8787
self.finalize_spawn(codex, conversation_id).await
8888
}
8989
}
@@ -137,19 +137,11 @@ impl ConversationManager {
137137
rollout_path: PathBuf,
138138
auth_manager: Arc<AuthManager>,
139139
) -> CodexResult<NewConversation> {
140-
let (conversation_id, initial_history) =
141-
RolloutRecorder::get_rollout_history(&rollout_path).await?;
140+
let initial_history = RolloutRecorder::get_rollout_history(&rollout_path).await?;
142141
let CodexSpawnOk {
143142
codex,
144143
conversation_id,
145-
} = Codex::spawn(
146-
config,
147-
auth_manager,
148-
conversation_id,
149-
initial_history,
150-
Some(rollout_path),
151-
)
152-
.await?;
144+
} = Codex::spawn(config, auth_manager, initial_history).await?;
153145
self.finalize_spawn(codex, conversation_id).await
154146
}
155147

@@ -176,7 +168,7 @@ impl ConversationManager {
176168
let CodexSpawnOk {
177169
codex,
178170
conversation_id,
179-
} = Codex::spawn(config, auth_manager, None, history, None).await?;
171+
} = Codex::spawn(config, auth_manager, history).await?;
180172

181173
self.finalize_spawn(codex, conversation_id).await
182174
}
@@ -186,7 +178,7 @@ impl ConversationManager {
186178
/// and all items that follow them.
187179
fn truncate_after_dropping_last_messages(items: Vec<ResponseItem>, n: usize) -> InitialHistory {
188180
if n == 0 {
189-
return InitialHistory::Resumed(items);
181+
return InitialHistory::Forked(items);
190182
}
191183

192184
// Walk backwards counting only `user` Message items, find cut index.
@@ -208,7 +200,7 @@ fn truncate_after_dropping_last_messages(items: Vec<ResponseItem>, n: usize) ->
208200
// No prefix remains after dropping; start a new conversation.
209201
InitialHistory::New
210202
} else {
211-
InitialHistory::Resumed(items.into_iter().take(cut_index).collect())
203+
InitialHistory::Forked(items.into_iter().take(cut_index).collect())
212204
}
213205
}
214206

@@ -266,7 +258,7 @@ mod tests {
266258
let truncated = truncate_after_dropping_last_messages(items.clone(), 1);
267259
assert_eq!(
268260
truncated,
269-
InitialHistory::Resumed(vec![items[0].clone(), items[1].clone(), items[2].clone(),])
261+
InitialHistory::Forked(vec![items[0].clone(), items[1].clone(), items[2].clone(),])
270262
);
271263

272264
let truncated2 = truncate_after_dropping_last_messages(items, 2);

codex-rs/core/src/message_history.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ const RETRY_SLEEP: Duration = Duration::from_millis(100);
4444

4545
#[derive(Serialize, Deserialize, Debug, Clone)]
4646
pub struct HistoryEntry {
47-
pub conversation_id: String,
47+
pub session_id: String,
4848
pub ts: u64,
4949
pub text: String,
5050
}
@@ -89,7 +89,7 @@ pub(crate) async fn append_entry(
8989

9090
// Construct the JSON line first so we can write it in a single syscall.
9191
let entry = HistoryEntry {
92-
conversation_id: conversation_id.to_string(),
92+
session_id: conversation_id.to_string(),
9393
ts,
9494
text: text.to_string(),
9595
};

0 commit comments

Comments
 (0)