mirror of
https://github.com/openai/codex.git
synced 2026-09-16 12:13:30 +00:00
## Summary - move regular-turn context diff/full-context persistence into `run_turn` so pre-turn compaction runs before incoming context updates are recorded - after successful pre-turn compaction, rely on a cleared `reference_context_item` to trigger full context reinjection on the follow-up regular turn (manual `/compact` keeps replacement history summary-only and also clears the baseline) - preserve `<model_switch>` when full context is reinjected, and inject it *before* the rest of the full-context items - scope `reference_context_item` and `previous_model` to regular user turns only so standalone tasks (`/compact`, shell, review, undo) cannot suppress future reinjection or `<model_switch>` behavior - make context-diff persistence + `reference_context_item` updates explicit in the regular-turn path, with clearer docs/comments around the invariant - stop persisting local `/compact` `RolloutItem::TurnContext` snapshots (only regular turns persist `TurnContextItem` now) - simplify resume/fork previous-model/reference-baseline hydration by looking up the last surviving turn context from rollout lifecycle events, including rollback and compaction-crossing handling - remove the legacy fallback that guessed from bare `TurnContext` rollouts without lifecycle events - update compaction/remote-compaction/model-visible snapshots and compact test assertions (including remote compaction mock response shape) ## Why We were persisting incoming context items before spawning the regular turn task, which let pre-turn compaction requests accidentally include incoming context diffs without the new user message. Fixing that exposed follow-on baseline issues around `/compact`, resume/fork, and standalone tasks that could cause duplicate context injection or suppress `<model_switch>` instructions. This PR re-centers the invariants around regular turns: - regular turns persist model-visible context diffs/full reinjection and update the `reference_context_item` - standalone tasks do not advance those regular-turn baselines - compaction clears the baseline when replacement history may have stripped the referenced context diffs ## Follow-ups (TODOs left in code) - `TODO(ccunningham)`: fix rollback/backtracking baseline handling more comprehensively - `TODO(ccunningham)`: include pending incoming context items in pre-turn compaction threshold estimation - `TODO(ccunningham)`: inject updated personality spec alongside `<model_switch>` so some model-switch paths can avoid forced full reinjection - `TODO(ccunningham)`: review task turn lifecycle (`TurnStarted`/`TurnComplete`) behavior and emit task-start context diffs for task types that should have them (excluding `/compact`) ## Validation - `just fmt` - CI should cover the updated compaction/resume/model-visible snapshot expectations and rollout-hydration behavior - I did **not** rerun the full local test suite after the latest resume-lookup / rollout-persistence simplifications
128 lines
4.0 KiB
Rust
128 lines
4.0 KiB
Rust
use std::sync::Arc;
|
|
|
|
use crate::codex::TurnContext;
|
|
use crate::protocol::EventMsg;
|
|
use crate::protocol::UndoCompletedEvent;
|
|
use crate::protocol::UndoStartedEvent;
|
|
use crate::state::TaskKind;
|
|
use crate::tasks::SessionTask;
|
|
use crate::tasks::SessionTaskContext;
|
|
use async_trait::async_trait;
|
|
use codex_git::RestoreGhostCommitOptions;
|
|
use codex_git::restore_ghost_commit_with_options;
|
|
use codex_protocol::models::ResponseItem;
|
|
use codex_protocol::user_input::UserInput;
|
|
use tokio_util::sync::CancellationToken;
|
|
use tracing::error;
|
|
use tracing::info;
|
|
use tracing::warn;
|
|
|
|
pub(crate) struct UndoTask;
|
|
|
|
impl UndoTask {
|
|
pub(crate) fn new() -> Self {
|
|
Self
|
|
}
|
|
}
|
|
|
|
#[async_trait]
|
|
impl SessionTask for UndoTask {
|
|
fn kind(&self) -> TaskKind {
|
|
TaskKind::Regular
|
|
}
|
|
|
|
async fn run(
|
|
self: Arc<Self>,
|
|
session: Arc<SessionTaskContext>,
|
|
ctx: Arc<TurnContext>,
|
|
_input: Vec<UserInput>,
|
|
cancellation_token: CancellationToken,
|
|
) -> Option<String> {
|
|
let _ = session
|
|
.session
|
|
.services
|
|
.otel_manager
|
|
.counter("codex.task.undo", 1, &[]);
|
|
let sess = session.clone_session();
|
|
sess.send_event(
|
|
ctx.as_ref(),
|
|
EventMsg::UndoStarted(UndoStartedEvent {
|
|
message: Some("Undo in progress...".to_string()),
|
|
}),
|
|
)
|
|
.await;
|
|
|
|
if cancellation_token.is_cancelled() {
|
|
sess.send_event(
|
|
ctx.as_ref(),
|
|
EventMsg::UndoCompleted(UndoCompletedEvent {
|
|
success: false,
|
|
message: Some("Undo cancelled.".to_string()),
|
|
}),
|
|
)
|
|
.await;
|
|
return None;
|
|
}
|
|
|
|
let history = sess.clone_history().await;
|
|
let mut items = history.raw_items().to_vec();
|
|
let mut completed = UndoCompletedEvent {
|
|
success: false,
|
|
message: None,
|
|
};
|
|
|
|
let Some((idx, ghost_commit)) =
|
|
items
|
|
.iter()
|
|
.enumerate()
|
|
.rev()
|
|
.find_map(|(idx, item)| match item {
|
|
ResponseItem::GhostSnapshot { ghost_commit } => {
|
|
Some((idx, ghost_commit.clone()))
|
|
}
|
|
_ => None,
|
|
})
|
|
else {
|
|
completed.message = Some("No ghost snapshot available to undo.".to_string());
|
|
sess.send_event(ctx.as_ref(), EventMsg::UndoCompleted(completed))
|
|
.await;
|
|
return None;
|
|
};
|
|
|
|
let commit_id = ghost_commit.id().to_string();
|
|
let repo_path = ctx.cwd.clone();
|
|
let ghost_snapshot = ctx.ghost_snapshot.clone();
|
|
let restore_result = tokio::task::spawn_blocking(move || {
|
|
let options = RestoreGhostCommitOptions::new(&repo_path).ghost_snapshot(ghost_snapshot);
|
|
restore_ghost_commit_with_options(&options, &ghost_commit)
|
|
})
|
|
.await;
|
|
|
|
match restore_result {
|
|
Ok(Ok(())) => {
|
|
items.remove(idx);
|
|
let reference_context_item = sess.reference_context_item().await;
|
|
sess.replace_history(items, reference_context_item).await;
|
|
let short_id: String = commit_id.chars().take(7).collect();
|
|
info!(commit_id = commit_id, "Undo restored ghost snapshot");
|
|
completed.success = true;
|
|
completed.message = Some(format!("Undo restored snapshot {short_id}."));
|
|
}
|
|
Ok(Err(err)) => {
|
|
let message = format!("Failed to restore snapshot {commit_id}: {err}");
|
|
warn!("{message}");
|
|
completed.message = Some(message);
|
|
}
|
|
Err(err) => {
|
|
let message = format!("Failed to restore snapshot {commit_id}: {err}");
|
|
error!("{message}");
|
|
completed.message = Some(message);
|
|
}
|
|
}
|
|
|
|
sess.send_event(ctx.as_ref(), EventMsg::UndoCompleted(completed))
|
|
.await;
|
|
None
|
|
}
|
|
}
|