fix(review/epoch): scope snapshot tracking per repo

This commit is contained in:
James Peter
2025-12-02 13:52:01 +10:00
parent 1805acfc4c
commit 6d4ce25397
7 changed files with 121 additions and 52 deletions
+1
View File
@@ -1174,6 +1174,7 @@ dependencies = [
"code-rmcp-client",
"code-version",
"core-foundation 0.9.4",
"crc32fast",
"dirs",
"dunce",
"encoding_rs",
+1
View File
@@ -100,6 +100,7 @@ clap = "4"
clap_complete = "4"
color-eyre = "0.6.3"
crossterm = "0.28.1"
crc32fast = "1.5"
ctor = "0.5.0"
derive_more = "2"
diffy = "0.4.2"
+1
View File
@@ -78,6 +78,7 @@ tokio = { workspace = true, features = [
] }
tokio-tungstenite = { version = "0.23", features = ["rustls-tls-webpki-roots"] }
tokio-util = { workspace = true }
crc32fast = { workspace = true }
tokio-stream = { workspace = true }
toml = { workspace = true }
toml_edit = { workspace = true }
+11 -11
View File
@@ -6,7 +6,7 @@ use std::path::{Path, PathBuf};
use tokio::fs::OpenOptions;
use tokio::process::Command;
use crate::review_coord::bump_snapshot_epoch;
use crate::review_coord::bump_snapshot_epoch_for;
/// Returns the `/.../.code/branches/<worktree>` root when `path` resides inside a branch worktree.
pub fn branch_worktree_root(path: &Path) -> Option<PathBuf> {
@@ -175,7 +175,7 @@ pub async fn setup_worktree(
let stderr = String::from_utf8_lossy(&clean.stderr);
return Err(format!("Failed to clean existing worktree: {stderr}"));
}
bump_snapshot_epoch();
bump_snapshot_epoch_for(&worktree_path);
record_worktree_in_session(git_root, &worktree_path).await;
return Ok((worktree_path, effective_branch));
}
@@ -198,7 +198,7 @@ pub async fn setup_worktree(
.await
{
if out.status.success() {
bump_snapshot_epoch();
bump_snapshot_epoch_for(&worktree_path);
}
}
}
@@ -224,7 +224,7 @@ pub async fn setup_worktree(
.map_err(|e| format!("Failed to create git worktree: {}", e))?;
if output.status.success() {
bump_snapshot_epoch();
bump_snapshot_epoch_for(&worktree_path);
}
if !output.status.success() {
@@ -266,7 +266,7 @@ pub async fn setup_worktree(
let retry_err = String::from_utf8_lossy(&retry.stderr);
return Err(format!("Failed to create worktree: {}", retry_err));
}
bump_snapshot_epoch();
bump_snapshot_epoch_for(&worktree_path);
record_worktree_in_session(git_root, &worktree_path).await;
} else {
return Err(format!("Failed to create worktree: {}", stderr));
@@ -337,7 +337,7 @@ pub async fn prepare_reusable_worktree(
return Err(format!("Failed to clean reusable worktree: {stderr}"));
}
bump_snapshot_epoch();
bump_snapshot_epoch_for(&worktree_path);
record_worktree_in_session(git_root, &worktree_path).await;
return Ok(worktree_path);
}
@@ -363,7 +363,7 @@ pub async fn prepare_reusable_worktree(
return Err(format!("Failed to create reusable worktree: {stderr}"));
}
bump_snapshot_epoch();
bump_snapshot_epoch_for(&worktree_path);
record_worktree_in_session(git_root, &worktree_path).await;
Ok(worktree_path)
}
@@ -414,7 +414,7 @@ pub async fn ensure_local_default_remote(
let stderr = String::from_utf8_lossy(&update.stderr).trim().to_string();
return Err(format!("Failed to set {remote_name} URL: {stderr}"));
}
bump_snapshot_epoch();
bump_snapshot_epoch_for(git_root);
}
}
_ => {
@@ -428,7 +428,7 @@ pub async fn ensure_local_default_remote(
let stderr = String::from_utf8_lossy(&add.stderr).trim().to_string();
return Err(format!("Failed to add {remote_name}: {stderr}"));
}
bump_snapshot_epoch();
bump_snapshot_epoch_for(git_root);
}
}
@@ -467,7 +467,7 @@ pub async fn ensure_local_default_remote(
.map_err(|e| format!("Failed to update {remote_ref}: {e}"))?;
if update.status.success() {
metadata.remote_ref = Some(format!("{remote_name}/{base}"));
bump_snapshot_epoch();
bump_snapshot_epoch_for(git_root);
}
}
}
@@ -592,7 +592,7 @@ async fn _ensure_origin_remote(git_root: &Path) -> Result<(), String> {
if !add.status.success() {
return Err("failed to add origin".to_string());
}
bump_snapshot_epoch();
bump_snapshot_epoch_for(git_root);
let _ = Command::new("git")
.current_dir(git_root)
.args(["remote", "set-head", "origin", "-a"])
+61 -12
View File
@@ -2,6 +2,7 @@ use serde::{Deserialize, Serialize};
use std::fs::{self, File, OpenOptions};
use std::io::{Read, Write};
use std::path::{Path, PathBuf};
use std::process::Command;
use std::time::{SystemTime, UNIX_EPOCH};
const LOCK_FILENAME: &str = "review.lock";
@@ -28,10 +29,23 @@ fn state_dir() -> std::io::Result<PathBuf> {
Ok(dir)
}
fn epoch_path() -> std::io::Result<PathBuf> {
let mut p = state_dir()?;
p.push(EPOCH_FILENAME);
Ok(p)
fn epoch_path(scope: Option<&Path>) -> std::io::Result<PathBuf> {
if let Some(scope) = scope {
let normalized_scope = scope
.canonicalize()
.unwrap_or_else(|_| scope.to_path_buf());
let mut dir = state_dir()?;
let key = crc32fast::hash(normalized_scope.to_string_lossy().as_bytes());
dir.push(format!("repo-{key:08x}"));
fs::create_dir_all(&dir)?;
let mut p = dir;
p.push(EPOCH_FILENAME);
Ok(p)
} else {
let mut p = state_dir()?;
p.push(EPOCH_FILENAME);
Ok(p)
}
}
fn lock_path() -> std::io::Result<PathBuf> {
@@ -40,8 +54,8 @@ fn lock_path() -> std::io::Result<PathBuf> {
Ok(p)
}
fn read_epoch() -> u64 {
if let Ok(p) = epoch_path() {
fn read_epoch(scope: Option<&Path>) -> u64 {
if let Ok(p) = epoch_path(scope) {
if let Ok(text) = fs::read_to_string(p) {
if let Ok(v) = text.trim().parse::<u64>() {
return v;
@@ -51,18 +65,53 @@ fn read_epoch() -> u64 {
0
}
fn write_epoch(val: u64) -> std::io::Result<()> {
let p = epoch_path()?;
fn write_epoch(scope: Option<&Path>, val: u64) -> std::io::Result<()> {
let p = epoch_path(scope)?;
fs::write(p, val.to_string())
}
pub fn bump_snapshot_epoch() {
let current = read_epoch();
let _ = write_epoch(current.saturating_add(1));
if let Some(scope) = scope_from_current_dir() {
bump_snapshot_epoch_for(&scope);
return;
}
let current = read_epoch(None);
let _ = write_epoch(None, current.saturating_add(1));
}
pub fn current_snapshot_epoch() -> u64 {
read_epoch()
if let Some(scope) = scope_from_current_dir() {
return current_snapshot_epoch_for(&scope);
}
read_epoch(None)
}
pub fn bump_snapshot_epoch_for(scope: &Path) {
let current = read_epoch(Some(scope));
let _ = write_epoch(Some(scope), current.saturating_add(1));
}
pub fn current_snapshot_epoch_for(scope: &Path) -> u64 {
read_epoch(Some(scope))
}
fn scope_from_current_dir() -> Option<PathBuf> {
let cwd = std::env::current_dir().ok()?;
if let Ok(out) = Command::new("git")
.current_dir(&cwd)
.args(["rev-parse", "--show-toplevel"])
.output()
{
if out.status.success() {
let path = String::from_utf8_lossy(&out.stdout).trim().to_string();
if !path.is_empty() {
return Some(PathBuf::from(path));
}
}
}
Some(cwd)
}
fn git_head(cwd: &Path) -> Option<String> {
@@ -94,7 +143,7 @@ pub fn try_acquire_lock(intent: &str, cwd: &Path) -> std::io::Result<Option<Revi
.as_secs(),
intent: intent.to_string(),
git_head: git_head(cwd),
snapshot_epoch: current_snapshot_epoch(),
snapshot_epoch: current_snapshot_epoch_for(cwd),
};
let body = serde_json::to_string_pretty(&info).unwrap_or_default();
let _ = f.write_all(body.as_bytes());
+20 -13
View File
@@ -25,7 +25,12 @@ use code_core::config::Config;
use code_core::config::ConfigOverrides;
use code_core::git_info::get_git_repo_root;
use code_core::git_info::recent_commits;
use code_core::review_coord::{bump_snapshot_epoch, current_snapshot_epoch, try_acquire_lock};
use code_core::review_coord::{
bump_snapshot_epoch_for,
clear_stale_lock_if_dead,
current_snapshot_epoch_for,
try_acquire_lock,
};
use code_core::protocol::AskForApproval;
use code_core::protocol::Event;
use code_core::protocol::EventMsg;
@@ -544,7 +549,7 @@ pub async fn run_main(cli: Cli, code_linux_sandbox_exe: Option<PathBuf>) -> anyh
let mut _review_guard: Option<code_core::review_coord::ReviewGuard> = None;
// Clear stale review lock in case a prior process crashed.
let _ = code_core::review_coord::clear_stale_lock_if_dead();
let _ = clear_stale_lock_if_dead();
let _initial_prompt_task_id = if let Some(mut review_request) = review_request.clone() {
// Cross-process review coordination
@@ -563,7 +568,7 @@ pub async fn run_main(cli: Cli, code_linux_sandbox_exe: Option<PathBuf>) -> anyh
if auto_resolve_base_snapshot.is_none() {
auto_resolve_base_snapshot = capture_auto_resolve_snapshot(&config.cwd, None, "auto-resolve base snapshot");
if let Some(state) = auto_resolve_state.as_mut() {
state.snapshot_epoch = Some(current_snapshot_epoch());
state.snapshot_epoch = Some(current_snapshot_epoch_for(&config.cwd));
}
}
@@ -583,7 +588,7 @@ pub async fn run_main(cli: Cli, code_linux_sandbox_exe: Option<PathBuf>) -> anyh
}
// Capture baseline epoch after any snapshot creation so we don't trip on our own bumps.
last_review_epoch = Some(current_snapshot_epoch());
last_review_epoch = Some(current_snapshot_epoch_for(&config.cwd));
let event_id = conversation.submit(Op::Review { review_request }).await?;
info!("Sent /review with event ID: {event_id}");
@@ -624,7 +629,7 @@ pub async fn run_main(cli: Cli, code_linux_sandbox_exe: Option<PathBuf>) -> anyh
final_review_snapshot = Some(snapshot.clone());
// detect stale snapshot epoch
if let Some(start_epoch) = last_review_epoch {
let current_epoch = current_snapshot_epoch();
let current_epoch = current_snapshot_epoch_for(&config.cwd);
if current_epoch != start_epoch {
eprintln!("Snapshot epoch changed during review; aborting auto-resolve and requiring restart.");
auto_resolve_state = None;
@@ -708,7 +713,7 @@ pub async fn run_main(cli: Cli, code_linux_sandbox_exe: Option<PathBuf>) -> anyh
}
EventMsg::TaskComplete(TaskCompleteEvent { last_agent_message }) => {
if let Some(state_snapshot) = auto_resolve_state.clone() {
let current_epoch = current_snapshot_epoch();
let current_epoch = current_snapshot_epoch_for(&config.cwd);
match state_snapshot.phase {
AutoResolvePhase::PendingFix { review } => {
if auto_resolve_fix_guard.is_none() {
@@ -799,7 +804,7 @@ pub async fn run_main(cli: Cli, code_linux_sandbox_exe: Option<PathBuf>) -> anyh
if let Some(state) = auto_resolve_state.as_mut() {
state.last_reviewed_commit = Some(snap.id().to_string());
state.snapshot_epoch = Some(current_snapshot_epoch());
state.snapshot_epoch = Some(current_snapshot_epoch_for(&config.cwd));
}
let followup_request = build_followup_review_request(
@@ -810,7 +815,7 @@ pub async fn run_main(cli: Cli, code_linux_sandbox_exe: Option<PathBuf>) -> anyh
Some(base.id()),
)
.await;
last_review_epoch = Some(current_snapshot_epoch());
last_review_epoch = Some(current_snapshot_epoch_for(&config.cwd));
let _ = conversation
.submit(Op::Review {
review_request: followup_request,
@@ -840,7 +845,7 @@ pub async fn run_main(cli: Cli, code_linux_sandbox_exe: Option<PathBuf>) -> anyh
auto_resolve_base_snapshot = None;
continue;
}
let current_epoch = current_snapshot_epoch();
let current_epoch = current_snapshot_epoch_for(&config.cwd);
if let Some(state) = auto_resolve_state.as_ref() {
if state
.metadata
@@ -883,7 +888,7 @@ pub async fn run_main(cli: Cli, code_linux_sandbox_exe: Option<PathBuf>) -> anyh
Some(base.id()),
)
.await;
last_review_epoch = Some(current_snapshot_epoch());
last_review_epoch = Some(current_snapshot_epoch_for(&config.cwd));
let _ = conversation
.submit(Op::Review {
review_request: followup_request,
@@ -1265,15 +1270,17 @@ fn capture_auto_resolve_snapshot(
parent: Option<&str>,
message: &'static str,
) -> Option<GhostCommit> {
let cwd_buf = cwd.to_path_buf();
let hook = move || bump_snapshot_epoch_for(&cwd_buf);
let mut options = CreateGhostCommitOptions::new(cwd)
.message(message)
.post_commit_hook(&|| bump_snapshot_epoch());
.post_commit_hook(&hook);
if let Some(parent) = parent {
options = options.parent(parent);
}
let snap = create_ghost_commit(&options).ok();
if snap.is_some() {
bump_snapshot_epoch();
bump_snapshot_epoch_for(cwd);
}
snap
}
@@ -1373,7 +1380,7 @@ fn capture_snapshot_against_base(
if diff_paths.is_empty() {
return None;
}
bump_snapshot_epoch();
bump_snapshot_epoch_for(cwd);
Some((snapshot, diff_paths))
}
+26 -16
View File
@@ -250,7 +250,7 @@ use code_core::protocol::PatchApplyEndEvent;
use code_core::protocol::TaskCompleteEvent;
use code_core::protocol::TokenUsage;
use code_core::protocol::TurnDiffEvent;
use code_core::review_coord::{bump_snapshot_epoch, try_acquire_lock, ReviewGuard};
use code_core::review_coord::{bump_snapshot_epoch_for, try_acquire_lock, ReviewGuard};
use code_core::ConversationManager;
use code_core::codex::compact::COMPACTION_CHECKPOINT_MESSAGE;
use crate::bottom_pane::{
@@ -716,7 +716,7 @@ async fn run_fast_forward_merge(state: &MergeRepoState) -> Result<(), String> {
));
}
bump_snapshot_epoch();
bump_snapshot_epoch_for(&state.git_root);
let worktree_remove = Command::new("git")
.current_dir(&state.git_root)
@@ -10043,8 +10043,11 @@ impl ChatWidget<'_> {
);
let repo_path = self.config.cwd.clone();
let started_at = request.started_at;
let result = create_ghost_commit(&CreateGhostCommitOptions::new(repo_path.as_path())
.post_commit_hook(&|| bump_snapshot_epoch()));
let hook_repo = repo_path.clone();
let result = create_ghost_commit(
&CreateGhostCommitOptions::new(repo_path.as_path())
.post_commit_hook(&move || bump_snapshot_epoch_for(&hook_repo)),
);
let elapsed = started_at.elapsed();
let snapshot = self.finalize_ghost_snapshot(request, result, elapsed);
snapshot
@@ -10228,8 +10231,9 @@ impl ChatWidget<'_> {
tokio::spawn(async move {
let handle = tokio::task::spawn_blocking(move || {
let hook_repo = repo_path.clone();
let options = CreateGhostCommitOptions::new(repo_path.as_path());
create_ghost_commit(&options.post_commit_hook(&|| bump_snapshot_epoch()))
create_ghost_commit(&options.post_commit_hook(&move || bump_snapshot_epoch_for(&hook_repo)))
});
tokio::pin!(handle);
@@ -10924,7 +10928,7 @@ impl ChatWidget<'_> {
.iter()
.any(|arg| matches!(arg.as_str(), "pull" | "checkout" | "merge" | "apply"))
{
bump_snapshot_epoch();
bump_snapshot_epoch_for(&self.config.cwd);
}
parser(String::from_utf8_lossy(&output.stdout).to_string())
}
@@ -16577,15 +16581,15 @@ Have we met every part of this goal and is there no further work to do?"#
let parent_id = parent.map(|commit| commit.id().to_string());
return stub(message, parent_id);
}
let mut options = CreateGhostCommitOptions::new(self.config.cwd.as_path())
.message(message)
.post_commit_hook(&|| bump_snapshot_epoch());
let mut options = CreateGhostCommitOptions::new(self.config.cwd.as_path()).message(message);
if let Some(parent_commit) = parent {
options = options.parent(parent_commit.id());
}
let result = create_ghost_commit(&options.post_commit_hook(&|| bump_snapshot_epoch()));
let hook_repo_follow = self.config.cwd.clone();
let hook = move || bump_snapshot_epoch_for(&hook_repo_follow);
let result = create_ghost_commit(&options.post_commit_hook(&hook));
if result.is_ok() {
bump_snapshot_epoch();
bump_snapshot_epoch_for(&self.config.cwd);
}
result
}
@@ -25599,12 +25603,13 @@ async fn run_background_review(
let base_snapshot = base_snapshot.clone();
move || {
let mut options = CreateGhostCommitOptions::new(repo_path.as_path())
.message("auto review snapshot")
.post_commit_hook(&|| bump_snapshot_epoch());
.message("auto review snapshot");
if let Some(base) = base_snapshot.as_ref() {
options = options.parent(base.id());
}
create_ghost_commit(&options.post_commit_hook(&|| bump_snapshot_epoch()))
let hook_repo = repo_path.clone();
let hook = move || bump_snapshot_epoch_for(&hook_repo);
create_ghost_commit(&options.post_commit_hook(&hook))
}
})
.await
@@ -25612,7 +25617,7 @@ async fn run_background_review(
.and_then(|res| res.map_err(|e| format!("failed to capture snapshot: {e}")))?;
let snapshot_id = snapshot.id().to_string();
bump_snapshot_epoch();
bump_snapshot_epoch_for(&config.cwd);
// Reuse a single detached worktree for auto-review to avoid churn; keep
// gitignored build outputs for faster incremental builds.
let worktree_path = code_core::git_worktree::prepare_reusable_worktree(
@@ -25773,7 +25778,12 @@ impl Drop for AutoReviewStubGuard {
};
use code_core::parse_command::ParsedCommand;
use code_core::protocol::OrderMeta;
use code_core::review_coord::{bump_snapshot_epoch, current_snapshot_epoch, try_acquire_lock, ReviewGuard};
use code_core::review_coord::{
bump_snapshot_epoch_for,
current_snapshot_epoch,
try_acquire_lock,
ReviewGuard,
};
use code_core::protocol::{
AskForApproval,
AgentMessageEvent,