diff --git a/docs/guides/codebase.md b/docs/guides/codebase.md index 41ab623..ac8a448 100644 --- a/docs/guides/codebase.md +++ b/docs/guides/codebase.md @@ -68,6 +68,24 @@ Each dispatch gets its own private environment holding: An eval that declares no `codebase` still gets a Git repository, initialized on `work`, exactly as it always has. +## One checkout per iteration + +Every environment a run provisions — each `(eval, condition, run)` cell — is built from one cached +checkout per distinct codebase, materialized once per iteration under `iteration-N/.codebase/` +when the run prepares. +Environments are provisioned from that cache with `git clone --local`: Git hard-links the object +store instead of copying it and checks out a fresh working tree, so `--runs 10` against a large +repository costs one clone plus a working tree per environment, not a full copy of the tree and +its history per environment. + +Shared objects are content-addressed and immutable — Git never rewrites an object once written — +so each environment is still an independent working tree with an independent history. Commits, +branches, and edits in one environment are invisible to the others and to the cache. + +Where hard-linking is unavailable (the cache and the environments on different filesystems) or the +source carries no Git history to clone, environments fall back to a plain copy of the cache. The +result is the same tree, provisioned more slowly. + ## `files` is an overlay `files` and `files_root` still work, and are applied *on top* of the codebase at their declared diff --git a/src/cli/run/orchestrate/mod.rs b/src/cli/run/orchestrate/mod.rs index 3e2be91..08f79ec 100644 --- a/src/cli/run/orchestrate/mod.rs +++ b/src/cli/run/orchestrate/mod.rs @@ -308,6 +308,21 @@ fn print_run_plan(ctx: &RunContext, opts: &RunOptions, r: &Resolved) { " skill source: {}{revision}", source.resolved_path.as_deref().unwrap_or(&source.source) ); + // The codebases the environments are built from, in the same shape as the + // skill source line — and the one-checkout-per-iteration fact the caching + // makes true. + for codebase in &r.codebases { + let source = &codebase.source; + let revision = source + .revision + .as_deref() + .map(|sha| format!(" ({})", &sha[..7.min(sha.len())])) + .unwrap_or_default(); + println!( + " codebase: {}{revision} — materialized once per iteration", + source.resolved_path.as_deref().unwrap_or(&source.source) + ); + } if r.selected_evals.len() != r.total_evals { let (flag, ids) = match (opts.only, opts.skip) { (Some(ids), _) => ("--only", ids), diff --git a/src/cli/run/orchestrate/stage.rs b/src/cli/run/orchestrate/stage.rs index 6c3d4e3..fc9b13e 100644 --- a/src/cli/run/orchestrate/stage.rs +++ b/src/cli/run/orchestrate/stage.rs @@ -110,13 +110,17 @@ pub(super) fn stage_conditions( // agent left behind. Start from nothing instead. fs::remove_dir_all(&target.root)?; } - fs::create_dir_all(&target.root)?; - // The codebase goes down first: staged skills and the `files` overlay are - // both applied *on top* of it. + // both applied *on top* of it. Every environment is provisioned from the + // iteration's single cached materialization of it — a local clone while + // the host allows the hard link, a plain copy otherwise — so `--runs 10` + // against a real repository costs one checkout, not ten copies. if let Some(codebase) = codebase { let source_tree = materialize_codebase(&r.iteration_dir, codebase, &mut materialized)?; - copy_entry_materialized(&source_tree, &target.root)?; + crate::source::provision_env(&codebase.source, &source_tree, &target.root) + .map_err(|error| RunError::msg(error.to_string()))?; + } else { + fs::create_dir_all(&target.root)?; } if !opts.no_stage { @@ -234,9 +238,11 @@ fn copy_skill_dir(source: &Path, dest: &Path, root: &Path) -> Result<(), RunErro /// The materialized tree for `codebase`, creating it on first use. /// -/// One materialization per distinct codebase per iteration; each environment is -/// then provisioned from it by copy. Cloning per environment instead would mean -/// one network round trip per `(group, condition, run)` cell. +/// One materialization per distinct codebase per iteration; every environment +/// is then provisioned from it ([`crate::source::provision_env`] — a local +/// clone while the host allows the hard link, a copy otherwise). Materializing +/// per environment instead would mean one network round trip per +/// `(group, condition, run)` cell. fn materialize_codebase( iteration_dir: &Path, codebase: &super::RunCodebase, diff --git a/src/core/fs.rs b/src/core/fs.rs index ec2f010..1529bfe 100644 --- a/src/core/fs.rs +++ b/src/core/fs.rs @@ -183,6 +183,31 @@ pub fn copy_entry_materialized(source: &Path, destination: &Path) -> io::Result< Ok(()) } +/// Whether a file created under `from` can be hard-linked into `to` — the +/// capability `git clone --local` relies on to share an object store instead +/// of copying it. +/// +/// Two directories a run owns can sit on different filesystems (a workspace +/// on a mounted volume, a cache on tmpfs), and `link(2)` is what says so. +/// Any failure reads as unavailable, so the caller falls back to copying rather +/// than provisioning wrong. +pub fn hardlinks_available(from: &Path, to: &Path) -> bool { + let Ok(probe) = tempfile::NamedTempFile::new_in(from) else { + return false; + }; + let Some(name) = probe.path().file_name() else { + return false; + }; + let target = to.join(name); + match fs::hard_link(probe.path(), &target) { + Ok(()) => { + let _ = fs::remove_file(&target); + true + } + Err(_) => false, + } +} + /// Create a symlink at `link` pointing at `target`. /// /// `to_directory` is consulted only on Windows, which has separate file and @@ -605,4 +630,42 @@ mod tests { assert_eq!(err.kind(), io::ErrorKind::NotFound); } + + /// The probe both succeeds and cleans up after itself: it runs inside the + /// per-iteration codebase cache, where a leftover file would ship into the + /// next environment built from it. + #[test] + fn hardlinks_available_is_true_between_directories_on_one_filesystem() { + let tmp = TempDir::new().unwrap(); + let from = tmp.path().join("from"); + let to = tmp.path().join("to"); + fs::create_dir_all(&from).unwrap(); + fs::create_dir_all(&to).unwrap(); + + assert!(hardlinks_available(&from, &to)); + + assert_eq!( + fs::read_dir(&from).unwrap().count(), + 0, + "no probe residue in from" + ); + assert_eq!( + fs::read_dir(&to).unwrap().count(), + 0, + "no probe residue in to" + ); + } + + /// Any failure — a missing directory on either side, a filesystem that + /// refuses the link — reads as "unavailable", so callers fall back to + /// copying rather than provisioning wrong. + #[test] + fn hardlinks_available_is_false_when_a_directory_is_missing() { + let tmp = TempDir::new().unwrap(); + let present = tmp.path().join("present"); + fs::create_dir_all(&present).unwrap(); + + assert!(!hardlinks_available(&tmp.path().join("absent"), &present)); + assert!(!hardlinks_available(&present, &tmp.path().join("absent"))); + } } diff --git a/src/source/mod.rs b/src/source/mod.rs index 4029b75..2562048 100644 --- a/src/source/mod.rs +++ b/src/source/mod.rs @@ -1,9 +1,12 @@ -//! Resolving a declared source to a revision, and materializing it as a tree. +//! Resolving a declared source to a revision, materializing it as a tree, and +//! provisioning task environments from that tree. //! -//! Two phases, deliberately split. [`resolve`] is read-only: it answers "what -//! exactly does this declaration point at?" without creating a directory, so a -//! run can fail on an unreachable repository or a ref that does not exist before -//! it has built any part of a workspace. +//! Three operations, deliberately split. [`resolve`] is read-only: it answers +//! "what exactly does this declaration point at?" without creating a directory, +//! so a run can fail on an unreachable repository or a ref that does not exist +//! before it has built any part of a workspace. [`materialize`] creates the one +//! cached checkout an iteration shares; [`provision_env`] turns that cache into +//! each individual task environment. //! //! Nothing here knows what a codebase is. A caller hands it a [`SourceSpec`] and //! gets back a [`ResolvedSource`]; the eval config's `codebase` block is one @@ -303,6 +306,96 @@ fn clone_repository( Ok(()) } +/// How [`provision_env`] produced an environment. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum EnvProvisioning { + /// `git clone --local` from the cache: Git hard-links the object store and + /// checks out a fresh working tree, so the history arrives intact while the + /// cache's bytes are paid for once per iteration, not once per environment. + LocalClone, + /// A plain materialized copy of the cache: taken when the host cannot + /// hard-link between the two directories, or the cache carries no commits + /// to check out. + PlainCopy, +} + +/// Produce one task environment at `dest` from the cached materialization of +/// `resolved` that [`materialize`] left at `cache`. +/// +/// A run materializes each distinct codebase once per iteration and provisions +/// every `(group, condition, run)` environment from that single checkout. A +/// local clone is the fast path: Git hard-links the object store instead of +/// copying it, and the clone's history is the checkout's history. A local +/// clone also names its source as an `origin` remote — removed here, so no +/// environment retains a path back to the cache. The plain copy stands in +/// wherever cloning could not deliver: a cache without commits (an empty +/// repository clones to an empty working tree) or a host that refuses the +/// hard link (a cache and an environment on different filesystems). +/// +/// `dest` must not already exist. Returns how the environment was provisioned. +pub fn provision_env( + resolved: &ResolvedSource, + cache: &Path, + dest: &Path, +) -> Result { + if resolved.revision.is_none() { + copy_from_cache(cache, dest)?; + return Ok(EnvProvisioning::PlainCopy); + } + let git = IsolatedGit::new().map_err(SourceError::msg)?; + let parent = dest.parent().unwrap_or(dest); + std::fs::create_dir_all(parent).map_err(|error| { + SourceError::msg(format!("could not create {}: {error}", parent.display())) + })?; + // Probing `cache/.git`, not the working tree: a probe file in the tree + // would be a leftover in the cache even after deletion racing a clone, + // while `.git` is metadata no checkout ever reads. + if !crate::core::fs::hardlinks_available(&cache.join(".git"), parent) { + copy_from_cache(cache, dest)?; + return Ok(EnvProvisioning::PlainCopy); + } + checked( + &git, + Path::new("."), + &[ + "clone", + "--quiet", + "--local", + "--template", + &git.template_dir().to_string_lossy(), + &cache.to_string_lossy(), + &dest.to_string_lossy(), + ], + "clone the cached codebase checkout", + )?; + checked( + &git, + dest, + &["remote", "remove", "origin"], + "remove the cache as a remote", + )?; + Ok(EnvProvisioning::LocalClone) +} + +/// The fallback provisioning: a materialized copy of the whole cache, for a +/// host that cannot hard-link between the two directories or a cache with no +/// commits to check out. +fn copy_from_cache(cache: &Path, dest: &Path) -> Result<(), SourceError> { + std::fs::create_dir_all(dest).map_err(|error| { + SourceError::msg(format!( + "could not create environment {}: {error}", + dest.display() + )) + })?; + crate::core::fs::copy_entry_materialized(cache, dest).map_err(|error| { + SourceError::msg(format!( + "could not copy cached codebase {} into {}: {error}", + cache.display(), + dest.display() + )) + }) +} + /// Run git in `cwd`, turning a non-zero exit into an error naming the intent. fn checked(git: &IsolatedGit, cwd: &Path, args: &[&str], intent: &str) -> Result<(), SourceError> { let output = git.run(cwd, args); diff --git a/src/source/tests.rs b/src/source/tests.rs index 7d2691e..81abea8 100644 --- a/src/source/tests.rs +++ b/src/source/tests.rs @@ -480,3 +480,131 @@ fn materializing_a_dirty_local_repository_carries_only_committed_state() { assert!(!dest.join("untracked.txt").exists()); assert_eq!(git_text(&dest, &["remote"]), ""); } + +/// The environment a run provisions from its cached materialization: +/// a local clone while the cache carries check-outable history and the +/// host allows hard links, a plain copy otherwise. +#[test] +fn provisioning_an_env_from_a_cached_checkout_clones_with_history_and_no_remote() { + let tmp = tempfile::TempDir::new().unwrap(); + let origin = source_repo(tmp.path(), "origin", "main"); + commit(&origin, "second.txt", "second"); + let resolved = resolve( + &SourceSpec::Git { + url: origin.to_string_lossy().into_owned(), + reference: "main".to_string(), + }, + tmp.path(), + "codebase", + ) + .unwrap(); + let cache = tmp.path().join("cache"); + materialize(&resolved, &cache).unwrap(); + let env = tmp.path().join("env"); + + let outcome = provision_env(&resolved, &cache, &env).expect("provisioning succeeds"); + + assert!( + matches!(outcome, EnvProvisioning::LocalClone), + "a cached checkout with commits and a hard-linking host clones" + ); + assert_eq!(sha(&env, "HEAD"), resolved.revision.unwrap()); + assert_eq!(git_text(&env, &["symbolic-ref", "--short", "HEAD"]), "main"); + assert_eq!( + git_text(&env, &["rev-list", "--count", "HEAD"]), + "2", + "a local clone carries the cached checkout's history" + ); + assert_eq!( + git_text(&env, &["remote"]), + "", + "a local clone names its source as a remote, so provisioning removes it" + ); + assert_eq!(git_text(&env, &["status", "--porcelain"]), ""); + assert_eq!( + std::fs::read_to_string(env.join("second.txt")).unwrap(), + "second +" + ); +} + +/// A cache materialized from a directory with no Git history has no commits, +/// and `git clone` of an empty repository checks out nothing — the only +/// correct provisioning there is the plain copy. +#[test] +fn provisioning_a_commitless_cache_falls_back_to_a_plain_copy() { + let tmp = tempfile::TempDir::new().unwrap(); + let plain = tmp.path().join("plain-project"); + std::fs::create_dir_all(plain.join("src")).unwrap(); + std::fs::write( + plain.join("src/main.rs"), + "fn main() {} +", + ) + .unwrap(); + let resolved = resolve( + &SourceSpec::Path { + path: plain.to_string_lossy().into_owned(), + }, + tmp.path(), + "codebase", + ) + .unwrap(); + let cache = tmp.path().join("cache"); + materialize(&resolved, &cache).unwrap(); + let env = tmp.path().join("env"); + + let outcome = provision_env(&resolved, &cache, &env).expect("provisioning succeeds"); + + assert!(matches!(outcome, EnvProvisioning::PlainCopy)); + assert_eq!( + std::fs::read_to_string(env.join("src/main.rs")).unwrap(), + "fn main() {} +", + "a commitless cache must still populate the environment's working tree" + ); +} + +/// Environments share the cache's objects by hard link, but +/// each keeps a private working tree and private refs — a +/// write in one is invisible to the others and to the cache. +#[test] +fn envs_provisioned_from_one_cache_are_independent_working_trees() { + let tmp = tempfile::TempDir::new().unwrap(); + let origin = source_repo(tmp.path(), "origin", "main"); + let resolved = resolve( + &SourceSpec::Git { + url: origin.to_string_lossy().into_owned(), + reference: "main".to_string(), + }, + tmp.path(), + "codebase", + ) + .unwrap(); + let cache = tmp.path().join("cache"); + materialize(&resolved, &cache).unwrap(); + let env_one = tmp.path().join("env-one"); + let env_two = tmp.path().join("env-two"); + provision_env(&resolved, &cache, &env_one).unwrap(); + provision_env(&resolved, &cache, &env_two).unwrap(); + + commit(&env_one, "agent-work.txt", "work committed in env one"); + + assert_eq!( + git_text(&env_one, &["rev-list", "--count", "HEAD"]), + "2", + "the writing environment moved ahead on its own history" + ); + assert_eq!( + git_text(&env_two, &["rev-list", "--count", "HEAD"]), + "1", + "the other environment's history is untouched" + ); + assert_eq!( + git_text(&cache, &["rev-list", "--count", "HEAD"]), + "1", + "the cache every environment shares is never mutated" + ); + assert!(!env_two.join("agent-work.txt").exists()); + assert!(!cache.join("agent-work.txt").exists()); +} diff --git a/tests/cli/docs.rs b/tests/cli/docs.rs index e9c10e2..633c81f 100644 --- a/tests/cli/docs.rs +++ b/tests/cli/docs.rs @@ -163,10 +163,11 @@ fn docs_isolation_keeps_remedies_and_verification() { /// The codebase guide is the reference surface for a feature with no CLI flag, /// so the parts a config author cannot infer have to survive an edit: that a -/// git ref is mandatory, that `files` layers over the checkout, and that a local -/// path is not reproducible by anyone reading the results. +/// git ref is mandatory, that `files` layers over the checkout, that a local +/// path is not reproducible by anyone reading the results, and how the +/// per-iteration cache provisions environments. #[test] -fn docs_codebase_keeps_the_declaration_rules_and_reproducibility_caveat() { +fn docs_codebase_keeps_the_declaration_rules_caveat_and_provisioning_contract() { skill_eval() .args(["docs", "codebase"]) .assert() @@ -177,7 +178,10 @@ fn docs_codebase_keeps_the_declaration_rules_and_reproducibility_caveat() { .stdout(contains("overlay")) .stdout(contains("refs/eval-magic/baseline")) .stdout(contains("host_local")) - .stdout(contains("not reproducible")); + .stdout(contains("not reproducible")) + .stdout(contains("materialized once")) + .stdout(contains("hard-link")) + .stdout(contains("independent working tree")); } #[test] diff --git a/tests/run/codebase.rs b/tests/run/codebase.rs index 146cfe3..15a54b8 100644 --- a/tests/run/codebase.rs +++ b/tests/run/codebase.rs @@ -347,3 +347,319 @@ fn a_fixture_only_eval_still_gets_the_repository_it_always_had() { git(&env, &["rev-parse", "HEAD"]) ); } + +/// The number of hard links to `file` — the mechanism `git clone --local` uses +/// to share the cache's object store with an environment instead of copying +/// it. Straight from stat metadata on Unix. +#[cfg(unix)] +fn link_count(file: &Path) -> u32 { + use std::os::unix::fs::MetadataExt; + fs::metadata(file).unwrap().nlink() as u32 +} + +/// The number of hard links to `file`, read from fsutil because Windows has no +/// stable std route to it: `number_of_links` rides the unstable +/// `windows_by_handle` trait. fsutil prints one path per hard link, sometimes +/// behind a `Hardlink list on ...` header — the header is the only printed +/// line that is not a path. +#[cfg(windows)] +fn link_count(file: &Path) -> u32 { + let output = Command::new("fsutil") + .args(["hardlink", "list"]) + .arg(file) + .output() + .expect("fsutil hardlink list must run"); + assert!( + output.status.success(), + "fsutil hardlink list failed for {}: {}", + file.display(), + String::from_utf8_lossy(&output.stderr) + ); + hardlink_list_count(&String::from_utf8_lossy(&output.stdout)) +} + +/// Count the hard links in `fsutil hardlink list` output: one path per line, +/// sometimes behind a `Hardlink list on ...` header — the header is the only +/// printed line that is not a path. +fn hardlink_list_count(output: &str) -> u32 { + output + .lines() + .map(str::trim) + .filter(|line| line.contains('\\') && !line.starts_with("Hardlink")) + .count() as u32 +} + +/// Both layouts `fsutil hardlink list` prints. Pinned here because the +/// Windows arm of `link_count` runs only on Windows, while the counting is +/// plain string logic every runner can execute. +#[test] +fn fsutil_link_list_output_is_counted_in_both_of_its_formats() { + // Modern Windows: one \?\-prefixed path per hard link, no header. + let modern = r"\\?\C:\cache\.git\objects\ab\cdef +\\?\C:\env\.git\objects\ab\cdef +"; + assert_eq!(hardlink_list_count(modern), 2); + // Older Windows: the same paths behind a `Hardlink list on ...` header, + // CRLF-terminated. + let older_lf = r"Hardlink list on C:\cache\.git\objects\ab\cdef +C:\cache\.git\objects\ab\cdef +C:\env\.git\objects\ab\cdef +"; + let older = older_lf.replace('\n', "\r\n"); + assert_eq!(hardlink_list_count(&older), 2); + // A file no other path shares lists exactly once — the count that fails + // the hard-link assertions when an environment was copied, not cloned. + let lone = r"\\?\C:\env\.git\objects\ab\cdef +"; + assert_eq!(hardlink_list_count(lone), 1); +} + +/// A file from `repo`'s object store — a loose object or a pack — that a local +/// clone shares with its source by hard link. `objects/info` is skipped: it +/// holds per-repository metadata (an exclude file), not objects, and is never +/// shared. +fn an_object_file(repo: &Path) -> PathBuf { + fn walk(dir: &Path) -> Option { + for entry in fs::read_dir(dir).unwrap() { + let path = entry.unwrap().path(); + if path.is_dir() { + if path.file_name().is_some_and(|name| name == "info") { + continue; + } + if let Some(found) = walk(&path) { + return Some(found); + } + } else { + return Some(path); + } + } + None + } + walk(&repo.join(".git/objects")).expect("a materialized repository has objects") +} + +/// One cached materialization provisions every environment of a +/// multi-run campaign, and the provisioning is a local clone — each +/// environment's object store is hard-linked to the cache's, not copied. +#[test] +fn multi_run_envs_are_provisioned_from_one_cached_materialization() { + let tmp = tempfile::TempDir::new().unwrap(); + let origin = codebase_repo(tmp.path(), "origin", "main"); + let source = format!(r#"{{ "url": "{}", "ref": "main" }}"#, wire_path(&origin)); + let (skill_dir, cwd) = setup(tmp.path(), &evals_with_codebase(&source)); + fs::write( + skill_dir.join("mr-review/evals/TASK.md"), + "task +", + ) + .unwrap(); + + skill_eval() + .current_dir(&cwd) + .args(["run", "--skill-dir"]) + .arg(&skill_dir) + .args([ + "--skill", + "mr-review", + "--mode", + "new-skill", + "--runs", + "2", + "--dry-run", + ]) + .assert() + .success(); + + // One codebase declaration resolves to one cached materialization, shared + // by every environment the run provisions. + let iteration = iteration_dir(&cwd); + let cached: Vec<_> = fs::read_dir(iteration.join(".codebase")).unwrap().collect(); + assert_eq!(cached.len(), 1, "one codebase, one cached materialization"); + // One object file the cache holds, spelled relative to the cache root — the + // same content-addressed path every environment provisioned from it holds. + let cache = cached[0].as_ref().unwrap().path(); + let object = an_object_file(&cache); + let object_relative = object.strip_prefix(&cache).unwrap(); + + for condition in ["with_skill", "without_skill"] { + for run in [1, 2] { + let env = iteration.join(format!("env-g1-{condition}-run-{run}")); + assert_eq!( + fs::read_to_string(env.join("src/main.rs")).unwrap(), + "fn main() {} +", + "{condition} run {run}: the codebase's files must be present" + ); + assert!( + git(&env, &["rev-list", "--count", "HEAD"]) + .parse::() + .unwrap() + >= 2, + "{condition} run {run}: the clone must carry the history" + ); + assert_eq!( + git(&env, &["remote"]), + "", + "{condition} run {run}: no env may retain a remote, the cache included" + ); + assert_eq!( + git(&env, &["rev-parse", "refs/eval-magic/baseline"]), + git(&env, &["rev-parse", "HEAD"]), + "{condition} run {run}: the baseline still names the start state" + ); + assert!( + link_count(&env.join(object_relative)) >= 2, + "{condition} run {run}: the object store must be hard-linked to the cache's, not copied byte by byte" + ); + } + } +} + +/// A codebase that carries no Git history — a plain directory — still yields +/// a working environment end to end. Its cache is a commitless repository, +/// which a local clone could not populate (cloning an empty repository checks +/// out nothing), so provisioning takes the plain-copy fallback. +#[test] +fn a_historyless_codebase_provisions_every_env_through_the_copy_fallback() { + let tmp = tempfile::TempDir::new().unwrap(); + let plain = tmp.path().join("legacy-service"); + fs::create_dir_all(plain.join("src")).unwrap(); + fs::write( + plain.join("src/main.rs"), + "fn main() {} +", + ) + .unwrap(); + let source = format!(r#"{{ "path": "{}" }}"#, wire_path(&plain)); + let (skill_dir, cwd) = setup(tmp.path(), &evals_with_codebase(&source)); + fs::write( + skill_dir.join("mr-review/evals/TASK.md"), + "task +", + ) + .unwrap(); + + skill_eval() + .current_dir(&cwd) + .args(["run", "--skill-dir"]) + .arg(&skill_dir) + .args(["--skill", "mr-review", "--mode", "new-skill", "--dry-run"]) + .assert() + .success(); + + for condition in ["with_skill", "without_skill"] { + let env = cli_env_dir(&cwd, "g1", condition); + assert_eq!( + fs::read_to_string(env.join("src/main.rs")).unwrap(), + "fn main() {} +", + "{condition}: a historyless codebase must still populate the working tree" + ); + assert_eq!( + git(&env, &["remote"]), + "", + "{condition}: no env may retain a remote" + ); + assert_eq!( + git(&env, &["rev-parse", "refs/eval-magic/baseline"]), + git(&env, &["rev-parse", "HEAD"]), + "{condition}: the baseline still names the start state" + ); + } +} + +/// The run plan names each codebase and the commit it resolved to, and says the +/// iteration materializes it once — the fact #254 turned into a guarantee. +#[test] +fn the_run_plan_names_the_codebase_and_its_resolved_commit() { + let tmp = tempfile::TempDir::new().unwrap(); + let origin = codebase_repo(tmp.path(), "origin", "main"); + let revision = git(&origin, &["rev-parse", "HEAD"]); + let source = format!(r#"{{ "url": "{}", "ref": "main" }}"#, wire_path(&origin)); + let (skill_dir, cwd) = setup(tmp.path(), &evals_with_codebase(&source)); + fs::write( + skill_dir.join("mr-review/evals/TASK.md"), + "task +", + ) + .unwrap(); + + skill_eval() + .current_dir(&cwd) + .args(["run", "--skill-dir"]) + .arg(&skill_dir) + .args(["--skill", "mr-review", "--mode", "new-skill", "--dry-run"]) + .assert() + .success() + .stdout(predicates::str::contains("codebase: ")) + .stdout(predicates::str::contains(&revision[..7])) + .stdout(predicates::str::contains("materialized once")); +} + +/// Mode B parity: a revision-mode run provisions both arms of the comparison +/// from the same cached codebase, so a skill edit is measured against the same +/// tree the previous skill ran on. +#[test] +fn revision_mode_provisions_both_arms_from_the_cached_codebase() { + let tmp = tempfile::TempDir::new().unwrap(); + let origin = codebase_repo(tmp.path(), "origin", "main"); + let source = format!(r#"{{ "url": "{}", "ref": "main" }}"#, wire_path(&origin)); + let (skill_dir, cwd) = setup(tmp.path(), &evals_with_codebase(&source)); + fs::write( + skill_dir.join("mr-review/evals/TASK.md"), + "task +", + ) + .unwrap(); + + skill_eval() + .current_dir(&cwd) + .args(["snapshot", "--skill-dir"]) + .arg(&skill_dir) + .args(["--skill", "mr-review", "--label", "baseline"]) + .assert() + .success(); + + skill_eval() + .current_dir(&cwd) + .args(["run", "--skill-dir"]) + .arg(&skill_dir) + .args(["--skill", "mr-review", "--mode", "revision", "--dry-run"]) + .assert() + .success(); + + let iteration = iteration_dir(&cwd); + let cached: Vec<_> = fs::read_dir(iteration.join(".codebase")).unwrap().collect(); + assert_eq!( + cached.len(), + 1, + "both arms of the comparison share one cached materialization" + ); + + for condition in ["old_skill", "new_skill"] { + let env = iteration.join(format!("env-g1-{condition}")); + assert_eq!( + fs::read_to_string(env.join("src/main.rs")).unwrap(), + "fn main() {} +", + "{condition}: the codebase's files must be present" + ); + assert!( + git(&env, &["rev-list", "--count", "HEAD"]) + .parse::() + .unwrap() + >= 2, + "{condition}: the history must survive provisioning" + ); + assert_eq!( + git(&env, &["remote"]), + "", + "{condition}: no env may retain a remote" + ); + assert_eq!( + git(&env, &["rev-parse", "refs/eval-magic/baseline"]), + git(&env, &["rev-parse", "HEAD"]), + "{condition}: the baseline still names the start state" + ); + } +}