diff --git a/docs/developer_overview.md b/docs/developer_overview.md index efe3adc..58fccc5 100644 --- a/docs/developer_overview.md +++ b/docs/developer_overview.md @@ -43,6 +43,9 @@ preconditions, handoffs, and recovery commands. few named capabilities that require harness-specific code. - `src/sandbox/`, `src/workspace/`, and `src/validation/` own task isolation, filesystem/workspace mechanics, and configuration checks. +- `src/source/` resolves a declared source — a git URL and ref, or a local directory — to a commit, + and materializes it as a tree. It knows nothing about what is being sourced, so both the codebase + a task environment is built from and the skills under test resolve through it. - `schema/` contains the JSON schemas for user input and generated artifacts. - `harnesses/` contains built-in descriptors, descriptor scaffolding, and embedded harness assets. - `profiles/` contains shared prompt profiles. @@ -143,3 +146,5 @@ implementation evidence in an internal note. `eval-magic docs byoh`. - [Shipped isolation guide](guides/isolation.md) is the repository source for `eval-magic docs isolation`. +- [Shipped codebase guide](guides/codebase.md) is the repository source for + `eval-magic docs codebase`. diff --git a/docs/guides/codebase.md b/docs/guides/codebase.md new file mode 100644 index 0000000..5245ddf --- /dev/null +++ b/docs/guides/codebase.md @@ -0,0 +1,123 @@ +# Sourcing a codebase into a task environment + +An eval's environment can be a real project rather than a handful of fixture files. Declare a +`codebase` in `evals.json` and every `(eval, condition, run)` environment is built from a checkout +of it — with history, on a branch, ready for the agent under test to work in. + +This matters for anything you cannot judge from a toy problem. Whether a skill makes an agent's +code *better* is not answerable when the task is small enough that any model succeeds. + +## Declare one + +A git repository, with an explicit ref: + +```json +{ + "skill_name": "working-with-tdd", + "codebase": { "url": "https://github.com/slowdini/example-project", "ref": "v1.4.0" }, + "evals": [ + { "id": "add-a-feature", "prompt": "...", "expected_output": "..." } + ] +} +``` + +Or a directory on this machine: + +```json +{ "codebase": { "path": "../../fixtures/legacy-service" } } +``` + +A relative `path` resolves against the directory holding `evals.json`, so a committed config means +the same thing in every clone of the skill. Unlike `files_root`, it may be absolute or point +outside the skill tree — that is the point of it. + +The config-level `codebase` is a default. Any eval can override it: + +```json +{ + "codebase": { "url": "https://github.com/slowdini/example-project", "ref": "main" }, + "evals": [ + { "id": "small-fix", "prompt": "...", "expected_output": "..." }, + { "id": "big-refactor", "prompt": "...", "expected_output": "...", + "codebase": { "path": "/srv/projects/monolith" } } + ] +} +``` + +## `ref` is required + +A git source must name a branch, tag, or full commit SHA. The runner resolves it and records the +commit, so a report says which tree it measured. An eval tracking whatever `main` happened to be +could not be re-run against the state it reported on, which is the point of recording provenance at +all. + +Resolution happens before any environment is created. An unreachable repository or a ref that does +not exist fails the run while it has still built nothing. + +## What the environment contains + +Each dispatch gets its own private environment holding: + +- the codebase, checked out at the resolved commit, with its history intact +- no remotes — nothing in the environment can reach or push to the source it came from +- hooks disabled, and a fixed committer identity for the runner's own commit +- the branch the codebase itself was on: the branch a `ref` names, or the repository's default + branch when the ref is a tag or a SHA +- `refs/eval-magic/baseline`, marking the state the agent started from + +An eval that declares no `codebase` still gets a Git repository, initialized on `work`, exactly as +it always has. + +## `files` is an overlay + +`files` and `files_root` still work, and are applied *on top* of the codebase at their declared +paths. Seeding a task-specific file into a real project is the common case: + +```json +{ + "id": "add-a-feature", + "prompt": "Implement what docs/TASK.md describes.", + "expected_output": "the feature, with tests", + "files": ["docs/TASK.md"] +} +``` + +A fixture overwrites a codebase file of the same path. + +The baseline the runner commits respects the codebase's `.gitignore`, so ignored build output stays +out of it. Fixtures and staged skills are committed regardless of what the codebase ignores. + +## A `path` source is not reproducible elsewhere + +Someone reading your published results cannot resolve `../../fixtures/legacy-service`. Their machine +has that directory somewhere else, or not at all. Nothing can fix that, so the artifacts label it: +the record carries `host_local: true`, the run prints a warning, and the `BASELINE.md` row says so. + +Where the directory is itself a Git repository, its `origin` URL and the resolved commit are +recorded too, and *those* resolve anywhere. Prefer a `url` source for anything you intend to +publish. + +A `path` source is materialized as a clean checkout of its committed state. Uncommitted work in the +source directory is not carried into the environment; the run warns when the source is dirty. + +## Verify the result + +From a prepared iteration directory, inspect one environment: + +```sh +cd env-g1-with_skill +git log --oneline | head +git remote -v +git rev-parse refs/eval-magic/baseline HEAD +git status --porcelain +``` + +`git remote -v` and `git status --porcelain` are both empty, and the two revisions match: the +baseline ref names exactly what the agent started from. + +The resolved commit appears in `conditions.json`, each `run.json`, `benchmark.json`, and the +`BASELINE.md` written by `promote-baseline`: + +```sh +jq '.codebases' conditions.json +``` diff --git a/docs/guides/isolation.md b/docs/guides/isolation.md index 1299dbb..7131b21 100644 --- a/docs/guides/isolation.md +++ b/docs/guides/isolation.md @@ -134,6 +134,17 @@ harnesses by checking every rendered eval-agent command in `RUNBOOK.md` and dispatch's setting-source selection. A plugin can appear there and remain absent from the dispatch, or the reverse. Use the dispatch's init event. +## The task repository is a separate boundary + +Skill-source isolation is about what a dispatch can *load*. The task repository is about what it can +*reach*: every dispatch runs in its own private environment, a Git repository with no remotes and +hooks disabled, marked with `refs/eval-magic/baseline` at the state the agent started from. That +holds whether the environment was built from fixture files or from a sourced codebase — see +`eval-magic docs codebase`. + +The two are independent. An environment can be a faithfully isolated repository while the dispatch +still loads a live skill source, and a shadowed skill is not made safe by the repository boundary. + ## When a source cannot be isolated Do not declare isolation. Retain the validity warning as the record of a known threat. A symmetric diff --git a/schema/benchmark.schema.json b/schema/benchmark.schema.json index 7e49e82..fbe4464 100644 --- a/schema/benchmark.schema.json +++ b/schema/benchmark.schema.json @@ -48,6 +48,11 @@ } } }, + "codebases": { + "type": "array", + "description": "Codebases the compared conditions ran against, echoed from conditions.json. Absent for fixture-only iterations.", + "items": { "$ref": "#/definitions/codebaseUse" } + }, "diff_scope": { "type": "object", "description": "Raw final-environment diff metrics per condition, ordered by eval id and then run index. Omitted for iterations created before diff-scope capture.", @@ -69,6 +74,42 @@ } }, "definitions": { + "codebaseUse": { + "type": "object", + "required": ["kind", "source", "branch", "evals"], + "additionalProperties": false, + "properties": { + "kind": { + "type": "string", + "enum": ["git", "path"], + "description": "Whether the codebase came from a repository URL or a directory on the host that ran it." + }, + "source": { "type": "string", "description": "The url or path exactly as declared in evals.json." }, + "resolved_path": { + "type": "string", + "description": "Absolute directory a path source resolved to on the host that ran it." + }, + "ref": { "type": "string", "description": "Declared branch, tag, or commit SHA, for a git source." }, + "revision": { + "type": "string", + "description": "The commit the run actually ran against. A declared ref does not identify this on its own, because a branch moves. Absent only for a directory carrying no history." + }, + "origin_url": { + "type": "string", + "description": "The source repository's origin. For a host-local path this is the only handle another reader can resolve: origin_url plus revision names the same tree anywhere." + }, + "branch": { "type": "string", "description": "Branch the task environment was checked out on." }, + "host_local": { + "type": "boolean", + "description": "True when the source cannot be resolved off the host that ran it, so a published claim citing it is not reproducible from the eval config alone." + }, + "evals": { + "type": "array", + "items": { "type": "string" }, + "description": "Ids of the evals whose environments were built from this codebase." + } + } + }, "assertionCount": { "type": "object", "required": ["passed", "n"], diff --git a/schema/evals.schema.json b/schema/evals.schema.json index 6177033..d118cb5 100644 --- a/schema/evals.schema.json +++ b/schema/evals.schema.json @@ -11,6 +11,10 @@ "type": "string", "description": "Name of the skill being evaluated. Should match the skill directory name." }, + "codebase": { + "$ref": "#/definitions/codebase", + "description": "Default codebase every eval's task environment is built from. A per-eval codebase overrides it." + }, "evals": { "type": "array", "minItems": 1, @@ -18,6 +22,41 @@ } }, "definitions": { + "codebase": { + "oneOf": [ + { "$ref": "#/definitions/gitCodebase" }, + { "$ref": "#/definitions/pathCodebase" } + ] + }, + "pathCodebase": { + "type": "object", + "required": ["path"], + "additionalProperties": false, + "properties": { + "path": { + "type": "string", + "minLength": 1, + "description": "Directory on this host to build the task environment from, resolved relative to this evals.json when relative. Unlike files_root it may be absolute or escape the skill tree, because it deliberately points outside it. A path source is host-local: another machine has the directory elsewhere or not at all, so a run recorded against one is not reproducible from this config alone. When the directory is a Git repository the runner also records its origin URL and resolved SHA, which are." + } + } + }, + "gitCodebase": { + "type": "object", + "required": ["url", "ref"], + "additionalProperties": false, + "properties": { + "url": { + "type": "string", + "minLength": 1, + "description": "Git repository to clone the task environment from." + }, + "ref": { + "type": "string", + "minLength": 1, + "description": "Branch, tag, or full commit SHA to check out. Required: the runner records the resolved SHA, so an eval tracking a moving branch could not be re-run against what it measured." + } + } + }, "eval": { "type": "object", "required": ["id", "prompt", "expected_output"], @@ -59,6 +98,10 @@ "minimum": 1, "description": "Runs per condition for this eval, for variance reduction; overrides the --runs flag. Defaults to the flag's value (1 unless raised)." }, + "codebase": { + "$ref": "#/definitions/codebase", + "description": "Codebase this eval's task environment is built from, overriding the config-level default." + }, "isolation": { "type": "string", "enum": ["shared", "isolated"], diff --git a/schema/run-record.schema.json b/schema/run-record.schema.json index 1bd6949..4acc7b4 100644 --- a/schema/run-record.schema.json +++ b/schema/run-record.schema.json @@ -84,6 +84,10 @@ "conversation": { "$ref": "#/definitions/conversation", "description": "Ordered multi-turn evidence and scripted-delivery outcome. Absent for one-shot runs." + }, + "codebase": { + "$ref": "#/definitions/codebase", + "description": "The codebase this run's environment was built from. Absent for a fixture-only run." } }, "definitions": { @@ -168,6 +172,37 @@ "text": { "type": "string" } } }, + "codebase": { + "type": "object", + "required": ["kind", "source", "branch"], + "additionalProperties": false, + "properties": { + "kind": { + "type": "string", + "enum": ["git", "path"], + "description": "Whether the codebase came from a repository URL or a directory on the host that ran it." + }, + "source": { "type": "string", "description": "The url or path exactly as declared in evals.json." }, + "resolved_path": { + "type": "string", + "description": "Absolute directory a path source resolved to on the host that ran it." + }, + "ref": { "type": "string", "description": "Declared branch, tag, or commit SHA, for a git source." }, + "revision": { + "type": "string", + "description": "The commit the run actually ran against. A declared ref does not identify this on its own, because a branch moves. Absent only for a directory carrying no history." + }, + "origin_url": { + "type": "string", + "description": "The source repository's origin. For a host-local path this is the only handle another reader can resolve: origin_url plus revision names the same tree anywhere." + }, + "branch": { "type": "string", "description": "Branch the task environment was checked out on." }, + "host_local": { + "type": "boolean", + "description": "True when the source cannot be resolved off the host that ran it, so a published claim citing it is not reproducible from the eval config alone." + } + } + }, "conversationTool": { "type": "object", "required": ["type", "ordinal", "round", "name"], diff --git a/src/cli/help.rs b/src/cli/help.rs index 69afc86..f3ca59e 100644 --- a/src/cli/help.rs +++ b/src/cli/help.rs @@ -30,6 +30,11 @@ EXAMPLES: # Reduce cost while iterating on the suite eval-magic run --only case-a,case-b + # Run the task against a real project instead of fixture files. The codebase + # is declared in evals.json, not on the command line, so it stays a reviewed + # property of the eval set + eval-magic docs codebase + # Select a built-in harness; `run --help` documents models and environment options eval-magic run --harness codex diff --git a/src/cli/run/dispatch.rs b/src/cli/run/dispatch.rs index a13fe10..c11bbef 100644 --- a/src/cli/run/dispatch.rs +++ b/src/cli/run/dispatch.rs @@ -14,7 +14,9 @@ use serde::{Deserialize, Serialize}; use crate::adapters::{CliManifestContext, adapter_for}; use crate::core::fs::artifact_path; -use crate::core::{AvailableSkill, Eval, Harness, POSIX_TOOLING_REQUIREMENT, ScriptedTurn}; +use crate::core::{ + AvailableSkill, CodebaseRecord, Eval, Harness, POSIX_TOOLING_REQUIREMENT, ScriptedTurn, +}; use super::RunError; @@ -53,6 +55,10 @@ pub struct DispatchTask { /// recipe's `` placeholder resolves to. #[serde(default, skip_serializing_if = "Option::is_none")] pub eval_root: Option, + /// The codebase this task's environment was built from. Carried here so the + /// run record written at ingest names the tree the agent actually worked in. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub codebase: Option, #[serde(default, skip_serializing)] pub dispatch_prompt: String, } @@ -90,6 +96,8 @@ pub struct DispatchTaskOpts<'a> { /// The task's env dir (the agent-under-test's cwd); `None` only for legacy /// callers that do not carry an environment manifest. pub eval_root: Option<&'a str>, + /// The codebase this task's environment was built from, if any. + pub codebase: Option<&'a CodebaseRecord>, } fn render_available_skills_block_for_harness( @@ -274,6 +282,7 @@ pub fn build_dispatch_task(opts: &DispatchTaskOpts) -> Result Result<(), RunError> { ))) } -pub(super) fn initialize_task_repositories(resolved: &Resolved) -> Result<(), RunError> { +pub(super) fn initialize_task_repositories( + ctx: &RunContext, + resolved: &Resolved, +) -> Result<(), RunError> { let targets = env_targets(&EnvLayoutInput { iteration_dir: &resolved.iteration_dir, groups: &resolved.groups, @@ -48,7 +55,19 @@ pub(super) fn initialize_task_repositories(resolved: &Resolved) -> Result<(), Ru skill_path_b: resolved.skill_path_b.as_deref(), }); for target in targets { - initialize_task_repository(&target.root).map_err(|error| { + let codebase = resolved.codebase_for(&target.eval_ids)?; + let plan = TaskRepository { + root: target.root.clone(), + // A sourced environment already *is* a repository, carrying the + // history the clone brought with it. + sourced: codebase.is_some(), + branch: codebase.map_or_else( + || INITIALIZED_BRANCH.to_string(), + |codebase| codebase.source.branch.clone(), + ), + forced_paths: runner_placed_paths(ctx, resolved, &target)?, + }; + initialize_task_repository(&plan).map_err(|error| { let hint = path_budget_hint(&target.root, cfg!(windows)) .map(|hint| format!("\n{hint}")) .unwrap_or_default(); @@ -61,6 +80,51 @@ pub(super) fn initialize_task_repositories(resolved: &Resolved) -> Result<(), Ru Ok(()) } +/// Env-relative paths the runner placed, which must reach the baseline commit +/// even when the sourced codebase's own `.gitignore` covers them. +/// +/// A real repository ignores its build output, and a blanket forced add would +/// sweep `target/` or `node_modules/` into the baseline. So the baseline add +/// respects `.gitignore` and these paths — the harness config directories, and +/// the declared fixture overlay — are forced on top of it. +fn runner_placed_paths( + ctx: &RunContext, + resolved: &Resolved, + target: &super::envs::EnvTarget, +) -> Result, RunError> { + let mut paths: Vec = all_config_dir_names() + .into_iter() + .filter(|name| target.root.join(name).exists()) + .collect(); + for eval_id in &target.eval_ids { + let Some(eval) = resolved + .selected_evals + .iter() + .find(|candidate| &candidate.id == eval_id) + else { + continue; + }; + for (dest, _source) in fixture_pairs(eval, &ctx.skill_subdir)? { + if target.root.join(&dest).exists() { + paths.push(dest); + } + } + } + paths.sort(); + paths.dedup(); + Ok(paths) +} + +/// One task repository to establish. +struct TaskRepository { + root: PathBuf, + /// Whether a codebase already put a repository here. A sourced environment + /// keeps its `.git`; a fixture-only one is initialized from nothing. + sourced: bool, + branch: String, + forced_paths: Vec, +} + /// A sentence naming the Windows path budget, for a task root too deep to hold /// what a run stages below it. /// @@ -79,8 +143,8 @@ fn path_budget_hint(root: &Path, windows: bool) -> Option { )) } -fn initialize_task_repository(root: &Path) -> Result<(), String> { - remove_existing_git_dir(root)?; +fn initialize_task_repository(plan: &TaskRepository) -> Result<(), String> { + let root = plan.root.as_path(); let isolated = tempfile::TempDir::new() .map_err(|error| format!("could not create isolated Git configuration: {error}"))?; @@ -91,20 +155,27 @@ fn initialize_task_repository(root: &Path) -> Result<(), String> { fs::write(&global_config, "") .map_err(|error| format!("could not create empty Git configuration: {error}"))?; - run_checked( - root, - &[ - OsString::from("init"), - OsString::from("--quiet"), - OsString::from("--initial-branch"), - OsString::from(BASELINE_BRANCH), - OsString::from("--template"), - template_dir.into_os_string(), - OsString::from("."), - ], - &global_config, - &[], - )?; + if plan.sourced { + // The clone's history is the point of sourcing a codebase, so this is + // the one case that must not reset `.git`. + strip_remotes(root, &global_config)?; + } else { + remove_existing_git_dir(root)?; + run_checked( + root, + &[ + OsString::from("init"), + OsString::from("--quiet"), + OsString::from("--initial-branch"), + OsString::from(&plan.branch), + OsString::from("--template"), + template_dir.into_os_string(), + OsString::from("."), + ], + &global_config, + &[], + )?; + } let hooks_dir = root.join(".git/eval-magic-disabled-hooks"); fs::create_dir_all(root.join(".git/info")) @@ -140,20 +211,36 @@ fn initialize_task_repository(root: &Path) -> Result<(), String> { )?; } + // Respects the sourced codebase's `.gitignore`: a real repository ignores + // its build output, and a forced add here would commit `target/` or + // `node_modules/` into the baseline every environment starts from. + // + // No exclude pathspec for `.eval-magic-outputs`: `.git/info/exclude` above + // already ignores it, and an unforced add honors that. The pathspecs this + // replaces existed only to carve it back out of a forced add. run_checked( root, &[ OsString::from("add"), - OsString::from("--force"), OsString::from("--all"), OsString::from("--"), OsString::from("."), - OsString::from(":(exclude,top).eval-magic-outputs"), - OsString::from(":(exclude,top).eval-magic-outputs/**"), ], &global_config, &[], )?; + // What the runner itself placed is forced in on top, so a codebase that + // ignores `.claude/` cannot hide the staged skill from the baseline — which + // would leave the condition under test outside every later diff. + if !plan.forced_paths.is_empty() { + let mut args = vec![ + OsString::from("add"), + OsString::from("--force"), + OsString::from("--"), + ]; + args.extend(plan.forced_paths.iter().map(OsString::from)); + run_checked(root, &args, &global_config, &[])?; + } run_checked( root, &[ @@ -176,9 +263,49 @@ fn initialize_task_repository(root: &Path) -> Result<(), String> { ], )?; + // The start state, named. Everything the agent does afterwards is measurable + // as the difference from this ref, whether the environment has one commit or + // a codebase's entire history behind it. + // + // Deliberately outside `refs/heads/`: it never appears in `git branch`, so + // it adds nothing to what the agent under test sees. + run_checked( + root, + &[ + OsString::from("update-ref"), + OsString::from(BASELINE_REF), + OsString::from("HEAD"), + ], + &global_config, + &[], + )?; + verify_task_repository(root, &global_config) } +/// Drop every remote, so nothing in the environment can reach the source it was +/// cloned from — or push to it. +fn strip_remotes(root: &Path, global_config: &Path) -> Result<(), String> { + let listed = run_checked(root, &[OsString::from("remote")], global_config, &[])?; + for remote in String::from_utf8_lossy(&listed.stdout) + .lines() + .map(str::trim) + .filter(|name| !name.is_empty()) + { + run_checked( + root, + &[ + OsString::from("remote"), + OsString::from("remove"), + OsString::from(remote), + ], + global_config, + &[], + )?; + } + Ok(()) +} + fn remove_existing_git_dir(root: &Path) -> Result<(), String> { let git_dir = root.join(".git"); let metadata = match fs::symlink_metadata(&git_dir) { @@ -313,6 +440,17 @@ mod tests { use crate::core::runtime::report_skip; + /// A repository with no codebase behind it — the shape these path-budget + /// tests exercise, and what a fixture-only run has always produced. + fn fixture_only(root: &Path) -> TaskRepository { + TaskRepository { + root: root.to_path_buf(), + sourced: false, + branch: INITIALIZED_BRANCH.to_string(), + forced_paths: Vec::new(), + } + } + /// A staged skill's path relative to its task root: 68 characters, the /// shortest realistic shape of `.claude/skills//SKILL.md`. const STAGED_SKILL: &str = @@ -388,7 +526,7 @@ mod tests { root.join(STAGED_SKILL).as_os_str().len() > WINDOWS_USABLE_PATH, "the fixture must exceed the Windows path budget to exercise anything" ); - initialize_task_repository(&root) + initialize_task_repository(&fixture_only(&root)) .expect("a task root with a deep staged skill initializes"); } @@ -427,7 +565,8 @@ mod tests { let Some(root) = deep_task_root(tmp.path(), 202, test) else { return; }; - initialize_task_repository(&root).expect("a task root in the quiet band initializes"); + initialize_task_repository(&fixture_only(&root)) + .expect("a task root in the quiet band initializes"); let tracked = run_git(&["ls-files"], &root); assert!( String::from_utf8_lossy(&tracked.stdout).contains("SKILL.md"), @@ -467,7 +606,7 @@ mod tests { length + 1 + GIT_CONFIG.len() + 3 <= WINDOWS_USABLE_PATH, "{length}-character root leaves `.git/config` no margin below the budget" ); - initialize_task_repository(&root) + initialize_task_repository(&fixture_only(&root)) .expect("a task root deeper than `.git` needs initializes"); } } diff --git a/src/cli/run/orchestrate/mod.rs b/src/cli/run/orchestrate/mod.rs index f7fa2ce..d484f89 100644 --- a/src/cli/run/orchestrate/mod.rs +++ b/src/cli/run/orchestrate/mod.rs @@ -12,11 +12,15 @@ //! stateless helpers in [`super::util`]. use std::collections::{BTreeMap, BTreeSet}; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use crate::adapters::{CliDispatchContext, adapter_for}; use crate::cli::command_target_args; -use crate::core::{Eval, Mode, RunContext}; +use crate::core::fs::artifact_path; +use crate::core::{ + CodebaseKind, CodebaseRecord, CodebaseSource, CodebaseUse, Eval, Mode, RunContext, +}; +use crate::source::ResolvedSource; use super::RunError; use super::statistics::format_minimum_attainable_fisher_p_value; @@ -72,6 +76,9 @@ impl RunOptions<'_> { struct Resolved { mode: Mode, baseline: Option, + /// Distinct codebases the selection declares, already resolved to a commit. + /// Empty for a fixture-only run, which is what keeps that path unchanged. + codebases: Vec, skill_md_path: PathBuf, iteration: u32, iteration_dir: PathBuf, @@ -88,6 +95,77 @@ struct Resolved { groups: Vec, } +/// One resolved codebase and the evals built from it. +struct RunCodebase { + /// The declaration as written, which is what deduplication compares. + declared: CodebaseSource, + source: ResolvedSource, + /// Directory name under `iteration-N/.codebase/` this materializes into. + key: String, + eval_ids: Vec, +} + +impl RunCodebase { + /// The artifact form, shared by every provenance surface so a reader never + /// has to reconcile two spellings of the same resolution. + fn record(&self) -> CodebaseRecord { + CodebaseRecord { + kind: match self.declared { + CodebaseSource::Git { .. } => CodebaseKind::Git, + CodebaseSource::Path { .. } => CodebaseKind::Path, + }, + source: self.source.source.clone(), + resolved_path: self + .source + .resolved_path + .as_deref() + .map(|path| artifact_path(Path::new(path))), + reference: self.source.reference.clone(), + revision: self.source.revision.clone(), + origin_url: self.source.origin_url.clone(), + branch: self.source.branch.clone(), + host_local: self.source.host_local, + } + } + + fn usage(&self) -> CodebaseUse { + CodebaseUse { + codebase: self.record(), + evals: self.eval_ids.clone(), + } + } +} + +impl Resolved { + /// The codebase backing an environment, given the evals sharing it. + /// + /// Production always task-scopes, so an environment carries exactly one + /// eval and the question is trivial. The error covers the planner's older + /// multi-eval grouping, where two evals with different codebases could not + /// share one working tree even in principle. + fn codebase_for(&self, eval_ids: &[String]) -> Result, RunError> { + let mut found: Option<&RunCodebase> = None; + for eval_id in eval_ids { + let codebase = self + .codebases + .iter() + .find(|candidate| candidate.eval_ids.contains(eval_id)); + match (found, codebase) { + (None, next) => found = next, + (Some(previous), Some(next)) if !std::ptr::eq(previous, next) => { + return Err(RunError::msg(format!( + "evals {} share an environment but declare different codebases; \ + give them distinct environments", + eval_ids.join(", ") + ))); + } + _ => {} + } + } + Ok(found) + } +} + /// The product of [`stage::stage_conditions`]: the staged slugs plus the /// dispatch-prompt inputs shared across every task. struct Staged { diff --git a/src/cli/run/orchestrate/resolve.rs b/src/cli/run/orchestrate/resolve.rs index 178ba62..1bdbc96 100644 --- a/src/cli/run/orchestrate/resolve.rs +++ b/src/cli/run/orchestrate/resolve.rs @@ -6,7 +6,8 @@ use std::fs; use serde_json::Value; use crate::cli::command_target_args; -use crate::core::{Assertion, Mode, RunContext}; +use crate::core::{Assertion, CodebaseSource, Eval, EvalsConfig, Mode, RunContext}; +use crate::source::{SourceSpec, resolve as resolve_source}; use crate::validation::validate_evals_config; use super::super::RunError; @@ -14,7 +15,62 @@ use super::super::dispatch::select_evals; use super::super::fixtures::{fixture_pairs, setup_file_pairs}; use super::super::grouping::{GroupInput, compute_groups}; use super::super::util::{condition_names_for, make_run_nonce, next_iteration}; -use super::{Resolved, RunOptions}; +use super::{Resolved, RunCodebase, RunOptions}; + +/// Resolve every distinct codebase the selected evals declare, deduplicated so +/// a config-level default shared by ten evals is one resolution and, later, one +/// materialization. +/// +/// The `CodebaseSource` → `SourceSpec` translation lives here rather than as a +/// `From` impl in [`crate::source`]: that module resolves skills for #253 too, +/// and stays useful precisely because it does not know what a codebase is. +fn resolve_codebases( + ctx: &RunContext, + config: &EvalsConfig, + selected: &[Eval], +) -> Result, RunError> { + // A declared relative path is relative to the config that declares it, so a + // committed `evals.json` means the same thing in every clone of the skill. + let base_dir = ctx.skill_subdir.join("evals"); + let mut codebases: Vec = Vec::new(); + + for eval in selected { + let Some(declared) = eval.codebase.as_ref().or(config.codebase.as_ref()) else { + continue; + }; + if let Some(existing) = codebases + .iter_mut() + .find(|candidate| &candidate.declared == declared) + { + existing.eval_ids.push(eval.id.clone()); + continue; + } + + let spec = match declared { + CodebaseSource::Git { url, reference } => SourceSpec::Git { + url: url.clone(), + reference: reference.clone(), + }, + CodebaseSource::Path { path } => SourceSpec::Path { path: path.clone() }, + }; + let source = resolve_source(&spec, &base_dir) + .map_err(|error| RunError::msg(format!("eval '{}': {error}", eval.id)))?; + // Keyed on the resolved commit so two evals naming the same tree by + // different refs still materialize once. A directory with no history has + // no commit to key on and falls back to declaration order. + let key = source + .revision + .clone() + .unwrap_or_else(|| format!("local-{}", codebases.len() + 1)); + codebases.push(RunCodebase { + declared: declared.clone(), + source, + key, + eval_ids: vec![eval.id.clone()], + }); + } + Ok(codebases) +} pub(super) fn resolve_request(ctx: &RunContext, opts: &RunOptions) -> Result { let mode = match opts.mode { @@ -57,6 +113,14 @@ pub(super) fn resolve_request(ctx: &RunContext, opts: &RunOptions) -> Result Result = HashMap::new(); for target in &targets { // Disarm a prior run's guard before re-staging, so a crashed run can't leave // the write-blocking hook armed across runs. Created unconditionally — even // under --no-stage, each env's fixtures still land here. teardown_guard(&target.root); + + let codebase = r.codebase_for(&target.eval_ids)?; + if codebase.is_some() && target.root.exists() { + // An explicit `--iteration N` rebuild would otherwise lay a fresh + // codebase over the last run's tree, including whatever the previous + // 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. + if let Some(codebase) = codebase { + let source_tree = materialize_codebase(&r.iteration_dir, codebase, &mut materialized)?; + copy_entry_materialized(&source_tree, &target.root)?; + } + if !opts.no_stage { cleanup_staged_skills(&target.root, ctx.harness)?; if ctx.stage_siblings { @@ -160,6 +180,29 @@ pub(super) fn stage_conditions( }) } +/// 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. +fn materialize_codebase( + iteration_dir: &Path, + codebase: &super::RunCodebase, + materialized: &mut HashMap, +) -> Result { + if let Some(existing) = materialized.get(&codebase.key) { + return Ok(existing.clone()); + } + let tree = iteration_dir.join(".codebase").join(&codebase.key); + if tree.exists() { + fs::remove_dir_all(&tree)?; + } + crate::source::materialize(&codebase.source, &tree) + .map_err(|error| RunError::msg(error.to_string()))?; + materialized.insert(codebase.key.clone(), tree.clone()); + Ok(tree) +} + /// Stage one condition's skill into `root` and return its slug; `Ok(None)` when /// the condition stages no skill (the new-skill control arm) or under --no-stage. fn stage_for( diff --git a/src/cli/run/util.rs b/src/cli/run/util.rs index 9f52c4b..89f4a95 100644 --- a/src/cli/run/util.rs +++ b/src/cli/run/util.rs @@ -475,6 +475,7 @@ mod tests { runs: None, isolation: None, turns: None, + codebase: None, } } diff --git a/src/core/types.rs b/src/core/types.rs index f6c179c..cb652c3 100644 --- a/src/core/types.rs +++ b/src/core/types.rs @@ -116,6 +116,11 @@ pub struct Eval { /// Ordered scripted user follow-ups. Absence preserves one-shot dispatch. #[serde(skip_serializing_if = "Option::is_none")] pub turns: Option>, + /// Codebase this eval's task environment is built from, overriding the + /// config-level default. Appended last so an eval that declares none + /// serializes exactly as it did before the field existed. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub codebase: Option, } /// One scripted user follow-up delivered after an assistant response. @@ -143,10 +148,84 @@ pub enum Isolation { Isolated, } +/// Where a task environment's contents come from: a Git repository at an +/// explicit ref, or a directory on this host. +/// +/// Untagged because the config spells the two apart by their keys (`url`+`ref` +/// versus `path`) rather than by a discriminator. `evals.schema.json` rejects +/// the ambiguous shapes before serde ever sees them, so the poor error messages +/// untagged enums produce on their own never reach a user. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(untagged)] +pub enum CodebaseSource { + Git { + url: String, + /// Required: the runner records the *resolved* SHA, so an eval that + /// tracked a moving branch could not be re-run against what it measured. + #[serde(rename = "ref")] + reference: String, + }, + Path { + path: String, + }, +} + +/// Whether a codebase came from a repository URL or a directory on this host. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum CodebaseKind { + Git, + Path, +} + +/// A resolved codebase, as every provenance artifact records it. +/// +/// The declared ref is not enough to identify what a run measured — a branch +/// moves — so [`Self::revision`] is the field a report is read against. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct CodebaseRecord { + pub kind: CodebaseKind, + /// The url or path exactly as declared, so a reader can find it in the config. + pub source: String, + /// Where a path source resolved to on the host that ran it. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub resolved_path: Option, + #[serde(rename = "ref", default, skip_serializing_if = "Option::is_none")] + pub reference: Option, + /// The commit the run actually ran against. Absent only for a directory + /// that carried no history to name one. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub revision: Option, + /// The source repository's `origin`. For a host-local path this is the only + /// handle another reader can resolve: `origin_url` + `revision` names the + /// same tree anywhere, where `source` names it only here. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub origin_url: Option, + pub branch: String, + /// Set when the source cannot be resolved off the host that ran it, so a + /// published claim citing it is not reproducible from the config alone. + #[serde(default, skip_serializing_if = "std::ops::Not::not")] + pub host_local: bool, +} + +/// One resolved codebase plus the evals built from it. `conditions.json` and +/// `benchmark.json` carry a list of these; a `run.json` carries the bare +/// [`CodebaseRecord`], having exactly one. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct CodebaseUse { + #[serde(flatten)] + pub codebase: CodebaseRecord, + pub evals: Vec, +} + /// The parsed `evals.json` for one skill. #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct EvalsConfig { pub skill_name: String, + /// Default codebase for every eval in this config; a per-eval `codebase` + /// overrides it. Mirrors how `runs` defaults and is overridden. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub codebase: Option, pub evals: Vec, } @@ -217,6 +296,10 @@ pub struct ConditionsRecord { /// Operator-declared provenance label, surfaced in `BASELINE.md` on promote. #[serde(skip_serializing_if = "Option::is_none")] pub label: Option, + /// Codebases the iteration's environments were built from. Empty for a + /// fixture-only iteration, which keeps its `conditions.json` unchanged. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub codebases: Vec, } /// Comparison mode for a run. @@ -261,6 +344,12 @@ pub struct RunRecord { /// legacy one-shot runs. #[serde(skip_serializing_if = "Option::is_none")] pub conversation: Option, + /// The codebase this run's environment was built from. Grading reads + /// `run.json` and nothing else, so a result can only be tied to a tree if + /// the record names one. Appended last, and omitted when absent, so a + /// fixture-only record serializes as it always did. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub codebase: Option, } /// The completed outcome of one scripted conversation. @@ -442,6 +531,7 @@ mod tests { runs: None, isolation: None, turns: None, + codebase: None, }; let out = serde_json::to_value(&eval).unwrap(); assert!(out.get("files").is_none()); @@ -465,6 +555,7 @@ mod tests { runs: None, isolation: Some(Isolation::Isolated), turns: None, + codebase: None, }; let out = serde_json::to_value(&eval).unwrap(); assert_eq!( @@ -489,6 +580,7 @@ mod tests { duration_ms: None, run_index: None, conversation: None, + codebase: None, }; let out = serde_json::to_value(&rec).unwrap(); // Required-but-nullable keys are present with a null value. @@ -557,6 +649,7 @@ mod tests { agent_env: BTreeMap::new(), judge_model: None, label: None, + codebases: Vec::new(), }; let out = serde_json::to_value(&rec).unwrap(); assert_eq!(out.get("mode"), Some(&Value::String("new-skill".into()))); diff --git a/src/lib.rs b/src/lib.rs index 93748b8..fe2eaf0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -11,5 +11,6 @@ pub mod cli; pub mod core; pub mod pipeline; pub mod sandbox; +pub mod source; pub mod validation; pub mod workspace; diff --git a/src/pipeline/aggregate.rs b/src/pipeline/aggregate.rs index fbe06cb..a2eeb4a 100644 --- a/src/pipeline/aggregate.rs +++ b/src/pipeline/aggregate.rs @@ -20,7 +20,7 @@ use serde_json::Value; use self::assertions::AssertionRollup; use crate::adapters::skill_shadow::PluginShadowArtifact; use crate::core::fs::write_json; -use crate::core::{ConditionsRecord, GradingResult, Mode, TimingRecord, TimingSource}; +use crate::core::{CodebaseUse, ConditionsRecord, GradingResult, Mode, TimingRecord, TimingSource}; use crate::pipeline::DiffScopeMetrics; use crate::pipeline::error::PipelineError; use crate::pipeline::git_isolation; @@ -114,6 +114,12 @@ pub struct Benchmark { pub warnings: Vec, pub run_summary: Value, pub assertions: Value, + /// Codebases the compared conditions ran against, echoed from + /// `conditions.json` so a published benchmark names the trees it measured + /// without a reader having to hold two artifacts side by side. Empty for a + /// fixture-only iteration, which keeps its benchmark unchanged. + #[serde(skip_serializing_if = "Vec::is_empty")] + pub codebases: Vec, #[serde(skip_serializing_if = "Option::is_none")] pub diff_scope: Option, delta: Delta, @@ -408,6 +414,7 @@ pub fn aggregate( generated: now_iso8601(), mode: conditions.mode, baseline: conditions.baseline.clone(), + codebases: conditions.codebases.clone(), conditions_compared: vec![a.clone(), b.clone()], missing_gradings, validity_warnings, diff --git a/src/pipeline/record_runs.rs b/src/pipeline/record_runs.rs index 7b6b07f..08de353 100644 --- a/src/pipeline/record_runs.rs +++ b/src/pipeline/record_runs.rs @@ -31,7 +31,8 @@ use serde::Deserialize; use crate::adapters::{PermissionDenial, TranscriptSummary, adapter_for}; use crate::core::fs::write_json; use crate::core::{ - ConversationEvent, ConversationRecord, Harness, RunRecord, TimingRecord, TimingSource, + CodebaseRecord, ConversationEvent, ConversationRecord, Harness, RunRecord, TimingRecord, + TimingSource, }; use crate::pipeline::error::PipelineError; use crate::pipeline::permission_denials::{self, TaskPermissionDenials}; @@ -72,6 +73,10 @@ struct DispatchTask { /// shadow finding names. #[serde(default)] group: Option, + /// The codebase the environment was built from, copied through to the run + /// record so grading can name the tree a result came from. + #[serde(default)] + codebase: Option, } /// Tally of what record-runs did across the dispatch's tasks. @@ -312,6 +317,7 @@ pub fn record_runs( duration_ms: None, run_index: task.run_index, conversation: conversation.clone(), + codebase: task.codebase.clone(), }; validate_against_schema::( SchemaName::RunRecord, diff --git a/src/pipeline/record_runs/tests/assembly.rs b/src/pipeline/record_runs/tests/assembly.rs index 3f5785a..ece109e 100644 --- a/src/pipeline/record_runs/tests/assembly.rs +++ b/src/pipeline/record_runs/tests/assembly.rs @@ -53,6 +53,78 @@ fn assembles_run_and_timing_for_every_task_from_disk() { assert_eq!(timing["source"], json!("transcript")); } +/// Grading reads `run.json` and nothing else, so the record has to name the +/// tree the agent worked in — otherwise a result cannot be tied to a codebase +/// at the only granularity that matters, the individual run. +#[test] +fn carries_the_codebase_from_dispatch_task_into_each_run_record() { + let root = TempDir::new().unwrap(); + let iter = dirs(&root); + let cond_dir = iter.join("eval-crash").join("with_skill"); + let outputs_dir = cond_dir.join("outputs"); + fs::create_dir_all(&outputs_dir).unwrap(); + fs::write(outputs_dir.join("final-message.md"), "Fixed it.").unwrap(); + write_codex_events(&outputs_dir, "unused"); + let codebase = json!({ + "kind": "git", + "source": "https://example.com/project.git", + "ref": "main", + "revision": "a1b2c3d4e5f60718293a4b5c6d7e8f9012345678", + "branch": "main" + }); + fs::write( + iter.join("dispatch.json"), + serde_json::to_string_pretty(&json!({ + "run_nonce": "nonce1", + "tasks": [{ + "eval_id": "crash", + "condition": "with_skill", + "skill_path": "/staged/skill/SKILL.md", + "user_prompt": "Do the crash task", + "fixtures": [], + "outputs_dir": outputs_dir.to_string_lossy(), + "run_record_path": cond_dir.join("run.json").to_string_lossy(), + "timing_path": cond_dir.join("timing.json").to_string_lossy(), + "agent_description": "crash:with_skill:i1-nonce1", + "codebase": codebase, + }] + })) + .unwrap(), + ) + .unwrap(); + + record_runs(&iter, 1, Harness::resolve("codex").unwrap(), false).unwrap(); + + let recorded: Value = + serde_json::from_str(&fs::read_to_string(cond_dir.join("run.json")).unwrap()).unwrap(); + assert_eq!(recorded["codebase"], codebase); +} + +/// A run with no codebase behind it serializes exactly as it did before the +/// field existed, so historical records stay comparable. +#[test] +fn omits_the_codebase_key_when_a_task_declares_none() { + let root = TempDir::new().unwrap(); + let iter = dirs(&root); + let paths = write_iteration( + &iter, + &[FixtureTask { + eval_id: "crash", + condition: "with_skill", + final_message: Some("Fixed it."), + }], + ); + write_claude_events(&paths[0].outputs_dir, "unused"); + + record_runs(&iter, 1, Harness::resolve("claude-code").unwrap(), false).unwrap(); + + let recorded: Value = serde_json::from_str( + &fs::read_to_string(iter.join("eval-crash").join("with_skill").join("run.json")).unwrap(), + ) + .unwrap(); + assert!(recorded.get("codebase").is_none()); +} + #[test] fn carries_run_index_from_dispatch_task_into_each_run_record() { let root = TempDir::new().unwrap(); diff --git a/src/source/git.rs b/src/source/git.rs new file mode 100644 index 0000000..2fc7eab --- /dev/null +++ b/src/source/git.rs @@ -0,0 +1,82 @@ +//! Running git with the operator's configuration held off. +//! +//! Sourcing a codebase runs git against a URL from an eval config, on a host +//! whose git configuration belongs to someone else. Left inherited, that +//! configuration decides things the runner has to decide itself: `insteadOf` +//! rewrites the URL, so the tree sourced is not the tree the report cites; +//! `init.templateDir` installs hooks into a repository the guard assumes has +//! none; `commit.gpgSign` blocks the baseline commit on a passphrase prompt. +//! +//! So every git invocation in this module runs with system and global +//! configuration switched off and the environment-variable configuration +//! mechanism cleared. + +use std::path::{Path, PathBuf}; +use std::process::Command; + +use crate::core::{GitOutput, clear_git_environment}; + +/// A scratch git configuration that resolves to nothing. +/// +/// Holds the `TempDir` alive: dropping it removes the empty global config file +/// and the empty template directory that make the isolation work. +pub(crate) struct IsolatedGit { + _scratch: tempfile::TempDir, + global_config: PathBuf, + template_dir: PathBuf, +} + +impl IsolatedGit { + pub(crate) fn new() -> Result { + let scratch = tempfile::TempDir::new() + .map_err(|error| format!("could not create isolated Git configuration: {error}"))?; + let global_config = scratch.path().join("global-config"); + let template_dir = scratch.path().join("template"); + std::fs::write(&global_config, "") + .map_err(|error| format!("could not create empty Git configuration: {error}"))?; + std::fs::create_dir(&template_dir) + .map_err(|error| format!("could not create empty Git template directory: {error}"))?; + Ok(Self { + _scratch: scratch, + global_config, + template_dir, + }) + } + + /// An empty template directory, for `git init --template`, so a configured + /// `init.templateDir` cannot seed hooks into a task repository. + pub(crate) fn template_dir(&self) -> &Path { + &self.template_dir + } + + pub(crate) fn run(&self, cwd: &Path, args: &[&str]) -> GitOutput { + let mut command = Command::new("git"); + command + // `git clone` and `git init` create paths inside `.git` before any + // repository-local configuration exists, so the Windows long-path + // lift has to ride on the invocation itself. + .args(["-c", "core.longpaths=true"]) + .args(args) + .current_dir(cwd) + .env("GIT_CONFIG_NOSYSTEM", "1") + .env("GIT_CONFIG_GLOBAL", &self.global_config) + // The environment-variable configuration mechanism: git reads + // `GIT_CONFIG_KEY_` / `GIT_CONFIG_VALUE_` only up to the count, + // so clearing the count disables all of them. + .env_remove("GIT_CONFIG_COUNT") + .env_remove("GIT_CONFIG_PARAMETERS"); + clear_git_environment(&mut command); + match command.output() { + Ok(output) => GitOutput { + status: output.status.code(), + stdout: output.stdout, + stderr: output.stderr, + }, + Err(error) => GitOutput { + status: None, + stdout: Vec::new(), + stderr: format!("{error}").into_bytes(), + }, + } + } +} diff --git a/src/source/mod.rs b/src/source/mod.rs new file mode 100644 index 0000000..4417ef1 --- /dev/null +++ b/src/source/mod.rs @@ -0,0 +1,340 @@ +//! Resolving a declared source to a revision, and materializing it as a 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. +//! +//! 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 +//! producer of that spec. + +use std::path::Path; + +mod git; + +use git::IsolatedGit; + +/// Branch a source that carries no Git history of its own is initialized on. +/// Matches the branch a fixture-only task repository has always used, so a run +/// without a codebase looks the same as it always did. +pub const INITIALIZED_BRANCH: &str = "work"; + +/// A declared source, independent of what it is being sourced *for*. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum SourceSpec { + Git { + url: String, + reference: String, + }, + /// A directory on this host. Relative paths resolve against the `base_dir` + /// handed to [`resolve`] — for an eval config, the directory holding it. + Path { + path: String, + }, +} + +/// The read-only outcome of [`resolve`]. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ResolvedSource { + /// The url or path exactly as declared. + pub source: String, + /// The absolute directory a path source resolved to. Absent for a git url, + /// which names no directory on this host. + pub resolved_path: Option, + /// The declared ref, for a git source. + pub reference: Option, + /// The commit the declaration resolves to. `None` when the source is a + /// directory that is not a Git repository — there is no commit to name. + pub revision: Option, + /// The source repository's `origin`, when it has one. Recorded because it is + /// the only reproducible handle a host-local path source can offer: another + /// reader cannot resolve the path, but can resolve `origin` + `revision`. + pub origin_url: Option, + /// Branch a materialized copy checks out. + pub branch: String, + /// True when the declaration cannot be resolved off this host, so a report + /// citing it is not reproducible from the config alone. + pub host_local: bool, + /// Things the operator should know about what this resolution did or did not + /// carry. This module never prints; the `cli` layer owns the `⚠ ` prefix. + pub warnings: Vec, +} + +#[derive(Debug, thiserror::Error)] +pub enum SourceError { + #[error("{0}")] + Message(String), +} + +impl SourceError { + fn msg(message: impl Into) -> Self { + Self::Message(message.into()) + } +} + +/// Resolve `spec` without creating anything on disk. +pub fn resolve(spec: &SourceSpec, base_dir: &Path) -> Result { + match spec { + SourceSpec::Git { url, reference } => resolve_git(url, reference), + SourceSpec::Path { path } => resolve_path(path, base_dir), + } +} + +fn resolve_path(declared: &str, base_dir: &Path) -> Result { + let joined = { + let path = Path::new(declared); + if path.is_absolute() { + path.to_path_buf() + } else { + base_dir.join(path) + } + }; + let directory = crate::core::fs::real_path(&joined).map_err(|error| { + SourceError::msg(format!( + "codebase path '{declared}' could not be resolved ({}): {error}", + joined.display() + )) + })?; + if !directory.is_dir() { + return Err(SourceError::msg(format!( + "codebase path '{declared}' is not a directory: {}", + directory.display() + ))); + } + + let git = IsolatedGit::new().map_err(SourceError::msg)?; + let text = |args: &[&str]| { + let output = git.run(&directory, args); + (output.status == Some(0)) + .then(|| String::from_utf8_lossy(&output.stdout).trim().to_string()) + .filter(|value| !value.is_empty()) + }; + + // Materialization takes a clean checkout of HEAD, so anything uncommitted in + // the source is not carried. That is the chosen behavior, not a bug — but it + // is invisible from the task environment, so it is said out loud here. + let mut warnings = Vec::new(); + if text(&["status", "--porcelain"]).is_some() { + warnings.push(format!( + "codebase path '{declared}' has uncommitted changes; the task environment is a clean \ + checkout of its committed state and does not include them" + )); + } + + Ok(ResolvedSource { + source: declared.to_string(), + resolved_path: Some(directory.to_string_lossy().into_owned()), + reference: None, + revision: text(&["rev-parse", "HEAD"]), + origin_url: text(&["remote", "get-url", "origin"]), + // A detached HEAD reports no symbolic ref either; both it and a plain + // directory land on the branch a fresh `git init` would have created. + branch: text(&["symbolic-ref", "--short", "HEAD"]) + .unwrap_or_else(|| INITIALIZED_BRANCH.to_string()), + host_local: true, + warnings, + }) +} + +fn resolve_git(url: &str, reference: &str) -> Result { + let refs = list_remote(url)?; + let value_of = |name: &str| { + refs.iter() + .find(|(candidate, _)| candidate == name) + .map(|(_, value)| value.clone()) + }; + + // A branch keeps its own name; anything else lands on the repository's + // default branch, since a tag or a bare SHA names no branch to be on. + let branch_ref = format!("refs/heads/{reference}"); + let (revision, branch) = match value_of(&branch_ref) { + Some(revision) => (revision, reference.to_string()), + None => { + // `refs/tags/^{}` is the commit an annotated tag peels to; a + // lightweight tag advertises only the unpeeled name, which already + // is a commit. + let tagged = value_of(&format!("refs/tags/{reference}^{{}}")) + .or_else(|| value_of(&format!("refs/tags/{reference}"))); + // A remote advertises refs, not arbitrary commits, so a SHA matches + // nothing above and is taken at face value. Materialization is what + // proves it exists — it fails loudly there if it does not. + let revision = tagged + .or_else(|| is_full_sha(reference).then(|| reference.to_string())) + .ok_or_else(|| { + SourceError::msg(format!( + "codebase ref '{reference}' does not exist in {url}" + )) + })?; + (revision, default_branch(&refs, url)?) + } + }; + + Ok(ResolvedSource { + source: url.to_string(), + resolved_path: None, + reference: Some(reference.to_string()), + revision: Some(revision), + // The url *is* the origin, and materialization strips the remote, so + // recording it here keeps the pointer the stripped remote would have been. + origin_url: Some(url.to_string()), + branch, + host_local: false, + warnings: Vec::new(), + }) +} + +/// Materialize `resolved` into `dest`, which must not already exist. +/// +/// A source with history is cloned, so the history arrives with it; a plain +/// directory is copied and initialized. Either way `dest` ends up a Git +/// repository, checked out on [`ResolvedSource::branch`], with no remote — a +/// task environment must not be able to reach the source it came from. +pub fn materialize(resolved: &ResolvedSource, dest: &Path) -> Result<(), SourceError> { + if let Some(parent) = dest.parent() { + std::fs::create_dir_all(parent).map_err(|error| { + SourceError::msg(format!("could not create {}: {error}", parent.display())) + })?; + } + + let git = IsolatedGit::new().map_err(SourceError::msg)?; + match (&resolved.resolved_path, &resolved.revision) { + // A directory carrying no history: copy it, then wrap it in a repository. + (Some(directory), None) => { + crate::core::fs::copy_entry_materialized(Path::new(directory), dest).map_err( + |error| { + SourceError::msg(format!( + "could not copy codebase directory {directory} into {}: {error}", + dest.display() + )) + }, + )?; + checked( + &git, + dest.parent().unwrap_or(dest), + &[ + "init", + "--quiet", + "--initial-branch", + &resolved.branch, + // An empty template, so a configured `init.templateDir` + // cannot seed hooks into a task repository. + "--template", + &git.template_dir().to_string_lossy(), + &dest.to_string_lossy(), + ], + "initialize the codebase directory as a repository", + )?; + } + _ => clone_repository(&git, resolved, dest)?, + } + Ok(()) +} + +fn clone_repository( + git: &IsolatedGit, + resolved: &ResolvedSource, + dest: &Path, +) -> Result<(), SourceError> { + let from = resolved + .resolved_path + .clone() + .unwrap_or_else(|| resolved.source.clone()); + let revision = resolved.revision.as_deref().ok_or_else(|| { + SourceError::msg(format!( + "codebase {from} resolved to no commit to check out" + )) + })?; + + // `--no-checkout` skips populating the working tree at the remote's default + // branch only to replace it a moment later. + checked( + git, + Path::new("."), + &[ + "clone", + "--quiet", + "--no-checkout", + // An empty template, for the same reason `init` uses one. + "--template", + &git.template_dir().to_string_lossy(), + &from, + &dest.to_string_lossy(), + ], + &format!("clone codebase {from}"), + )?; + // `-B` both creates the branch at the resolved commit and checks it out, so a + // tag or bare SHA never leaves the environment on a detached HEAD. + checked( + git, + dest, + &["checkout", "--quiet", "-B", &resolved.branch, revision], + &format!("check out {revision} of codebase {from}"), + )?; + checked( + git, + dest, + &["remote", "remove", "origin"], + "remove the cloned remote", + )?; + Ok(()) +} + +/// 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); + if output.status == Some(0) { + return Ok(()); + } + Err(SourceError::msg(format!( + "could not {intent}: {}", + String::from_utf8_lossy(&output.stderr).trim() + ))) +} + +/// Whether `reference` is a full 40-character object name. +/// +/// Only the full form. An abbreviated SHA cannot be distinguished from a branch +/// named `abc1234`, and guessing wrong would silently source the wrong tree. +fn is_full_sha(reference: &str) -> bool { + reference.len() == 40 && reference.chars().all(|c| c.is_ascii_hexdigit()) +} + +/// The branch `HEAD` points at on the remote, from the `--symref` line. +fn default_branch(refs: &[(String, String)], url: &str) -> Result { + refs.iter() + .find(|(name, value)| name == "HEAD" && value.starts_with("ref: refs/heads/")) + .and_then(|(_, value)| value.strip_prefix("ref: refs/heads/")) + .map(str::to_string) + .ok_or_else(|| { + SourceError::msg(format!( + "could not determine the default branch of {url}; it advertises no HEAD symref" + )) + }) +} + +/// `(ref name, value)` pairs advertised by `url`, including the `HEAD` symref. +/// +/// Deliberately unfiltered. Passing a ref pattern makes `ls-remote` list only +/// matching refs, which drops the `ref: refs/heads/\tHEAD` line — and that +/// line is the only way to learn the remote's default branch. One unfiltered +/// call answers both questions in one round trip. +fn list_remote(url: &str) -> Result, SourceError> { + let git = IsolatedGit::new().map_err(SourceError::msg)?; + let output = git.run(Path::new("."), &["ls-remote", "--symref", url]); + if output.status != Some(0) { + return Err(SourceError::msg(format!( + "could not read codebase repository {url}: {}", + String::from_utf8_lossy(&output.stderr).trim() + ))); + } + Ok(String::from_utf8_lossy(&output.stdout) + .lines() + .filter_map(|line| line.split_once('\t')) + .map(|(value, name)| (name.trim().to_string(), value.trim().to_string())) + .collect()) +} + +#[cfg(test)] +#[path = "tests.rs"] +mod tests; diff --git a/src/source/tests.rs b/src/source/tests.rs new file mode 100644 index 0000000..90c8318 --- /dev/null +++ b/src/source/tests.rs @@ -0,0 +1,424 @@ +//! Tests for [`super`]: resolving a declared source and materializing it. +//! +//! Extracted from `mod.rs` because the module outgrew the file it exercised +//! — the convention in CLAUDE.md, whose trigger is size rather than style. + +use super::*; + +use std::path::{Path, PathBuf}; + +use crate::core::run_git; + +/// A repository at `name` with one commit on `branch`, usable as a clone URL. +fn source_repo(root: &Path, name: &str, branch: &str) -> PathBuf { + let repo = root.join(name); + std::fs::create_dir_all(&repo).unwrap(); + run_git(&["init", "--quiet", "--initial-branch", branch, "."], &repo); + std::fs::write(repo.join("README.md"), "source\n").unwrap(); + run_git(&["add", "--all"], &repo); + run_git( + &[ + "-c", + "user.name=source", + "-c", + "user.email=source@localhost", + "commit", + "--quiet", + "--no-gpg-sign", + "-m", + "initial", + ], + &repo, + ); + repo +} + +/// The commit `revision` names in `repo`. +fn sha(repo: &Path, revision: &str) -> String { + let out = run_git(&["rev-parse", revision], repo); + String::from_utf8_lossy(&out.stdout).trim().to_string() +} + +/// Trimmed stdout of a git invocation in `repo`. +fn git_text(repo: &Path, args: &[&str]) -> String { + let out = run_git(args, repo); + String::from_utf8_lossy(&out.stdout).trim().to_string() +} + +/// Add one more commit touching `file`, so history has depth to preserve. +fn commit(repo: &Path, file: &str, message: &str) { + std::fs::write(repo.join(file), format!("{message}\n")).unwrap(); + run_git(&["add", "--all"], repo); + run_git( + &[ + "-c", + "user.name=source", + "-c", + "user.email=source@localhost", + "commit", + "--quiet", + "--no-gpg-sign", + "-m", + message, + ], + repo, + ); +} + +#[test] +fn git_source_resolves_a_branch_ref_to_its_commit_and_default_branch() { + 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(), + ) + .expect("a branch ref on a reachable repository resolves"); + + assert_eq!( + resolved.revision.as_deref(), + Some(sha(&origin, "main").as_str()) + ); + assert_eq!(resolved.branch, "main"); + assert!( + !resolved.host_local, + "a git url is reproducible from the config alone" + ); +} + +/// A tag names no branch, so the checkout has to land somewhere. It lands on +/// the repository's *own* default branch — which is only knowable from the +/// `HEAD` symref line, and `ls-remote` suppresses that line when a ref +/// pattern is passed. This test is what holds the unfiltered call in place. +#[test] +fn git_source_resolves_an_annotated_tag_to_its_commit_on_the_default_branch() { + let tmp = tempfile::TempDir::new().unwrap(); + let origin = source_repo(tmp.path(), "origin", "trunk"); + run_git( + &[ + "-c", + "user.name=source", + "-c", + "user.email=source@localhost", + "tag", + "--annotate", + "v1", + "-m", + "release", + ], + &origin, + ); + + let resolved = resolve( + &SourceSpec::Git { + url: origin.to_string_lossy().into_owned(), + reference: "v1".to_string(), + }, + tmp.path(), + ) + .expect("an annotated tag resolves"); + + assert_eq!( + resolved.revision.as_deref(), + Some(sha(&origin, "v1^{commit}").as_str()), + "an annotated tag must resolve to the commit it peels to" + ); + assert_ne!( + resolved.revision.as_deref(), + Some(sha(&origin, "v1").as_str()), + "the tag object is not a commit and cannot be checked out as one" + ); + assert_eq!(resolved.branch, "trunk"); +} + +#[test] +fn path_source_that_is_a_repository_records_its_revision_origin_and_branch() { + let tmp = tempfile::TempDir::new().unwrap(); + let upstream = source_repo(tmp.path(), "upstream", "main"); + let local = source_repo(tmp.path(), "local", "feature"); + run_git( + &["remote", "add", "origin", &upstream.to_string_lossy()], + &local, + ); + + // Declared relative, so this also pins resolution against `base_dir`. + let resolved = resolve( + &SourceSpec::Path { + path: "local".to_string(), + }, + tmp.path(), + ) + .expect("a local repository resolves"); + + assert_eq!( + resolved.revision.as_deref(), + Some(sha(&local, "HEAD").as_str()) + ); + assert_eq!(resolved.branch, "feature"); + assert!( + resolved.host_local, + "a path names a directory only this host has" + ); + // The origin is what makes a host-local source citable elsewhere: + // `origin` + `revision` is reproducible even though `path` is not. + assert_eq!( + resolved.origin_url.as_deref(), + Some(upstream.to_string_lossy().as_ref()) + ); +} + +/// The ticket's second acceptance criterion: a plain directory still has to +/// yield a working task repository, so it resolves rather than failing — +/// with no commit to name, on the branch a fresh `git init` will create. +#[test] +fn path_source_that_is_not_a_repository_resolves_without_a_revision() { + 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() {}\n").unwrap(); + + let resolved = resolve( + &SourceSpec::Path { + path: plain.to_string_lossy().into_owned(), + }, + tmp.path(), + ) + .expect("a directory that is not a repository still resolves"); + + assert_eq!(resolved.revision, None, "a plain directory names no commit"); + assert_eq!(resolved.origin_url, None); + assert_eq!(resolved.branch, INITIALIZED_BRANCH); + assert!(resolved.host_local); +} + +/// A remote advertises refs, not arbitrary commits, so a SHA matches nothing +/// in `ls-remote` and is taken at face value here; the clone proves it exists. +#[test] +fn git_source_accepts_a_full_sha_ref_on_the_default_branch() { + let tmp = tempfile::TempDir::new().unwrap(); + let origin = source_repo(tmp.path(), "origin", "trunk"); + let head = sha(&origin, "HEAD"); + + let resolved = resolve( + &SourceSpec::Git { + url: origin.to_string_lossy().into_owned(), + reference: head.clone(), + }, + tmp.path(), + ) + .expect("a full commit SHA resolves"); + + assert_eq!(resolved.revision.as_deref(), Some(head.as_str())); + assert_eq!(resolved.branch, "trunk"); +} + +#[test] +fn git_source_ref_that_does_not_exist_names_the_ref_and_the_url() { + let tmp = tempfile::TempDir::new().unwrap(); + let origin = source_repo(tmp.path(), "origin", "main"); + + let error = resolve( + &SourceSpec::Git { + url: origin.to_string_lossy().into_owned(), + reference: "no-such-branch".to_string(), + }, + tmp.path(), + ) + .expect_err("an unresolvable ref fails") + .to_string(); + + assert!(error.contains("no-such-branch"), "error was: {error}"); + assert!( + error.contains(&origin.to_string_lossy().into_owned()), + "error was: {error}" + ); +} + +/// The user chose a clean checkout of HEAD over a verbatim copy, so a dirty +/// working tree is silently *not* carried. Saying so is what keeps that from +/// being a surprise. +#[test] +fn path_source_with_uncommitted_changes_warns_that_they_are_not_carried() { + let tmp = tempfile::TempDir::new().unwrap(); + let local = source_repo(tmp.path(), "local", "main"); + std::fs::write(local.join("README.md"), "edited but never committed\n").unwrap(); + + let resolved = resolve( + &SourceSpec::Path { + path: local.to_string_lossy().into_owned(), + }, + tmp.path(), + ) + .expect("a dirty repository still resolves"); + + assert!( + resolved + .warnings + .iter() + .any(|warning| warning.contains("uncommitted")), + "warnings were: {:?}", + resolved.warnings + ); +} + +#[test] +fn path_source_with_a_clean_tree_warns_about_nothing() { + let tmp = tempfile::TempDir::new().unwrap(); + let local = source_repo(tmp.path(), "local", "main"); + + let resolved = resolve( + &SourceSpec::Path { + path: local.to_string_lossy().into_owned(), + }, + tmp.path(), + ) + .expect("a clean repository resolves"); + + assert!(resolved.warnings.is_empty(), "{:?}", resolved.warnings); +} + +/// The ticket's first acceptance criterion, at the resolver boundary: a real +/// checkout, history intact, no remotes configured. +#[test] +fn materializing_a_git_source_keeps_history_and_configures_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(), + ) + .unwrap(); + let dest = tmp.path().join("materialized"); + + materialize(&resolved, &dest).expect("a git source materializes"); + + assert_eq!(sha(&dest, "HEAD"), resolved.revision.unwrap()); + assert_eq!( + git_text(&dest, &["symbolic-ref", "--short", "HEAD"]), + "main" + ); + assert_eq!( + git_text(&dest, &["rev-list", "--count", "HEAD"]), + "2", + "the clone must carry the source's history, not a squashed snapshot" + ); + assert_eq!( + git_text(&dest, &["remote"]), + "", + "a task environment must not be able to reach the source it came from" + ); + assert_eq!( + std::fs::read_to_string(dest.join("second.txt")).unwrap(), + "second\n" + ); +} + +/// A tag checks out detached by default; the resolver promised a branch, so +/// materialization has to put one there. +#[test] +fn materializing_a_tag_lands_on_the_default_branch_not_a_detached_head() { + let tmp = tempfile::TempDir::new().unwrap(); + let origin = source_repo(tmp.path(), "origin", "trunk"); + commit(&origin, "second.txt", "second"); + run_git( + &[ + "-c", + "user.name=source", + "-c", + "user.email=source@localhost", + "tag", + "--annotate", + "v1", + "-m", + "release", + ], + &origin, + ); + let resolved = resolve( + &SourceSpec::Git { + url: origin.to_string_lossy().into_owned(), + reference: "v1".to_string(), + }, + tmp.path(), + ) + .unwrap(); + let dest = tmp.path().join("materialized"); + + materialize(&resolved, &dest).expect("a tag materializes"); + + assert_eq!( + git_text(&dest, &["symbolic-ref", "--short", "HEAD"]), + "trunk" + ); + assert_eq!(sha(&dest, "HEAD"), resolved.revision.unwrap()); +} + +/// The ticket's second acceptance criterion: a plain directory becomes a +/// working task repository rather than failing for lack of one. +#[test] +fn materializing_a_plain_directory_initializes_a_repository_around_it() { + 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() {}\n").unwrap(); + let resolved = resolve( + &SourceSpec::Path { + path: plain.to_string_lossy().into_owned(), + }, + tmp.path(), + ) + .unwrap(); + let dest = tmp.path().join("materialized"); + + materialize(&resolved, &dest).expect("a plain directory materializes"); + + assert_eq!( + std::fs::read_to_string(dest.join("src/main.rs")).unwrap(), + "fn main() {}\n" + ); + assert_eq!( + git_text(&dest, &["rev-parse", "--is-inside-work-tree"]), + "true", + "a plain directory still has to arrive as a repository" + ); + assert_eq!( + git_text(&dest, &["symbolic-ref", "--short", "HEAD"]), + INITIALIZED_BRANCH + ); +} + +/// A local repository source is a clean checkout of its committed state — +/// the decision taken on the ticket — so an uncommitted edit is not carried. +#[test] +fn materializing_a_dirty_local_repository_carries_only_committed_state() { + let tmp = tempfile::TempDir::new().unwrap(); + let local = source_repo(tmp.path(), "local", "main"); + std::fs::write(local.join("README.md"), "uncommitted\n").unwrap(); + std::fs::write(local.join("untracked.txt"), "untracked\n").unwrap(); + let resolved = resolve( + &SourceSpec::Path { + path: local.to_string_lossy().into_owned(), + }, + tmp.path(), + ) + .unwrap(); + let dest = tmp.path().join("materialized"); + + materialize(&resolved, &dest).expect("a dirty local repository materializes"); + + assert_eq!( + std::fs::read_to_string(dest.join("README.md")).unwrap(), + "source\n", + "the committed content, not the working-tree edit" + ); + assert!(!dest.join("untracked.txt").exists()); + assert_eq!(git_text(&dest, &["remote"]), ""); +} diff --git a/src/validation/evals.rs b/src/validation/evals.rs index f25cdb2..16742f9 100644 --- a/src/validation/evals.rs +++ b/src/validation/evals.rs @@ -15,6 +15,7 @@ use crate::validation::schema::{SchemaName, validate_against_schema}; /// supplemental duplicate-`id`, command environment, and held-out path guards, /// returning the typed config on success. pub fn validate_evals_config(config: &Value, source: &str) -> Result { + validate_codebase_declarations(config, source)?; let validated: EvalsConfig = validate_against_schema(SchemaName::Evals, config, source)?; let mut seen = HashSet::new(); @@ -129,6 +130,67 @@ pub fn validate_evals_config(config: &Value, source: &str) -> Result Result<(), ValidationError> { + if let Some(codebase) = config.get("codebase") { + validate_codebase(source, "codebase", codebase)?; + } + let evals = config.get("evals").and_then(Value::as_array); + for (index, eval) in evals.into_iter().flatten().enumerate() { + let Some(codebase) = eval.get("codebase") else { + continue; + }; + let id = eval + .get("id") + .and_then(Value::as_str) + .map_or_else(|| format!("evals[{index}]"), str::to_string); + validate_codebase(source, &format!("eval '{id}', codebase"), codebase)?; + } + Ok(()) +} + +fn validate_codebase(source: &str, label: &str, value: &Value) -> Result<(), ValidationError> { + // A non-object is a plain type error the schema words perfectly well. + let Some(fields) = value.as_object() else { + return Ok(()); + }; + let invalid = |message: String| ValidationError::InvalidConfig { + path: source.to_string(), + message, + }; + + if fields.contains_key("url") && fields.contains_key("path") { + return Err(invalid(format!( + "{label}: declares both 'url' and 'path'; a codebase is sourced from one or the other" + ))); + } + if fields.contains_key("url") && !fields.contains_key("ref") { + return Err(invalid(format!( + "{label}: 'url' requires an explicit 'ref' (branch, tag, or commit SHA). The runner \ + records the resolved SHA, so an eval tracking a moving branch could not be re-run \ + against what it measured." + ))); + } + for field in ["url", "ref", "path"] { + if let Some(Value::String(text)) = fields.get(field) + && text.trim().is_empty() + { + return Err(invalid(format!( + "{label}: '{field}' must contain non-whitespace text" + ))); + } + } + Ok(()) +} + fn validate_environment_name( source: &str, eval_id: &str, @@ -185,6 +247,7 @@ fn paths_overlap(left: &Path, right: &Path) -> bool { #[cfg(test)] mod tests { use super::validate_evals_config; + use crate::core::CodebaseSource; use serde_json::{Value, json}; /// The minimal valid config the cases below mutate. @@ -628,4 +691,110 @@ mod tests { let config = with_command_check(&["src/main.rs"], &["holdout/test.txt"]); validate_evals_config(&config, "evals.json").unwrap(); } + + #[test] + fn accepts_a_top_level_git_codebase_as_the_default() { + let mut config = base(); + config["codebase"] = json!({ "url": "https://example.com/project.git", "ref": "main" }); + + let parsed = validate_evals_config(&config, "evals.json").unwrap(); + + assert_eq!( + parsed.codebase, + Some(CodebaseSource::Git { + url: "https://example.com/project.git".to_string(), + reference: "main".to_string(), + }) + ); + } + + #[test] + fn accepts_a_per_eval_path_codebase_overriding_the_default() { + let mut config = base(); + config["codebase"] = json!({ "url": "https://example.com/project.git", "ref": "main" }); + config["evals"][0]["codebase"] = json!({ "path": "../fixtures/legacy-service" }); + + let parsed = validate_evals_config(&config, "evals.json").unwrap(); + + assert_eq!( + parsed.evals[0].codebase, + Some(CodebaseSource::Path { + path: "../fixtures/legacy-service".to_string(), + }) + ); + } + + #[test] + fn accepts_a_top_level_path_codebase() { + let mut config = base(); + config["codebase"] = json!({ "path": "/srv/projects/legacy-service" }); + + let parsed = validate_evals_config(&config, "evals.json").unwrap(); + + assert_eq!( + parsed.codebase, + Some(CodebaseSource::Path { + path: "/srv/projects/legacy-service".to_string(), + }) + ); + } + + /// `minLength: 1` admits `" "`, so the schema cannot carry this on its own. + #[test] + fn rejects_whitespace_only_codebase_values() { + for (field, codebase) in [ + ("url", json!({ "url": " ", "ref": "main" })), + ( + "ref", + json!({ "url": "https://example.com/p.git", "ref": "\t" }), + ), + ("path", json!({ "path": " " })), + ] { + let mut config = base(); + config["codebase"] = codebase.clone(); + let error = validate_evals_config(&config, "evals.json") + .unwrap_err() + .to_string(); + assert!(error.contains("codebase"), "{field}: error was: {error}"); + assert!(error.contains(field), "{field}: error was: {error}"); + + // The per-eval override runs through the same guard, and names the eval. + let mut config = base(); + config["evals"][0]["codebase"] = codebase; + let error = validate_evals_config(&config, "evals.json") + .unwrap_err() + .to_string(); + assert!(error.contains("e1"), "{field}: error was: {error}"); + assert!(error.contains(field), "{field}: error was: {error}"); + } + } + + /// A source is one thing or the other. The schema's `oneOf` plus + /// `additionalProperties: false` on each branch is what rejects the hybrid; + /// this pins that so a later schema edit cannot quietly admit it. + #[test] + fn rejects_a_codebase_that_is_both_git_and_path() { + let mut config = base(); + config["codebase"] = json!({ + "url": "https://example.com/p.git", + "ref": "main", + "path": "/srv/p" + }); + + assert!(validate_evals_config(&config, "evals.json").is_err()); + } + + /// #244 decision 5: the runner records the resolved SHA, so a git source + /// without an explicit ref could not be re-run against what it measured. + #[test] + fn rejects_a_git_codebase_without_a_ref() { + let mut config = base(); + config["codebase"] = json!({ "url": "https://example.com/p.git" }); + + let error = validate_evals_config(&config, "evals.json") + .unwrap_err() + .to_string(); + + assert!(error.contains("ref"), "error was: {error}"); + } } diff --git a/src/workspace/promote.rs b/src/workspace/promote.rs index 076365a..c883659 100644 --- a/src/workspace/promote.rs +++ b/src/workspace/promote.rs @@ -228,6 +228,50 @@ fn label(value: &impl Serialize) -> String { .unwrap_or_else(|| "unknown".to_string()) } +/// Provenance-table rows naming each codebase the iteration ran against, or an +/// empty string when it ran against none. +/// +/// A reader deciding whether to believe a published baseline needs the commit, +/// not the ref: a branch has moved by the time they read it. Where the source is +/// a directory on the machine that ran it, the row says so — that reader cannot +/// resolve the path, and the row should not imply otherwise. +fn codebase_rows(conditions: Option<&ConditionsRecord>) -> String { + let codebases = conditions.map(|c| c.codebases.as_slice()).unwrap_or(&[]); + if codebases.is_empty() { + return String::new(); + } + let multiple = codebases.len() > 1; + codebases + .iter() + .map(|used| { + // One codebase needs no disambiguation; several do, and the eval ids + // are what tie a row to the cells it covers. + let label = if multiple { + format!("Codebase ({})", used.evals.join(", ")) + } else { + "Codebase".to_string() + }; + let mut cell = used.codebase.source.clone(); + if let Some(reference) = &used.codebase.reference { + cell.push('@'); + cell.push_str(reference); + } + if let Some(revision) = &used.codebase.revision { + let short: String = revision.chars().take(7).collect(); + cell.push_str(&format!(" ({short})")); + } + if used.codebase.host_local { + cell.push_str(" — host-local path, not reproducible from this config alone"); + if let Some(origin) = &used.codebase.origin_url { + cell.push_str(&format!("; origin {origin}")); + } + } + format!("| {label} | {cell} |") + }) + .collect::>() + .join("\n") +} + /// Build the `BASELINE.md` provenance document — byte-for-byte the layout of /// `promote-baseline.ts`. fn provenance(opts: &PromoteOptions, conditions: Option<&ConditionsRecord>, head: &str) -> String { @@ -262,6 +306,8 @@ fn provenance(opts: &PromoteOptions, conditions: Option<&ConditionsRecord>, head .or_else(|| conditions.and_then(|c| c.label.as_deref())) .unwrap_or("(none)"); + let codebase_rows = codebase_rows(conditions); + let lines = [ format!("# Baseline — {}", opts.skill_name), String::new(), @@ -284,6 +330,7 @@ fn provenance(opts: &PromoteOptions, conditions: Option<&ConditionsRecord>, head format!("| Conditions | {conditions_cell} |"), format!("| Run timestamp | {timestamp} |"), format!("| Label | {run_label} |"), + codebase_rows, format!("| Promoted from commit | {head} |"), String::new(), "Files:".to_string(), @@ -301,6 +348,7 @@ fn provenance(opts: &PromoteOptions, conditions: Option<&ConditionsRecord>, head #[cfg(test)] mod tests { use super::*; + use serde_json::Value; use tempfile::TempDir; /// Write `body` to `path`, creating parent dirs. @@ -546,6 +594,79 @@ mod tests { assert!(provenance.contains("Label | canonical-run")); } + /// A published baseline is read by people deciding whether to believe it. + /// Naming the commit is what lets them check. + #[test] + fn provenance_names_the_codebase_and_the_commit_it_resolved_to() { + let f = fixture(1); + let conditions: Value = serde_json::from_str(CONDITIONS_WITH_PROVENANCE).unwrap(); + let mut conditions = conditions; + conditions["codebases"] = serde_json::json!([{ + "kind": "git", + "source": "https://example.com/project.git", + "ref": "v1.4.0", + "revision": "a1b2c3d4e5f60718293a4b5c6d7e8f9012345678", + "branch": "v1.4.0", + "evals": ["e1"] + }]); + write( + &f.iteration_dir.join("conditions.json"), + &serde_json::to_string(&conditions).unwrap(), + ); + write( + &f.iteration_dir.join("benchmark.json"), + r#"{"delta":{"pass_rate":0}}"#, + ); + + promote_baseline(&opts(&f, 1)).unwrap(); + + let provenance = + fs::read_to_string(f.skill_subdir.join("evals/baseline/BASELINE.md")).unwrap(); + assert!(provenance.contains("Codebase"), "{provenance}"); + assert!( + provenance.contains("https://example.com/project.git"), + "{provenance}" + ); + assert!(provenance.contains("v1.4.0"), "{provenance}"); + assert!(provenance.contains("a1b2c3d"), "{provenance}"); + } + + /// A host-local path is not reproducible by the reader, so the row says so + /// rather than presenting it like a resolvable reference. + #[test] + fn provenance_marks_a_host_local_codebase_as_unreproducible() { + let f = fixture(1); + let mut conditions: Value = serde_json::from_str(CONDITIONS_WITH_PROVENANCE).unwrap(); + conditions["codebases"] = serde_json::json!([{ + "kind": "path", + "source": "../fixtures/legacy-service", + "revision": "a1b2c3d4e5f60718293a4b5c6d7e8f9012345678", + "origin_url": "https://example.com/legacy.git", + "branch": "main", + "host_local": true, + "evals": ["e1"] + }]); + write( + &f.iteration_dir.join("conditions.json"), + &serde_json::to_string(&conditions).unwrap(), + ); + write( + &f.iteration_dir.join("benchmark.json"), + r#"{"delta":{"pass_rate":0}}"#, + ); + + promote_baseline(&opts(&f, 1)).unwrap(); + + let provenance = + fs::read_to_string(f.skill_subdir.join("evals/baseline/BASELINE.md")).unwrap(); + assert!(provenance.contains("host-local"), "{provenance}"); + // The origin is what a reader elsewhere can actually resolve. + assert!( + provenance.contains("https://example.com/legacy.git"), + "{provenance}" + ); + } + #[test] fn promote_flags_override_manifest_values() { let f = fixture(1); diff --git a/tests/cli/aggregate/shadow.rs b/tests/cli/aggregate/shadow.rs index 238a02e..62f267e 100644 --- a/tests/cli/aggregate/shadow.rs +++ b/tests/cli/aggregate/shadow.rs @@ -252,3 +252,50 @@ fn aggregate_suppresses_declared_isolated_shadows_for_every_harness() { ); } } + +/// `benchmark.json` is the artifact a published comparison is read from, so the +/// tree each condition ran against has to survive the aggregation step rather +/// than stopping at `conditions.json`. +#[test] +fn aggregate_echoes_the_resolved_codebases_into_the_benchmark() { + use serde_json::json; + let (_tmp, root) = canonical_root(); + let (skill_dir, skill_md, iteration_dir, cwd) = setup_agg(&root); + new_skill_conditions(&iteration_dir, &skill_md); + let conditions_path = iteration_dir.join("conditions.json"); + let mut conditions: serde_json::Value = + serde_json::from_str(&fs::read_to_string(&conditions_path).unwrap()).unwrap(); + conditions.as_object_mut().unwrap().insert( + "codebases".to_string(), + json!([{ + "kind": "git", + "source": "https://example.com/project.git", + "ref": "v1.4.0", + "revision": "a1b2c3d4e5f60718293a4b5c6d7e8f9012345678", + "branch": "v1.4.0", + "evals": ["e1"] + }]), + ); + fs::write( + &conditions_path, + serde_json::to_string(&conditions).unwrap(), + ) + .unwrap(); + for cond in ["with_skill", "without_skill"] { + write_grading(&iteration_dir, cond, 1.0); + write_timing( + &iteration_dir, + cond, + json!({"total_tokens": 100, "duration_ms": 1}), + ); + } + + agg_cmd(&cwd, &skill_dir).assert().success(); + + let b = read_benchmark(&iteration_dir); + assert_eq!( + b["codebases"][0]["revision"], + "a1b2c3d4e5f60718293a4b5c6d7e8f9012345678" + ); + assert_eq!(b["codebases"][0]["evals"][0], "e1"); +} diff --git a/tests/cli/docs.rs b/tests/cli/docs.rs index 8f01c74..e9c10e2 100644 --- a/tests/cli/docs.rs +++ b/tests/cli/docs.rs @@ -161,6 +161,25 @@ fn docs_isolation_keeps_remedies_and_verification() { .stdout(contains("\"subtype\":\"init\"")); } +/// 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. +#[test] +fn docs_codebase_keeps_the_declaration_rules_and_reproducibility_caveat() { + skill_eval() + .args(["docs", "codebase"]) + .assert() + .success() + .stdout(contains("# Sourcing a codebase into a task environment")) + .stdout(contains("\"ref\"")) + .stdout(contains("`ref` is required")) + .stdout(contains("overlay")) + .stdout(contains("refs/eval-magic/baseline")) + .stdout(contains("host_local")) + .stdout(contains("not reproducible")); +} + #[test] fn shipped_guides_do_not_depend_on_repository_relative_links() { for (topic, _, body, path) in guide_sources() { diff --git a/tests/run/codebase.rs b/tests/run/codebase.rs new file mode 100644 index 0000000..146cfe3 --- /dev/null +++ b/tests/run/codebase.rs @@ -0,0 +1,349 @@ +//! Sourcing a real codebase into each task environment (issue #252). +//! +//! The environments a run builds are asserted here rather than in unit tests +//! because the property under test spans resolution, provisioning, staging, and +//! the fixture overlay — it is only true of a whole prepared workspace. + +use crate::helpers::*; +use std::fs; +use std::path::{Path, PathBuf}; +use std::process::Command; + +fn git(cwd: &Path, args: &[&str]) -> String { + let output = Command::new("git") + .current_dir(cwd) + .args(args) + .output() + .unwrap(); + assert!( + output.status.success(), + "git {} failed in {}:\n{}", + args.join(" "), + cwd.display(), + String::from_utf8_lossy(&output.stderr) + ); + String::from_utf8(output.stdout).unwrap().trim().to_string() +} + +/// A repository usable as a codebase source: two commits on `branch`, with a +/// `.gitignore` that ignores `build/`, and an ignored file already present. +fn codebase_repo(root: &Path, name: &str, branch: &str) -> PathBuf { + let repo = root.join(name); + fs::create_dir_all(repo.join("src")).unwrap(); + git(&repo, &["init", "--quiet", "--initial-branch", branch, "."]); + fs::write(repo.join(".gitignore"), "build/\n").unwrap(); + fs::write(repo.join("src/lib.rs"), "pub fn one() -> u32 { 1 }\n").unwrap(); + commit(&repo, "first"); + fs::write(repo.join("src/main.rs"), "fn main() {}\n").unwrap(); + commit(&repo, "second"); + fs::create_dir_all(repo.join("build")).unwrap(); + fs::write(repo.join("build/artifact.bin"), "not source\n").unwrap(); + repo +} + +fn commit(cwd: &Path, message: &str) { + git(cwd, &["add", "--all"]); + git( + cwd, + &[ + "-c", + "user.name=Codebase Author", + "-c", + "user.email=codebase@example.com", + "commit", + "--quiet", + "-m", + message, + ], + ); +} + +/// An evals config whose single eval overlays `TASK.md` onto `codebase`. +fn evals_with_codebase(codebase: &str) -> String { + format!( + r#"{{ + "skill_name": "mr-review", + "codebase": {codebase}, + "evals": [ + {{ + "id": "e1", + "prompt": "add a function", + "expected_output": "a function", + "files": ["TASK.md"] + }} + ] + }}"# + ) +} + +#[test] +fn a_git_codebase_arrives_in_every_env_with_history_and_no_remote() { + 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"), + "Add a `two()` function.\n", + ) + .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() {}\n", + "{condition}: the codebase's files must be present" + ); + assert!( + git(&env, &["rev-list", "--count", "HEAD"]) + .parse::() + .unwrap() + >= 2, + "{condition}: the codebase's history must survive provisioning" + ); + assert_eq!( + git(&env, &["remote"]), + "", + "{condition}: no env may retain a remote" + ); + // The overlay: a declared fixture lands on top, at its declared path. + assert_eq!( + fs::read_to_string(env.join("TASK.md")).unwrap(), + "Add a `two()` function.\n", + "{condition}: files are an overlay on the codebase" + ); + } +} + +#[test] +fn the_baseline_ref_marks_the_state_every_codebase_env_starts_from() { + 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\n").unwrap(); + + skill_eval() + .current_dir(&cwd) + .args(["run", "--skill-dir"]) + .arg(&skill_dir) + .args(["--skill", "mr-review", "--mode", "new-skill", "--dry-run"]) + .assert() + .success(); + + let env = cli_env_dir(&cwd, "g1", "with_skill"); + assert_eq!( + git(&env, &["rev-parse", "refs/eval-magic/baseline"]), + git(&env, &["rev-parse", "HEAD"]), + "the baseline ref must name the start state" + ); + // Outside refs/heads, so it never shows up in what the agent sees. + assert_eq!(git(&env, &["branch", "--list"]), "* main"); + assert_eq!( + git(&env, &["status", "--porcelain"]), + "", + "the baseline commit must leave nothing uncommitted" + ); +} + +/// A real repository ignores its build output. Committing that into the +/// baseline would put megabytes of artifacts in every environment's start +/// state — but the runner's own files have to land regardless of what the +/// codebase ignores, or the condition under test falls outside every diff. +#[test] +fn the_baseline_respects_codebase_gitignore_but_still_tracks_runner_files() { + let tmp = tempfile::TempDir::new().unwrap(); + let origin = codebase_repo(tmp.path(), "origin", "main"); + // The codebase ignores the harness config dir the runner stages into. + fs::write(origin.join(".gitignore"), "build/\n.claude/\n").unwrap(); + commit(&origin, "ignore the harness config dir too"); + 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\n").unwrap(); + + skill_eval() + .current_dir(&cwd) + .args(["run", "--skill-dir"]) + .arg(&skill_dir) + .args(["--skill", "mr-review", "--mode", "new-skill", "--dry-run"]) + .assert() + .success(); + + let env = cli_env_dir(&cwd, "g1", "with_skill"); + let tracked = git(&env, &["ls-files"]); + + assert!( + !tracked.lines().any(|path| path.starts_with("build/")), + "gitignored build output must stay out of the baseline:\n{tracked}" + ); + assert!( + tracked.lines().any(|path| path.starts_with(".claude/")), + "the staged skill must be tracked even though the codebase ignores .claude/:\n{tracked}" + ); + assert!( + tracked.lines().any(|path| path == "TASK.md"), + "the fixture overlay must be tracked:\n{tracked}" + ); + assert!( + !tracked + .lines() + .any(|path| path.starts_with(".eval-magic-outputs")), + "framework output stays excluded:\n{tracked}" + ); +} + +/// Sourcing a codebase runs git against a URL the operator supplied, on a host +/// whose git configuration the operator also controls. `insteadOf` rewrites that +/// URL, so a leak here would silently source a *different* tree than the one the +/// eval declared — and the report would still cite the declared one. +/// +/// Injected through `GIT_CONFIG_COUNT` because that is the one mechanism a test +/// can use without writing to the developer's real `~/.gitconfig`. +#[test] +fn sourcing_a_codebase_ignores_the_operators_git_configuration() { + 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\n").unwrap(); + + skill_eval() + .current_dir(&cwd) + // Rewrites the codebase URL to somewhere that does not resolve. + .env("GIT_CONFIG_COUNT", "1") + .env( + "GIT_CONFIG_KEY_0", + "url.https://eval-magic.invalid/.insteadOf", + ) + .env("GIT_CONFIG_VALUE_0", wire_path(&origin)) + .args(["run", "--skill-dir"]) + .arg(&skill_dir) + .args(["--skill", "mr-review", "--mode", "new-skill", "--dry-run"]) + .assert() + .success(); + + let env = cli_env_dir(&cwd, "g1", "with_skill"); + assert_eq!( + fs::read_to_string(env.join("src/main.rs")).unwrap(), + "fn main() {}\n", + "the declared codebase must be the one sourced" + ); +} + +/// A report that cites a codebase has to say *which* tree it measured. The +/// declared ref is not enough — a branch moves — so the resolved commit is what +/// every provenance surface carries. +#[test] +fn the_resolved_codebase_reaches_conditions_and_every_dispatch_task() { + 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\n").unwrap(); + + skill_eval() + .current_dir(&cwd) + .args(["run", "--skill-dir"]) + .arg(&skill_dir) + .args(["--skill", "mr-review", "--mode", "new-skill", "--dry-run"]) + .assert() + .success(); + + let conditions = read_json(&iteration_dir(&cwd).join("conditions.json")); + let codebases = conditions["codebases"].as_array().unwrap(); + assert_eq!(codebases.len(), 1, "one declared codebase, resolved once"); + let recorded = &codebases[0]; + assert_eq!(recorded["kind"], "git"); + assert_eq!(recorded["source"], wire_path(&origin)); + assert_eq!(recorded["ref"], "main"); + assert_eq!(recorded["revision"], revision); + assert_eq!(recorded["branch"], "main"); + assert_eq!(recorded["evals"][0], "e1"); + assert!( + recorded.get("host_local").is_none(), + "a git url is reproducible, so the flag stays off the artifact" + ); + + // Every dispatch task carries it, which is how it reaches each run.json. + let dispatch = read_json(&iteration_dir(&cwd).join("dispatch.json")); + let tasks = dispatch["tasks"].as_array().unwrap(); + assert!(!tasks.is_empty()); + for task in tasks { + assert_eq!( + task["codebase"]["revision"], revision, + "each task records the tree it ran against" + ); + } +} + +/// A `path` source cannot be resolved by anyone else — a different machine has +/// the directory somewhere else, or nowhere. That is unfixable, so the artifact +/// says so rather than implying a reproducibility it does not have. +#[test] +fn a_path_codebase_is_recorded_as_host_local_with_its_origin_for_citation() { + let tmp = tempfile::TempDir::new().unwrap(); + let upstream = codebase_repo(tmp.path(), "upstream", "main"); + let local = codebase_repo(tmp.path(), "local", "main"); + // Git stores a remote URL byte-for-byte, and eval-magic cites it unchanged + // rather than rewriting what a user configured. Registering it in the host's + // own spelling is what pins that: on Windows the separators are backslashes, + // so any normalization on the way to the artifact shows up here. + let origin_url = upstream.to_string_lossy().to_string(); + git(&local, &["remote", "add", "origin", &origin_url]); + let revision = git(&local, &["rev-parse", "HEAD"]); + let source = format!(r#"{{ "path": "{}" }}"#, wire_path(&local)); + let (skill_dir, cwd) = setup(tmp.path(), &evals_with_codebase(&source)); + fs::write(skill_dir.join("mr-review/evals/TASK.md"), "task\n").unwrap(); + + skill_eval() + .current_dir(&cwd) + .args(["run", "--skill-dir"]) + .arg(&skill_dir) + .args(["--skill", "mr-review", "--mode", "new-skill", "--dry-run"]) + .assert() + .success(); + + let conditions = read_json(&iteration_dir(&cwd).join("conditions.json")); + let recorded = &conditions["codebases"][0]; + assert_eq!(recorded["kind"], "path"); + assert_eq!(recorded["host_local"], true); + assert_eq!(recorded["revision"], revision); + // What makes it citable anyway: origin + revision resolve anywhere. + assert_eq!(recorded["origin_url"], origin_url); +} + +/// The ticket's last acceptance criterion: an eval declaring no codebase keeps +/// the environment it has always had. +#[test] +fn a_fixture_only_eval_still_gets_the_repository_it_always_had() { + let tmp = tempfile::TempDir::new().unwrap(); + let (skill_dir, cwd) = setup(tmp.path(), DEFAULT_EVALS); + + skill_eval() + .current_dir(&cwd) + .args(["run", "--skill-dir"]) + .arg(&skill_dir) + .args(["--skill", "mr-review", "--mode", "new-skill", "--dry-run"]) + .assert() + .success(); + + let env = cli_env_dir(&cwd, "g1", "with_skill"); + assert_eq!(git(&env, &["symbolic-ref", "--short", "HEAD"]), "work"); + assert_eq!(git(&env, &["rev-list", "--count", "HEAD"]), "1"); + assert_eq!(git(&env, &["remote"]), ""); + assert_eq!(git(&env, &["status", "--porcelain"]), ""); + assert_eq!( + git(&env, &["rev-parse", "refs/eval-magic/baseline"]), + git(&env, &["rev-parse", "HEAD"]) + ); +} diff --git a/tests/run/git_isolation.rs b/tests/run/git_isolation.rs index e69c756..299f943 100644 --- a/tests/run/git_isolation.rs +++ b/tests/run/git_isolation.rs @@ -91,11 +91,15 @@ fn every_task_is_a_clean_local_git_repo_inside_a_dirty_ignored_parent_repo() { ), ".eval-magic-outputs/probe.txt" ); + // Git spells a zero UTC offset either `+00:00` (2.43) or `Z` (newer). + // Both name the same instant, so normalize rather than pin a version. + let log = git( + eval_root, + &["log", "-1", "--format=%an|%ae|%aI|%cn|%ce|%cI|%s"], + ) + .replace("+00:00", "Z"); assert_eq!( - git( - eval_root, - &["log", "-1", "--format=%an|%ae|%aI|%cn|%ce|%cI|%s"] - ), + log, "eval-magic|eval-magic@localhost|2000-01-01T00:00:00Z|\ eval-magic|eval-magic@localhost|2000-01-01T00:00:00Z|\ eval-magic task baseline" diff --git a/tests/run/main.rs b/tests/run/main.rs index f94b25f..d369dc8 100644 --- a/tests/run/main.rs +++ b/tests/run/main.rs @@ -14,6 +14,7 @@ mod byoh; mod claude_cli; mod cline; mod cline_permission_denials; +mod codebase; mod codex; mod codex_guard; mod codex_permission_denials;