mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Remove ghost snapshots (#19481)
## Summary - Remove `ghost_snapshot` / `GhostCommit` from the Responses API surface and generated SDK/schema artifacts. - Keep legacy config loading compatible, but make undo a no-op that reports the feature is unavailable. - Clean up core history, compaction, telemetry, rollout, and tests to stop carrying ghost snapshot items. ## Testing - Unit tests passed for `codex-protocol`, `codex-core` targeted undo and compaction flows, `codex-rollout`, and `codex-app-server-protocol`. - Regenerated config and app-server schemas plus Python SDK artifacts and verified they match the checked-in outputs.
This commit is contained in:
@@ -37,7 +37,6 @@ pub fn should_persist_response_item(item: &ResponseItem) -> bool {
|
||||
| ResponseItem::CustomToolCallOutput { .. }
|
||||
| ResponseItem::WebSearchCall { .. }
|
||||
| ResponseItem::ImageGenerationCall { .. }
|
||||
| ResponseItem::GhostSnapshot { .. }
|
||||
| ResponseItem::Compaction { .. } => true,
|
||||
ResponseItem::Other => false,
|
||||
}
|
||||
@@ -58,7 +57,6 @@ pub fn should_persist_response_item_for_memories(item: &ResponseItem) -> bool {
|
||||
| ResponseItem::WebSearchCall { .. } => true,
|
||||
ResponseItem::Reasoning { .. }
|
||||
| ResponseItem::ImageGenerationCall { .. }
|
||||
| ResponseItem::GhostSnapshot { .. }
|
||||
| ResponseItem::Compaction { .. }
|
||||
| ResponseItem::Other => false,
|
||||
}
|
||||
|
||||
@@ -867,7 +867,7 @@ impl RolloutRecorder {
|
||||
if line.trim().is_empty() {
|
||||
continue;
|
||||
}
|
||||
let v: Value = match serde_json::from_str(line) {
|
||||
let mut v: Value = match serde_json::from_str(line) {
|
||||
Ok(v) => v,
|
||||
Err(e) => {
|
||||
warn!("failed to parse line as JSON: {line:?}, error: {e}");
|
||||
@@ -875,6 +875,10 @@ impl RolloutRecorder {
|
||||
continue;
|
||||
}
|
||||
};
|
||||
if strip_legacy_ghost_snapshot_rollout_line(&mut v) {
|
||||
trace!("skipping legacy ghost_snapshot rollout line");
|
||||
continue;
|
||||
}
|
||||
|
||||
// Parse the rollout line structure
|
||||
match serde_json::from_value::<RolloutLine>(v.clone()) {
|
||||
@@ -961,6 +965,29 @@ impl RolloutRecorder {
|
||||
}
|
||||
}
|
||||
|
||||
fn strip_legacy_ghost_snapshot_rollout_line(value: &mut Value) -> bool {
|
||||
match value.get("type").and_then(Value::as_str) {
|
||||
Some("response_item") => value
|
||||
.get("payload")
|
||||
.is_some_and(is_legacy_ghost_snapshot_response_item),
|
||||
Some("compacted") => {
|
||||
if let Some(replacement_history) = value
|
||||
.get_mut("payload")
|
||||
.and_then(|payload| payload.get_mut("replacement_history"))
|
||||
.and_then(Value::as_array_mut)
|
||||
{
|
||||
replacement_history.retain(|item| !is_legacy_ghost_snapshot_response_item(item));
|
||||
}
|
||||
false
|
||||
}
|
||||
_ => false,
|
||||
}
|
||||
}
|
||||
|
||||
fn is_legacy_ghost_snapshot_response_item(value: &Value) -> bool {
|
||||
value.get("type").and_then(Value::as_str) == Some("ghost_snapshot")
|
||||
}
|
||||
|
||||
fn truncate_fs_page(
|
||||
mut page: ThreadsPage,
|
||||
page_size: usize,
|
||||
|
||||
@@ -4,9 +4,12 @@ use super::*;
|
||||
use crate::config::RolloutConfig;
|
||||
use chrono::TimeZone;
|
||||
use codex_protocol::config_types::ReasoningSummary as ReasoningSummaryConfig;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use codex_protocol::protocol::AgentMessageEvent;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::RolloutItem;
|
||||
use codex_protocol::protocol::RolloutLine;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use codex_protocol::protocol::TurnContextItem;
|
||||
use codex_protocol::protocol::UserMessageEvent;
|
||||
@@ -62,6 +65,161 @@ fn write_session_file(root: &Path, ts: &str, uuid: Uuid) -> std::io::Result<Path
|
||||
Ok(path)
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn load_rollout_items_skips_legacy_ghost_snapshot_lines() -> std::io::Result<()> {
|
||||
let home = TempDir::new().expect("temp dir");
|
||||
let rollout_path = home.path().join("rollout.jsonl");
|
||||
let mut file = File::create(&rollout_path)?;
|
||||
let thread_id = ThreadId::new();
|
||||
let ts = "2025-01-03T12:00:00Z";
|
||||
|
||||
writeln!(
|
||||
file,
|
||||
"{}",
|
||||
serde_json::json!({
|
||||
"timestamp": ts,
|
||||
"type": "session_meta",
|
||||
"payload": {
|
||||
"id": thread_id,
|
||||
"timestamp": ts,
|
||||
"cwd": ".",
|
||||
"originator": "test_originator",
|
||||
"cli_version": "test_version",
|
||||
"source": "cli",
|
||||
"model_provider": "test-provider",
|
||||
},
|
||||
})
|
||||
)?;
|
||||
writeln!(
|
||||
file,
|
||||
"{}",
|
||||
serde_json::json!({
|
||||
"timestamp": ts,
|
||||
"type": "response_item",
|
||||
"payload": {
|
||||
"type": "ghost_snapshot",
|
||||
"ghost_commit": {
|
||||
"id": "deadbeef",
|
||||
"preexisting_untracked_dirs": [],
|
||||
"preexisting_untracked_files": [],
|
||||
},
|
||||
},
|
||||
})
|
||||
)?;
|
||||
writeln!(
|
||||
file,
|
||||
"{}",
|
||||
serde_json::json!({
|
||||
"timestamp": ts,
|
||||
"type": "response_item",
|
||||
"payload": {
|
||||
"type": "message",
|
||||
"role": "assistant",
|
||||
"content": [
|
||||
{
|
||||
"type": "output_text",
|
||||
"text": "hello",
|
||||
}
|
||||
],
|
||||
},
|
||||
})
|
||||
)?;
|
||||
|
||||
let (items, loaded_thread_id, parse_errors) =
|
||||
RolloutRecorder::load_rollout_items(&rollout_path).await?;
|
||||
|
||||
assert_eq!(loaded_thread_id, Some(thread_id));
|
||||
assert_eq!(parse_errors, 0);
|
||||
assert_eq!(items.len(), 2);
|
||||
assert!(matches!(items[0], RolloutItem::SessionMeta(_)));
|
||||
assert!(matches!(
|
||||
items[1],
|
||||
RolloutItem::ResponseItem(ResponseItem::Message { .. })
|
||||
));
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn load_rollout_items_filters_legacy_ghost_snapshots_from_compaction_history()
|
||||
-> std::io::Result<()> {
|
||||
let home = TempDir::new().expect("temp dir");
|
||||
let rollout_path = home.path().join("rollout.jsonl");
|
||||
let mut file = File::create(&rollout_path)?;
|
||||
let thread_id = ThreadId::new();
|
||||
let ts = "2025-01-03T12:00:00Z";
|
||||
|
||||
writeln!(
|
||||
file,
|
||||
"{}",
|
||||
serde_json::json!({
|
||||
"timestamp": ts,
|
||||
"type": "session_meta",
|
||||
"payload": {
|
||||
"id": thread_id,
|
||||
"timestamp": ts,
|
||||
"cwd": ".",
|
||||
"originator": "test_originator",
|
||||
"cli_version": "test_version",
|
||||
"source": "cli",
|
||||
"model_provider": "test-provider",
|
||||
},
|
||||
})
|
||||
)?;
|
||||
writeln!(
|
||||
file,
|
||||
"{}",
|
||||
serde_json::json!({
|
||||
"timestamp": ts,
|
||||
"type": "compacted",
|
||||
"payload": {
|
||||
"message": "summary",
|
||||
"replacement_history": [
|
||||
{
|
||||
"type": "message",
|
||||
"role": "assistant",
|
||||
"content": [
|
||||
{
|
||||
"type": "output_text",
|
||||
"text": "kept",
|
||||
}
|
||||
],
|
||||
},
|
||||
{
|
||||
"type": "ghost_snapshot",
|
||||
"ghost_commit": {
|
||||
"id": "deadbeef",
|
||||
"preexisting_untracked_dirs": [],
|
||||
"preexisting_untracked_files": [],
|
||||
},
|
||||
}
|
||||
],
|
||||
},
|
||||
})
|
||||
)?;
|
||||
|
||||
let (items, loaded_thread_id, parse_errors) =
|
||||
RolloutRecorder::load_rollout_items(&rollout_path).await?;
|
||||
|
||||
assert_eq!(loaded_thread_id, Some(thread_id));
|
||||
assert_eq!(parse_errors, 0);
|
||||
assert_eq!(items.len(), 2);
|
||||
let RolloutItem::Compacted(compacted) = &items[1] else {
|
||||
panic!("expected compacted rollout item");
|
||||
};
|
||||
let replacement_history = compacted
|
||||
.replacement_history
|
||||
.as_ref()
|
||||
.expect("replacement history");
|
||||
assert_eq!(replacement_history.len(), 1);
|
||||
assert!(matches!(
|
||||
&replacement_history[0],
|
||||
ResponseItem::Message { .. }
|
||||
));
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn recorder_materializes_on_flush_with_pending_items() -> std::io::Result<()> {
|
||||
let home = TempDir::new().expect("temp dir");
|
||||
|
||||
Reference in New Issue
Block a user