revert /name for now (#4978)

There was a regression where we'd read entire rollout contents if there
was no /name present.
This commit is contained in:
dedrisian-oai
2025-10-08 17:13:49 -07:00
committed by GitHub
parent ec238a2c39
commit 4300236681
13 changed files with 22 additions and 167 deletions

View File

@@ -88,7 +88,6 @@ use crate::protocol::ReviewDecision;
use crate::protocol::ReviewOutputEvent;
use crate::protocol::SandboxPolicy;
use crate::protocol::SessionConfiguredEvent;
use crate::protocol::SessionRenamedEvent;
use crate::protocol::StreamErrorEvent;
use crate::protocol::Submission;
use crate::protocol::TokenCountEvent;
@@ -1508,16 +1507,6 @@ async fn submission_loop(
};
sess.send_event(event).await;
}
Op::SetSessionName { name } => {
// Persist a rename event and notify the client. We rely on the
// recorder's filtering to include this in the rollout.
let sub_id = sub.id.clone();
let event = Event {
id: sub_id,
msg: EventMsg::SessionRenamed(SessionRenamedEvent { name }),
};
sess.send_event(event).await;
}
Op::Review { review_request } => {
spawn_review_thread(
sess.clone(),

View File

@@ -17,7 +17,6 @@ use super::SESSIONS_SUBDIR;
use crate::protocol::EventMsg;
use codex_protocol::protocol::RolloutItem;
use codex_protocol::protocol::RolloutLine;
use codex_protocol::protocol::SessionRenamedEvent;
use codex_protocol::protocol::SessionSource;
/// Returned page of conversation summaries.
@@ -42,8 +41,6 @@ pub struct ConversationItem {
pub head: Vec<serde_json::Value>,
/// Last up to `TAIL_RECORD_LIMIT` JSONL response records parsed as JSON.
pub tail: Vec<serde_json::Value>,
/// Latest human-friendly session name, if any.
pub name: Option<String>,
/// RFC3339 timestamp string for when the session was created, if available.
pub created_at: Option<String>,
/// RFC3339 timestamp string for the most recent response in the tail, if available.
@@ -59,7 +56,6 @@ struct HeadTailSummary {
source: Option<SessionSource>,
created_at: Option<String>,
updated_at: Option<String>,
name: Option<String>,
}
/// Hard cap to bound worstcase work per request.
@@ -226,7 +222,6 @@ async fn traverse_directories_for_paths(
path,
head,
tail,
name: summary.name,
created_at,
updated_at,
});
@@ -387,21 +382,14 @@ async fn read_head_and_tail(
if matches!(ev, EventMsg::UserMessage(_)) {
summary.saw_user_event = true;
}
if let EventMsg::SessionRenamed(SessionRenamedEvent { name }) = ev {
summary.name = Some(name);
}
}
}
}
if tail_limit != 0 {
let (tail, updated_at, latest_name) = read_tail_records(path, tail_limit).await?;
let (tail, updated_at) = read_tail_records(path, tail_limit).await?;
summary.tail = tail;
summary.updated_at = updated_at;
// Prefer the most recent rename event discovered from the tail scan; fallback to any name seen in head.
if latest_name.is_some() {
summary.name = latest_name;
}
}
Ok(summary)
}
@@ -409,13 +397,13 @@ async fn read_head_and_tail(
async fn read_tail_records(
path: &Path,
max_records: usize,
) -> io::Result<(Vec<serde_json::Value>, Option<String>, Option<String>)> {
) -> io::Result<(Vec<serde_json::Value>, Option<String>)> {
use std::io::SeekFrom;
use tokio::io::AsyncReadExt;
use tokio::io::AsyncSeekExt;
if max_records == 0 {
return Ok((Vec::new(), None, None));
return Ok((Vec::new(), None));
}
const CHUNK_SIZE: usize = 8192;
@@ -423,33 +411,28 @@ async fn read_tail_records(
let mut file = tokio::fs::File::open(path).await?;
let mut pos = file.seek(SeekFrom::End(0)).await?;
if pos == 0 {
return Ok((Vec::new(), None, None));
return Ok((Vec::new(), None));
}
let mut buffer: Vec<u8> = Vec::new();
let mut latest_timestamp: Option<String> = None;
let mut latest_name: Option<String> = None;
loop {
let slice_start = match (pos > 0, buffer.iter().position(|&b| b == b'\n')) {
(true, Some(idx)) => idx + 1,
_ => 0,
};
let (tail, newest_ts, name_opt) =
collect_last_response_values(&buffer[slice_start..], max_records);
let (tail, newest_ts) = collect_last_response_values(&buffer[slice_start..], max_records);
if latest_timestamp.is_none() {
latest_timestamp = newest_ts.clone();
}
if latest_name.is_none() {
latest_name = name_opt.clone();
}
if tail.len() >= max_records || pos == 0 {
return Ok((tail, latest_timestamp.or(newest_ts), latest_name));
return Ok((tail, latest_timestamp.or(newest_ts)));
}
let read_size = CHUNK_SIZE.min(pos as usize);
if read_size == 0 {
return Ok((tail, latest_timestamp.or(newest_ts), latest_name));
return Ok((tail, latest_timestamp.or(newest_ts)));
}
pos -= read_size as u64;
file.seek(SeekFrom::Start(pos)).await?;
@@ -463,17 +446,16 @@ async fn read_tail_records(
fn collect_last_response_values(
buffer: &[u8],
max_records: usize,
) -> (Vec<serde_json::Value>, Option<String>, Option<String>) {
) -> (Vec<serde_json::Value>, Option<String>) {
use std::borrow::Cow;
if buffer.is_empty() || max_records == 0 {
return (Vec::new(), None, None);
return (Vec::new(), None);
}
let text: Cow<'_, str> = String::from_utf8_lossy(buffer);
let mut collected_rev: Vec<serde_json::Value> = Vec::new();
let mut latest_timestamp: Option<String> = None;
let mut latest_name: Option<String> = None;
for line in text.lines().rev() {
let trimmed = line.trim();
if trimmed.is_empty() {
@@ -482,30 +464,20 @@ fn collect_last_response_values(
let parsed: serde_json::Result<RolloutLine> = serde_json::from_str(trimmed);
let Ok(rollout_line) = parsed else { continue };
let RolloutLine { timestamp, item } = rollout_line;
match item {
RolloutItem::ResponseItem(item) => {
if let Ok(val) = serde_json::to_value(&item) {
if latest_timestamp.is_none() {
latest_timestamp = Some(timestamp.clone());
}
collected_rev.push(val);
if collected_rev.len() == max_records {
break;
}
}
if let RolloutItem::ResponseItem(item) = item
&& let Ok(val) = serde_json::to_value(&item)
{
if latest_timestamp.is_none() {
latest_timestamp = Some(timestamp.clone());
}
RolloutItem::EventMsg(ev) => {
if latest_name.is_none()
&& let EventMsg::SessionRenamed(SessionRenamedEvent { name }) = ev
{
latest_name = Some(name);
}
collected_rev.push(val);
if collected_rev.len() == max_records {
break;
}
_ => {}
}
}
collected_rev.reverse();
(collected_rev, latest_timestamp, latest_name)
(collected_rev, latest_timestamp)
}
/// Locate a recorded conversation rollout file by its UUID string using the existing

View File

@@ -39,7 +39,6 @@ pub(crate) fn should_persist_event_msg(ev: &EventMsg) -> bool {
| EventMsg::AgentMessage(_)
| EventMsg::AgentReasoning(_)
| EventMsg::AgentReasoningRawContent(_)
| EventMsg::SessionRenamed(_)
| EventMsg::TokenCount(_)
| EventMsg::EnteredReviewMode(_)
| EventMsg::ExitedReviewMode(_)

View File

@@ -196,7 +196,6 @@ async fn test_list_conversations_latest_first() {
path: p1,
head: head_3,
tail: Vec::new(),
name: None,
created_at: Some("2025-01-03T12-00-00".into()),
updated_at: Some("2025-01-03T12-00-00".into()),
},
@@ -204,7 +203,6 @@ async fn test_list_conversations_latest_first() {
path: p2,
head: head_2,
tail: Vec::new(),
name: None,
created_at: Some("2025-01-02T12-00-00".into()),
updated_at: Some("2025-01-02T12-00-00".into()),
},
@@ -212,7 +210,6 @@ async fn test_list_conversations_latest_first() {
path: p3,
head: head_1,
tail: Vec::new(),
name: None,
created_at: Some("2025-01-01T12-00-00".into()),
updated_at: Some("2025-01-01T12-00-00".into()),
},
@@ -320,7 +317,6 @@ async fn test_pagination_cursor() {
path: p5,
head: head_5,
tail: Vec::new(),
name: None,
created_at: Some("2025-03-05T09-00-00".into()),
updated_at: Some("2025-03-05T09-00-00".into()),
},
@@ -328,7 +324,6 @@ async fn test_pagination_cursor() {
path: p4,
head: head_4,
tail: Vec::new(),
name: None,
created_at: Some("2025-03-04T09-00-00".into()),
updated_at: Some("2025-03-04T09-00-00".into()),
},
@@ -385,7 +380,6 @@ async fn test_pagination_cursor() {
path: p3,
head: head_3,
tail: Vec::new(),
name: None,
created_at: Some("2025-03-03T09-00-00".into()),
updated_at: Some("2025-03-03T09-00-00".into()),
},
@@ -393,7 +387,6 @@ async fn test_pagination_cursor() {
path: p2,
head: head_2,
tail: Vec::new(),
name: None,
created_at: Some("2025-03-02T09-00-00".into()),
updated_at: Some("2025-03-02T09-00-00".into()),
},
@@ -434,7 +427,6 @@ async fn test_pagination_cursor() {
path: p1,
head: head_1,
tail: Vec::new(),
name: None,
created_at: Some("2025-03-01T09-00-00".into()),
updated_at: Some("2025-03-01T09-00-00".into()),
}],
@@ -483,7 +475,6 @@ async fn test_get_conversation_contents() {
path: expected_path,
head: expected_head,
tail: Vec::new(),
name: None,
created_at: Some(ts.into()),
updated_at: Some(ts.into()),
}],
@@ -832,7 +823,6 @@ async fn test_stable_ordering_same_second_pagination() {
path: p3,
head: head(u3),
tail: Vec::new(),
name: None,
created_at: Some(ts.to_string()),
updated_at: Some(ts.to_string()),
},
@@ -840,7 +830,6 @@ async fn test_stable_ordering_same_second_pagination() {
path: p2,
head: head(u2),
tail: Vec::new(),
name: None,
created_at: Some(ts.to_string()),
updated_at: Some(ts.to_string()),
},
@@ -871,7 +860,6 @@ async fn test_stable_ordering_same_second_pagination() {
path: p1,
head: head(u1),
tail: Vec::new(),
name: None,
created_at: Some(ts.to_string()),
updated_at: Some(ts.to_string()),
}],