From a343cdbd4017d2271d929a0a0e0a0bfaf3e0b344 Mon Sep 17 00:00:00 2001 From: samiamorwas Date: Sun, 16 Aug 2026 19:50:05 -0400 Subject: [PATCH 01/11] feat(evals): declare the codebase a task environment is built from An eval environment could only be assembled from individual fixture files copied out of `/evals/`, so there was no way to say "run this task against *this project*". Add a `codebase` block: a git `url` + required `ref`, or a local `path`. It is declarable at the config level as a default and overridable per eval, mirroring how `runs` already works. `ref` is required on a git source because the runner records the resolved SHA. An eval tracking a moving branch could not be re-run against the tree it actually measured, which is the whole point of recording provenance. The schema owns the structural contract, but `oneOf` cannot explain itself: a git source missing its `ref` reports only that the block matched neither branch, never naming `ref`. A small check ahead of the schema names the mistakes worth a sentence, and covers the whitespace-only case that `minLength: 1` admits. Refs #252 Co-Authored-By: Claude Opus 5 --- schema/evals.schema.json | 43 ++++++++++ src/cli/run/dispatch.rs | 1 + src/cli/run/fixtures.rs | 1 + src/cli/run/util.rs | 1 + src/core/types.rs | 33 ++++++++ src/validation/evals.rs | 169 +++++++++++++++++++++++++++++++++++++++ 6 files changed, 248 insertions(+) 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/src/cli/run/dispatch.rs b/src/cli/run/dispatch.rs index a13fe10..be1090f 100644 --- a/src/cli/run/dispatch.rs +++ b/src/cli/run/dispatch.rs @@ -522,6 +522,7 @@ mod tests { runs: None, isolation: None, turns: None, + codebase: None, }) .collect() } diff --git a/src/cli/run/fixtures.rs b/src/cli/run/fixtures.rs index 74928e4..3b2753e 100644 --- a/src/cli/run/fixtures.rs +++ b/src/cli/run/fixtures.rs @@ -199,6 +199,7 @@ mod tests { runs: None, isolation: None, turns: None, + codebase: None, } } 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..87f2978 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,36 @@ 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, + }, +} + /// 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, } @@ -442,6 +473,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 +497,7 @@ mod tests { runs: None, isolation: Some(Isolation::Isolated), turns: None, + codebase: None, }; let out = serde_json::to_value(&eval).unwrap(); assert_eq!( 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}"); + } } From 84cdd6b811f98f171b413e353c87aede39d49f3e Mon Sep 17 00:00:00 2001 From: samiamorwas Date: Sun, 16 Aug 2026 19:50:51 -0400 Subject: [PATCH 02/11] feat(source): resolve and materialize a declared source MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The codebase block needs turning into a real tree, and #253 needs the same machinery for skills, so this lands as a shared module rather than inline in the codebase path. Nothing in it knows what a codebase is. Two phases, deliberately split. `resolve` is read-only, so a run fails on an unreachable repository or a ref that does not exist before it has built any part of a workspace. `materialize` then clones a source that has history, or copies and initializes one that does not — either way the destination is a Git repository with no remote, since a task environment must not be able to reach the source it came from. Two details worth naming, both pinned by tests: `ls-remote` runs unfiltered. Passing a ref pattern suppresses the `ref: refs/heads/\tHEAD` line, and that line is the only way to learn the remote's default branch — which is where a tag or a bare SHA has to land, having no branch of its own. One unfiltered call answers both questions. An annotated tag resolves through `refs/tags/^{}` to the commit it peels to. The tag object itself is not a commit and cannot be checked out as one. A local path is materialized as a clean checkout of its committed state, so uncommitted work in the source is not carried; resolution warns when the source is dirty rather than letting that pass unnoticed. Refs #252 Co-Authored-By: Claude Opus 5 --- src/lib.rs | 1 + src/source/mod.rs | 739 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 740 insertions(+) create mode 100644 src/source/mod.rs 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/source/mod.rs b/src/source/mod.rs new file mode 100644 index 0000000..7c02e4a --- /dev/null +++ b/src/source/mod.rs @@ -0,0 +1,739 @@ +//! 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; + +use crate::core::run_git; + +/// 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 text = |args: &[&str]| { + let output = run_git(args, &directory); + (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())) + })?; + } + + 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( + dest.parent().unwrap_or(dest), + &[ + "init", + "--quiet", + "--initial-branch", + &resolved.branch, + &dest.to_string_lossy(), + ], + "initialize the codebase directory as a repository", + )?; + } + _ => clone_repository(resolved, dest)?, + } + Ok(()) +} + +fn clone_repository(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( + Path::new("."), + &[ + "clone", + "--quiet", + "--no-checkout", + &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( + dest, + &["checkout", "--quiet", "-B", &resolved.branch, revision], + &format!("check out {revision} of codebase {from}"), + )?; + checked( + 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(cwd: &Path, args: &[&str], intent: &str) -> Result<(), SourceError> { + let output = run_git(args, cwd); + 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 output = run_git(&["ls-remote", "--symref", url], Path::new(".")); + 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)] +mod tests { + 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"]), ""); + } +} From a01edcb6dca1764364b207f0d5ebcb6bddd037de Mon Sep 17 00:00:00 2001 From: samiamorwas Date: Sun, 16 Aug 2026 19:58:24 -0400 Subject: [PATCH 03/11] test(run): compare baseline commit dates across git versions The task-repository assertion pinned `%aI` as `2000-01-01T00:00:00Z`, but git renders a zero UTC offset as `+00:00` on 2.43 and `Z` only on newer versions. The test therefore passed on CI and failed on any host with the older git, for a difference in spelling rather than in behavior. Normalize the offset before comparing, so the assertion stays exact about the instant it cares about without pinning a git version. Co-Authored-By: Claude Opus 5 --- tests/run/git_isolation.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) 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" From 89552efeea4ec953108529b7d4a03880d34ddaa0 Mon Sep 17 00:00:00 2001 From: samiamorwas Date: Sun, 16 Aug 2026 19:58:39 -0400 Subject: [PATCH 04/11] feat(run): build task environments from a sourced codebase MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolution now happens before anything is created, so an unreachable repository or a ref that does not exist fails while the run has still built nothing. Each distinct codebase is materialized once per iteration and every `(group, condition, run)` environment is provisioned from that one tree; `files` is copied on top, making it an overlay on a real project rather than the whole of the environment. The task-repository lifecycle had two invariants a real codebase breaks by construction: it `git init`ed every environment from nothing, and it rejected any remote. A sourced environment now keeps the `.git` its clone brought, has its remotes stripped rather than asserted absent, and stays on the branch the codebase itself was on. A fixture-only environment still starts from `git init` on `work`, so evals that declare no codebase are untouched. Both kinds now mark their start state with `refs/eval-magic/baseline`, which #255 measures against. It sits outside `refs/heads/`, so it adds nothing to what the agent under test sees. The baseline `git add` drops `--force`. Forcing made sense when every file in the environment was one the runner had placed; against a real repository it would sweep `target/` or `node_modules/` into the state every run starts from. The add now respects the codebase's `.gitignore`, and the paths the runner placed — harness config directories and the fixture overlay — are forced in on top, so a codebase that ignores `.claude/` cannot hide the staged skill from the baseline and put the condition under test outside every later diff. Refs #252 Co-Authored-By: Claude Opus 5 --- src/cli/run/orchestrate/build.rs | 2 +- src/cli/run/orchestrate/git.rs | 189 ++++++++++++++++++++---- src/cli/run/orchestrate/mod.rs | 46 +++++- src/cli/run/orchestrate/resolve.rs | 69 ++++++++- src/cli/run/orchestrate/stage.rs | 45 +++++- tests/run/codebase.rs | 228 +++++++++++++++++++++++++++++ tests/run/main.rs | 1 + 7 files changed, 550 insertions(+), 30 deletions(-) create mode 100644 tests/run/codebase.rs diff --git a/src/cli/run/orchestrate/build.rs b/src/cli/run/orchestrate/build.rs index 1951009..f78214c 100644 --- a/src/cli/run/orchestrate/build.rs +++ b/src/cli/run/orchestrate/build.rs @@ -406,7 +406,7 @@ pub(super) fn post_build( // exist, but before project-local skill discovery inspects ancestor state. // Recreating `.git` also resets explicit iteration rebuilds to one clean, // runner-owned baseline with no inherited history or remotes. - super::git::initialize_task_repositories(r)?; + super::git::initialize_task_repositories(ctx, r)?; super::shadow_preflight::run(ctx, opts, r, staged, &targets)?; crate::pipeline::capture_iteration_baselines(&r.iteration_dir) diff --git a/src/cli/run/orchestrate/git.rs b/src/cli/run/orchestrate/git.rs index e6e2084..ced515a 100644 --- a/src/cli/run/orchestrate/git.rs +++ b/src/cli/run/orchestrate/git.rs @@ -5,14 +5,18 @@ use std::fs; use std::path::{Path, PathBuf}; use std::process::{Command, Output}; +use crate::adapters::registry::all_config_dir_names; use crate::core::{clear_git_environment, run_git}; +use crate::source::INITIALIZED_BRANCH; use super::super::RunError; +use super::super::fixtures::fixture_pairs; use super::Resolved; use super::envs::{EnvLayoutInput, env_targets}; use crate::core::RunContext; -const BASELINE_BRANCH: &str = "work"; +/// Marks the state every environment starts from, for later diffing. +const BASELINE_REF: &str = "refs/eval-magic/baseline"; const BASELINE_MESSAGE: &str = "eval-magic task baseline"; const BASELINE_NAME: &str = "eval-magic"; const BASELINE_EMAIL: &str = "eval-magic@localhost"; @@ -38,7 +42,10 @@ pub(super) fn preflight_git(ctx: &RunContext) -> 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..c9e9414 100644 --- a/src/cli/run/orchestrate/mod.rs +++ b/src/cli/run/orchestrate/mod.rs @@ -16,7 +16,8 @@ use std::path::PathBuf; use crate::adapters::{CliDispatchContext, adapter_for}; use crate::cli::command_target_args; -use crate::core::{Eval, Mode, RunContext}; +use crate::core::{CodebaseSource, Eval, Mode, RunContext}; +use crate::source::ResolvedSource; use super::RunError; use super::statistics::format_minimum_attainable_fisher_p_value; @@ -72,6 +73,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 +92,46 @@ 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 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/tests/run/codebase.rs b/tests/run/codebase.rs new file mode 100644 index 0000000..5f0778e --- /dev/null +++ b/tests/run/codebase.rs @@ -0,0 +1,228 @@ +//! 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}" + ); +} + +/// 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/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; From c621cd3c81192f567b4c5c361bc034095387af9f Mon Sep 17 00:00:00 2001 From: samiamorwas Date: Sun, 16 Aug 2026 20:00:42 -0400 Subject: [PATCH 05/11] fix(source): hold the operator's git configuration off a sourced codebase MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sourcing runs git against a URL from an eval config, on a host whose git configuration belongs to someone else. Inherited, that configuration decides things the runner has to decide itself. `url..insteadOf` is the sharp one: it rewrites the URL, so the tree sourced is not the tree the report cites — a silent wrong answer rather than a failure. `init.templateDir` is the quiet one: it seeds hooks into a repository the write guard assumes has none. Every invocation in the module now runs with system and global configuration switched off, the `GIT_CONFIG_COUNT` environment mechanism cleared, and an empty template directory passed to `clone` and `init`. Tested at the run boundary rather than in a unit test: the injection mechanism is process-global environment variables, which a unit test cannot set without racing every other test in the binary. Refs #252 Co-Authored-By: Claude Opus 5 --- src/source/git.rs | 82 +++++++++++++++++++++++++++++++++++++++++++ src/source/mod.rs | 34 ++++++++++++++---- tests/run/codebase.rs | 38 ++++++++++++++++++++ 3 files changed, 147 insertions(+), 7 deletions(-) create mode 100644 src/source/git.rs 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 index 7c02e4a..199280a 100644 --- a/src/source/mod.rs +++ b/src/source/mod.rs @@ -11,7 +11,9 @@ use std::path::Path; -use crate::core::run_git; +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 @@ -101,8 +103,9 @@ fn resolve_path(declared: &str, base_dir: &Path) -> Result Result<(), SourceE })?; } + 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) => { @@ -206,23 +210,32 @@ pub fn materialize(resolved: &ResolvedSource, dest: &Path) -> Result<(), SourceE }, )?; 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(resolved, dest)?, + _ => clone_repository(&git, resolved, dest)?, } Ok(()) } -fn clone_repository(resolved: &ResolvedSource, dest: &Path) -> Result<(), SourceError> { +fn clone_repository( + git: &IsolatedGit, + resolved: &ResolvedSource, + dest: &Path, +) -> Result<(), SourceError> { let from = resolved .resolved_path .clone() @@ -236,11 +249,15 @@ fn clone_repository(resolved: &ResolvedSource, dest: &Path) -> Result<(), Source // `--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(), ], @@ -249,11 +266,13 @@ fn clone_repository(resolved: &ResolvedSource, dest: &Path) -> Result<(), Source // `-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", @@ -262,8 +281,8 @@ fn clone_repository(resolved: &ResolvedSource, dest: &Path) -> Result<(), Source } /// Run git in `cwd`, turning a non-zero exit into an error naming the intent. -fn checked(cwd: &Path, args: &[&str], intent: &str) -> Result<(), SourceError> { - let output = run_git(args, cwd); +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(()); } @@ -301,7 +320,8 @@ fn default_branch(refs: &[(String, String)], url: &str) -> Result Result, SourceError> { - let output = run_git(&["ls-remote", "--symref", url], Path::new(".")); + 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}: {}", diff --git a/tests/run/codebase.rs b/tests/run/codebase.rs index 5f0778e..586f776 100644 --- a/tests/run/codebase.rs +++ b/tests/run/codebase.rs @@ -201,6 +201,44 @@ fn the_baseline_respects_codebase_gitignore_but_still_tracks_runner_files() { ); } +/// 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" + ); +} + /// The ticket's last acceptance criterion: an eval declaring no codebase keeps /// the environment it has always had. #[test] From 68f059987d8c395c0bb84601d3f6b4ae9d6cf2b9 Mon Sep 17 00:00:00 2001 From: samiamorwas Date: Mon, 17 Aug 2026 00:50:01 -0400 Subject: [PATCH 06/11] feat(run): record the resolved codebase in conditions and dispatch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A report that cites a codebase has to say which tree it measured, and the declared ref cannot say it — a branch moves. `conditions.json` now carries each distinct resolved codebase with the commit it resolved to and the evals built from it, and every dispatch task carries the same record, which is the route it takes to each run record. One shape, `CodebaseRecord`, is shared by every surface so a reader never has to reconcile two spellings of one resolution. A `path` source is flagged `host_local`. Another machine has that directory somewhere else, or nowhere, so a run citing it is not reproducible from the config alone. Nothing can fix that, so the artifact states it instead of implying a reproducibility it does not have — and where the directory is a repository, its `origin` is recorded too, since `origin_url` + `revision` does resolve anywhere. Fixture-only iterations serialize unchanged: the field is omitted when empty. Refs #252 Co-Authored-By: Claude Opus 5 --- src/cli/run/dispatch.rs | 11 ++++- src/cli/run/orchestrate/build.rs | 5 ++ src/cli/run/orchestrate/mod.rs | 38 ++++++++++++++- src/core/types.rs | 53 +++++++++++++++++++++ tests/run/codebase.rs | 81 ++++++++++++++++++++++++++++++++ 5 files changed, 185 insertions(+), 3 deletions(-) diff --git a/src/cli/run/dispatch.rs b/src/cli/run/dispatch.rs index be1090f..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, } +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. /// diff --git a/src/core/types.rs b/src/core/types.rs index 87f2978..218d005 100644 --- a/src/core/types.rs +++ b/src/core/types.rs @@ -170,6 +170,54 @@ pub enum CodebaseSource { }, } +/// 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 { @@ -248,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. @@ -590,6 +642,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/tests/run/codebase.rs b/tests/run/codebase.rs index 586f776..c6fb5f6 100644 --- a/tests/run/codebase.rs +++ b/tests/run/codebase.rs @@ -239,6 +239,87 @@ fn sourcing_a_codebase_ignores_the_operators_git_configuration() { ); } +/// 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( + &local, + &["remote", "add", "origin", &upstream.to_string_lossy()], + ); + 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"], wire_path(&upstream)); +} + /// The ticket's last acceptance criterion: an eval declaring no codebase keeps /// the environment it has always had. #[test] From 6da7031eeee43420bde390ae1cb05a620a2b4d7e Mon Sep 17 00:00:00 2001 From: samiamorwas Date: Mon, 17 Aug 2026 00:51:27 -0400 Subject: [PATCH 07/11] refactor(source): move the resolver's tests out of the module they exercise MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `mod.rs` reached 759 lines with 422 of them tests — the test module had grown larger than the implementation it covers. CLAUDE.md's rule is a size trigger, and this crossed it. No behavior change; the same twelve tests run from a sibling file. Co-Authored-By: Claude Opus 5 --- src/source/mod.rs | 423 +------------------------------------------ src/source/tests.rs | 424 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 426 insertions(+), 421 deletions(-) create mode 100644 src/source/tests.rs diff --git a/src/source/mod.rs b/src/source/mod.rs index 199280a..4417ef1 100644 --- a/src/source/mod.rs +++ b/src/source/mod.rs @@ -336,424 +336,5 @@ fn list_remote(url: &str) -> Result, SourceError> { } #[cfg(test)] -mod tests { - 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"]), ""); - } -} +#[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"]), ""); +} From 87c0797c1a733c314c9d17df99bdf5aefa431e44 Mon Sep 17 00:00:00 2001 From: samiamorwas Date: Mon, 17 Aug 2026 00:59:23 -0400 Subject: [PATCH 08/11] feat(pipeline): carry the resolved codebase to run, benchmark, and baseline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Provenance stopped at `conditions.json` and `dispatch.json`, which is short of where it is read. Grading consumes `run.json` and nothing else, so without the record there a result cannot be tied to a tree at the granularity that matters — the individual run. `benchmark.json` is the artifact a published comparison is read from. `BASELINE.md` is what someone reads when deciding whether to believe the claim. All three now carry it, and both schemas gain the property (each is `additionalProperties: false`, so the artifacts would otherwise fail their own validation). The `BASELINE.md` row names the resolved commit rather than the ref, since a branch has moved by the time the baseline is read. A host-local path says so in the cell and shows its origin URL, which is the part a reader elsewhere can actually resolve. Absent-when-empty throughout, so fixture-only artifacts are byte-identical. Refs #252 Co-Authored-By: Claude Opus 5 --- schema/benchmark.schema.json | 206 ++++++++++++++--- schema/run-record.schema.json | 244 +++++++++++++++++---- src/core/types.rs | 7 + src/pipeline/aggregate.rs | 9 +- src/pipeline/record_runs.rs | 8 +- src/pipeline/record_runs/tests/assembly.rs | 72 ++++++ src/workspace/promote.rs | 121 ++++++++++ tests/cli/aggregate/shadow.rs | 47 ++++ 8 files changed, 644 insertions(+), 70 deletions(-) diff --git a/schema/benchmark.schema.json b/schema/benchmark.schema.json index 7e49e82..4f2b9ae 100644 --- a/schema/benchmark.schema.json +++ b/schema/benchmark.schema.json @@ -15,27 +15,47 @@ ], "additionalProperties": false, "properties": { - "generated": { "type": "string", "description": "ISO timestamp" }, - "mode": { "type": "string", "enum": ["new-skill", "revision"] }, + "generated": { + "type": "string", + "description": "ISO timestamp" + }, + "mode": { + "type": "string", + "enum": [ + "new-skill", + "revision" + ] + }, "baseline": { - "type": ["string", "null"], + "type": [ + "string", + "null" + ], "description": "Baseline label for revision mode; omitted otherwise." }, "conditions_compared": { "type": "array", - "items": { "type": "string" }, + "items": { + "type": "string" + }, "minItems": 2, "maxItems": 2 }, - "missing_gradings": { "type": "integer" }, + "missing_gradings": { + "type": "integer" + }, "validity_warnings": { "type": "array", - "items": { "type": "string" } + "items": { + "type": "string" + } }, "run_summary": { "type": "object", "description": "Per-condition rollup, keyed by condition name.", - "additionalProperties": { "$ref": "#/definitions/conditionSummary" } + "additionalProperties": { + "$ref": "#/definitions/conditionSummary" + } }, "assertions": { "type": "object", @@ -44,7 +64,9 @@ "type": "object", "additionalProperties": { "type": "object", - "additionalProperties": { "$ref": "#/definitions/assertionCount" } + "additionalProperties": { + "$ref": "#/definitions/assertionCount" + } } } }, @@ -53,28 +75,108 @@ "description": "Raw final-environment diff metrics per condition, ordered by eval id and then run index. Omitted for iterations created before diff-scope capture.", "additionalProperties": { "type": "array", - "items": { "$ref": "#/definitions/diffScopeRun" } + "items": { + "$ref": "#/definitions/diffScopeRun" + } } }, "delta": { "type": "object", - "required": ["direction", "pass_rate", "duration_ms", "total_tokens"], + "required": [ + "direction", + "pass_rate", + "duration_ms", + "total_tokens" + ], "additionalProperties": false, "properties": { - "direction": { "type": "string" }, - "pass_rate": { "type": "number" }, - "duration_ms": { "type": "number" }, - "total_tokens": { "type": "number" } + "direction": { + "type": "string" + }, + "pass_rate": { + "type": "number" + }, + "duration_ms": { + "type": "number" + }, + "total_tokens": { + "type": "number" + } + } + }, + "codebases": { + "type": "array", + "description": "Codebases the compared iterations ran against, echoed from conditions.json. Absent for fixture-only iterations.", + "items": { + "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 + 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." + } + } } } }, "definitions": { "assertionCount": { "type": "object", - "required": ["passed", "n"], + "required": [ + "passed", + "n" + ], "additionalProperties": false, "properties": { - "passed": { "type": "integer", "minimum": 0 }, + "passed": { + "type": "integer", + "minimum": 0 + }, "n": { "type": "integer", "minimum": 1, @@ -84,7 +186,11 @@ }, "stats": { "type": "object", - "required": ["mean", "stddev", "n"], + "required": [ + "mean", + "stddev", + "n" + ], "additionalProperties": false, "properties": { "mean": { @@ -103,27 +209,67 @@ }, "conditionSummary": { "type": "object", - "required": ["pass_rate", "duration_ms", "total_tokens"], + "required": [ + "pass_rate", + "duration_ms", + "total_tokens" + ], "additionalProperties": false, "properties": { - "pass_rate": { "$ref": "#/definitions/stats" }, - "duration_ms": { "$ref": "#/definitions/stats" }, - "total_tokens": { "$ref": "#/definitions/stats" }, - "skill_invocation_n": { "type": "integer" }, - "skill_invocation_rate": { "type": ["number", "null"] } + "pass_rate": { + "$ref": "#/definitions/stats" + }, + "duration_ms": { + "$ref": "#/definitions/stats" + }, + "total_tokens": { + "$ref": "#/definitions/stats" + }, + "skill_invocation_n": { + "type": "integer" + }, + "skill_invocation_rate": { + "type": [ + "number", + "null" + ] + } } }, "diffScopeRun": { "type": "object", - "required": ["eval_id", "files_touched", "lines_added", "lines_removed", "hunks"], + "required": [ + "eval_id", + "files_touched", + "lines_added", + "lines_removed", + "hunks" + ], "additionalProperties": false, "properties": { - "eval_id": { "type": "string" }, - "run_index": { "type": "integer", "minimum": 1 }, - "files_touched": { "type": "integer", "minimum": 0 }, - "lines_added": { "type": "integer", "minimum": 0 }, - "lines_removed": { "type": "integer", "minimum": 0 }, - "hunks": { "type": "integer", "minimum": 0 } + "eval_id": { + "type": "string" + }, + "run_index": { + "type": "integer", + "minimum": 1 + }, + "files_touched": { + "type": "integer", + "minimum": 0 + }, + "lines_added": { + "type": "integer", + "minimum": 0 + }, + "lines_removed": { + "type": "integer", + "minimum": 0 + }, + "hunks": { + "type": "integer", + "minimum": 0 + } } } } diff --git a/schema/run-record.schema.json b/schema/run-record.schema.json index 1bd6949..9d28e27 100644 --- a/schema/run-record.schema.json +++ b/schema/run-record.schema.json @@ -2,7 +2,7 @@ "$schema": "http://json-schema.org/draft-07/schema#", "$id": "https://slow-powers.dev/schemas/run-record.schema.json", "title": "Portable Run Record", - "description": "Captures one subagent run. Harness-agnostic — each harness writes an adapter from its native transcript format to this shape. Downstream grading reads only this file.", + "description": "Captures one subagent run. Harness-agnostic \u2014 each harness writes an adapter from its native transcript format to this shape. Downstream grading reads only this file.", "type": "object", "required": [ "eval_id", @@ -24,7 +24,10 @@ "description": "Reserved names: with_skill, without_skill, old_skill, new_skill." }, "skill_path": { - "type": ["string", "null"], + "type": [ + "string", + "null" + ], "description": "Absolute path to the SKILL.md the subagent could load, or null if no skill was provided (without_skill condition)." }, "prompt": { @@ -33,7 +36,9 @@ }, "files": { "type": "array", - "items": { "type": "string" }, + "items": { + "type": "string" + }, "description": "Fixture files the subagent had access to (absolute paths inside the run's workspace)." }, "final_message": { @@ -45,7 +50,10 @@ "description": "Ordered list of tool calls during the run.", "items": { "type": "object", - "required": ["name", "ordinal"], + "required": [ + "name", + "ordinal" + ], "additionalProperties": false, "properties": { "name": { @@ -54,11 +62,20 @@ }, "args": { "description": "Tool arguments. Object for structured tools, string for raw command-style tools.", - "type": ["object", "string", "array", "null"] + "type": [ + "object", + "string", + "array", + "null" + ] }, "result": { "description": "Tool output, if captured. Truncate long outputs to ~2KB.", - "type": ["string", "object", "null"] + "type": [ + "string", + "object", + "null" + ] }, "ordinal": { "type": "integer", @@ -69,11 +86,17 @@ } }, "total_tokens": { - "type": ["integer", "null"], + "type": [ + "integer", + "null" + ], "description": "From the harness's task completion event, or derived from the persisted transcript by record-runs using harness-specific normalization. Canonical timing lives in the sibling timing.json, whose `source` field records which origin produced it. May be null if neither source is available." }, "duration_ms": { - "type": ["integer", "null"], + "type": [ + "integer", + "null" + ], "description": "From the harness's task completion event, a native duration field, or enough persisted transcript timestamps to derive wall-clock time. Canonical timing lives in the sibling timing.json. May be null when the harness does not expose reliable timing." }, "run_index": { @@ -84,17 +107,72 @@ "conversation": { "$ref": "#/definitions/conversation", "description": "Ordered multi-turn evidence and scripted-delivery outcome. Absent for one-shot runs." + }, + "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 + 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." + } + }, + "description": "The codebase this run's environment was built from. Absent for a fixture-only run." } }, "definitions": { "conversation": { "type": "object", - "required": ["status", "delivered_followups", "events"], + "required": [ + "status", + "delivered_followups", + "events" + ], "additionalProperties": false, "properties": { "status": { "type": "string", - "enum": ["completed", "stopped"] + "enum": [ + "completed", + "stopped" + ] }, "delivered_followups": { "type": "integer", @@ -102,7 +180,10 @@ }, "stop_reason": { "type": "string", - "enum": ["agent_did_not_ask", "agent_response_mismatch"] + "enum": [ + "agent_did_not_ask", + "agent_response_mismatch" + ] }, "stopped_before_followup": { "type": "integer", @@ -113,9 +194,15 @@ "minItems": 2, "items": { "oneOf": [ - { "$ref": "#/definitions/userMessage" }, - { "$ref": "#/definitions/assistantMessage" }, - { "$ref": "#/definitions/conversationTool" } + { + "$ref": "#/definitions/userMessage" + }, + { + "$ref": "#/definitions/assistantMessage" + }, + { + "$ref": "#/definitions/conversationTool" + } ] } } @@ -123,23 +210,46 @@ "allOf": [ { "if": { - "properties": { "status": { "const": "stopped" } }, - "required": ["status"] + "properties": { + "status": { + "const": "stopped" + } + }, + "required": [ + "status" + ] }, "then": { - "required": ["stop_reason", "stopped_before_followup"] + "required": [ + "stop_reason", + "stopped_before_followup" + ] } }, { "if": { - "properties": { "status": { "const": "completed" } }, - "required": ["status"] + "properties": { + "status": { + "const": "completed" + } + }, + "required": [ + "status" + ] }, "then": { "not": { "anyOf": [ - { "required": ["stop_reason"] }, - { "required": ["stopped_before_followup"] } + { + "required": [ + "stop_reason" + ] + }, + { + "required": [ + "stopped_before_followup" + ] + } ] } } @@ -148,37 +258,95 @@ }, "userMessage": { "type": "object", - "required": ["type", "ordinal", "round", "text"], + "required": [ + "type", + "ordinal", + "round", + "text" + ], "additionalProperties": false, "properties": { - "type": { "const": "user_message" }, - "ordinal": { "type": "integer", "minimum": 0 }, - "round": { "type": "integer", "minimum": 1 }, - "text": { "type": "string" } + "type": { + "const": "user_message" + }, + "ordinal": { + "type": "integer", + "minimum": 0 + }, + "round": { + "type": "integer", + "minimum": 1 + }, + "text": { + "type": "string" + } } }, "assistantMessage": { "type": "object", - "required": ["type", "ordinal", "round", "text"], + "required": [ + "type", + "ordinal", + "round", + "text" + ], "additionalProperties": false, "properties": { - "type": { "const": "assistant_message" }, - "ordinal": { "type": "integer", "minimum": 0 }, - "round": { "type": "integer", "minimum": 1 }, - "text": { "type": "string" } + "type": { + "const": "assistant_message" + }, + "ordinal": { + "type": "integer", + "minimum": 0 + }, + "round": { + "type": "integer", + "minimum": 1 + }, + "text": { + "type": "string" + } } }, "conversationTool": { "type": "object", - "required": ["type", "ordinal", "round", "name"], + "required": [ + "type", + "ordinal", + "round", + "name" + ], "additionalProperties": false, "properties": { - "type": { "const": "tool_invocation" }, - "ordinal": { "type": "integer", "minimum": 0 }, - "round": { "type": "integer", "minimum": 1 }, - "name": { "type": "string" }, - "args": { "type": ["object", "string", "array", "null"] }, - "result": { "type": ["string", "object", "null"] } + "type": { + "const": "tool_invocation" + }, + "ordinal": { + "type": "integer", + "minimum": 0 + }, + "round": { + "type": "integer", + "minimum": 1 + }, + "name": { + "type": "string" + }, + "args": { + "type": [ + "object", + "string", + "array", + "null" + ] + }, + "result": { + "type": [ + "string", + "object", + "null" + ] + } } } } diff --git a/src/core/types.rs b/src/core/types.rs index 218d005..cb652c3 100644 --- a/src/core/types.rs +++ b/src/core/types.rs @@ -344,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. @@ -574,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. 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/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"); +} From 0dea5701d02842fa09dadffcda07c09cf474bd6f Mon Sep 17 00:00:00 2001 From: samiamorwas Date: Mon, 17 Aug 2026 01:02:58 -0400 Subject: [PATCH 09/11] docs: ship a guide for sourcing a codebase The codebase block has no CLI flag, so `--help` cannot carry its rules and a config author has nowhere to discover them. It gets its own shipped topic, which `build.rs` picks up from `docs/guides/`. The guide leads with the parts that cannot be inferred from the schema: that a git ref is mandatory and why, that `files` layers over the checkout rather than replacing it, what the resulting repository looks like, and that a local path is not reproducible by anyone reading the published results. The isolation guide gains a short section separating the two boundaries it would otherwise be read as covering: what a dispatch can load is not what a dispatch can reach. Co-Authored-By: Claude Opus 5 --- docs/developer_overview.md | 5 ++ docs/guides/codebase.md | 123 +++++++++++++++++++++++++++++++++++++ docs/guides/isolation.md | 11 ++++ src/cli/help.rs | 5 ++ tests/cli/docs.rs | 19 ++++++ 5 files changed, 163 insertions(+) create mode 100644 docs/guides/codebase.md 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/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/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() { From 05eedf08645097a9329abead38a61447853ded57 Mon Sep 17 00:00:00 2001 From: samiamorwas Date: Mon, 17 Aug 2026 01:04:28 -0400 Subject: [PATCH 10/11] fix(schema): keep the codebase additions to the schemas additive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adding the property programmatically rewrote both files: every compact one-line object was expanded, and the em dash in the run-record title was escaped to `—` — a content change to a shipped description, buried in 450 lines of formatting churn. Hand-written now, in the surrounding style. Both diffs are additions only. Co-Authored-By: Claude Opus 5 --- schema/benchmark.schema.json | 243 +++++++++---------------------- schema/run-record.schema.json | 265 +++++++++------------------------- 2 files changed, 135 insertions(+), 373 deletions(-) diff --git a/schema/benchmark.schema.json b/schema/benchmark.schema.json index 4f2b9ae..fbe4464 100644 --- a/schema/benchmark.schema.json +++ b/schema/benchmark.schema.json @@ -15,47 +15,27 @@ ], "additionalProperties": false, "properties": { - "generated": { - "type": "string", - "description": "ISO timestamp" - }, - "mode": { - "type": "string", - "enum": [ - "new-skill", - "revision" - ] - }, + "generated": { "type": "string", "description": "ISO timestamp" }, + "mode": { "type": "string", "enum": ["new-skill", "revision"] }, "baseline": { - "type": [ - "string", - "null" - ], + "type": ["string", "null"], "description": "Baseline label for revision mode; omitted otherwise." }, "conditions_compared": { "type": "array", - "items": { - "type": "string" - }, + "items": { "type": "string" }, "minItems": 2, "maxItems": 2 }, - "missing_gradings": { - "type": "integer" - }, + "missing_gradings": { "type": "integer" }, "validity_warnings": { "type": "array", - "items": { - "type": "string" - } + "items": { "type": "string" } }, "run_summary": { "type": "object", "description": "Per-condition rollup, keyed by condition name.", - "additionalProperties": { - "$ref": "#/definitions/conditionSummary" - } + "additionalProperties": { "$ref": "#/definitions/conditionSummary" } }, "assertions": { "type": "object", @@ -64,119 +44,78 @@ "type": "object", "additionalProperties": { "type": "object", - "additionalProperties": { - "$ref": "#/definitions/assertionCount" - } + "additionalProperties": { "$ref": "#/definitions/assertionCount" } } } }, + "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.", "additionalProperties": { "type": "array", - "items": { - "$ref": "#/definitions/diffScopeRun" - } + "items": { "$ref": "#/definitions/diffScopeRun" } } }, "delta": { "type": "object", - "required": [ - "direction", - "pass_rate", - "duration_ms", - "total_tokens" - ], + "required": ["direction", "pass_rate", "duration_ms", "total_tokens"], "additionalProperties": false, "properties": { - "direction": { - "type": "string" + "direction": { "type": "string" }, + "pass_rate": { "type": "number" }, + "duration_ms": { "type": "number" }, + "total_tokens": { "type": "number" } + } + } + }, + "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." }, - "pass_rate": { - "type": "number" + "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." }, - "duration_ms": { - "type": "number" + "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." }, - "total_tokens": { - "type": "number" + "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." } } }, - "codebases": { - "type": "array", - "description": "Codebases the compared iterations ran against, echoed from conditions.json. Absent for fixture-only iterations.", - "items": { - "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 + 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." - } - } - } - } - }, - "definitions": { "assertionCount": { "type": "object", - "required": [ - "passed", - "n" - ], + "required": ["passed", "n"], "additionalProperties": false, "properties": { - "passed": { - "type": "integer", - "minimum": 0 - }, + "passed": { "type": "integer", "minimum": 0 }, "n": { "type": "integer", "minimum": 1, @@ -186,11 +125,7 @@ }, "stats": { "type": "object", - "required": [ - "mean", - "stddev", - "n" - ], + "required": ["mean", "stddev", "n"], "additionalProperties": false, "properties": { "mean": { @@ -209,67 +144,27 @@ }, "conditionSummary": { "type": "object", - "required": [ - "pass_rate", - "duration_ms", - "total_tokens" - ], + "required": ["pass_rate", "duration_ms", "total_tokens"], "additionalProperties": false, "properties": { - "pass_rate": { - "$ref": "#/definitions/stats" - }, - "duration_ms": { - "$ref": "#/definitions/stats" - }, - "total_tokens": { - "$ref": "#/definitions/stats" - }, - "skill_invocation_n": { - "type": "integer" - }, - "skill_invocation_rate": { - "type": [ - "number", - "null" - ] - } + "pass_rate": { "$ref": "#/definitions/stats" }, + "duration_ms": { "$ref": "#/definitions/stats" }, + "total_tokens": { "$ref": "#/definitions/stats" }, + "skill_invocation_n": { "type": "integer" }, + "skill_invocation_rate": { "type": ["number", "null"] } } }, "diffScopeRun": { "type": "object", - "required": [ - "eval_id", - "files_touched", - "lines_added", - "lines_removed", - "hunks" - ], + "required": ["eval_id", "files_touched", "lines_added", "lines_removed", "hunks"], "additionalProperties": false, "properties": { - "eval_id": { - "type": "string" - }, - "run_index": { - "type": "integer", - "minimum": 1 - }, - "files_touched": { - "type": "integer", - "minimum": 0 - }, - "lines_added": { - "type": "integer", - "minimum": 0 - }, - "lines_removed": { - "type": "integer", - "minimum": 0 - }, - "hunks": { - "type": "integer", - "minimum": 0 - } + "eval_id": { "type": "string" }, + "run_index": { "type": "integer", "minimum": 1 }, + "files_touched": { "type": "integer", "minimum": 0 }, + "lines_added": { "type": "integer", "minimum": 0 }, + "lines_removed": { "type": "integer", "minimum": 0 }, + "hunks": { "type": "integer", "minimum": 0 } } } } diff --git a/schema/run-record.schema.json b/schema/run-record.schema.json index 9d28e27..4acc7b4 100644 --- a/schema/run-record.schema.json +++ b/schema/run-record.schema.json @@ -2,7 +2,7 @@ "$schema": "http://json-schema.org/draft-07/schema#", "$id": "https://slow-powers.dev/schemas/run-record.schema.json", "title": "Portable Run Record", - "description": "Captures one subagent run. Harness-agnostic \u2014 each harness writes an adapter from its native transcript format to this shape. Downstream grading reads only this file.", + "description": "Captures one subagent run. Harness-agnostic — each harness writes an adapter from its native transcript format to this shape. Downstream grading reads only this file.", "type": "object", "required": [ "eval_id", @@ -24,10 +24,7 @@ "description": "Reserved names: with_skill, without_skill, old_skill, new_skill." }, "skill_path": { - "type": [ - "string", - "null" - ], + "type": ["string", "null"], "description": "Absolute path to the SKILL.md the subagent could load, or null if no skill was provided (without_skill condition)." }, "prompt": { @@ -36,9 +33,7 @@ }, "files": { "type": "array", - "items": { - "type": "string" - }, + "items": { "type": "string" }, "description": "Fixture files the subagent had access to (absolute paths inside the run's workspace)." }, "final_message": { @@ -50,10 +45,7 @@ "description": "Ordered list of tool calls during the run.", "items": { "type": "object", - "required": [ - "name", - "ordinal" - ], + "required": ["name", "ordinal"], "additionalProperties": false, "properties": { "name": { @@ -62,20 +54,11 @@ }, "args": { "description": "Tool arguments. Object for structured tools, string for raw command-style tools.", - "type": [ - "object", - "string", - "array", - "null" - ] + "type": ["object", "string", "array", "null"] }, "result": { "description": "Tool output, if captured. Truncate long outputs to ~2KB.", - "type": [ - "string", - "object", - "null" - ] + "type": ["string", "object", "null"] }, "ordinal": { "type": "integer", @@ -86,17 +69,11 @@ } }, "total_tokens": { - "type": [ - "integer", - "null" - ], + "type": ["integer", "null"], "description": "From the harness's task completion event, or derived from the persisted transcript by record-runs using harness-specific normalization. Canonical timing lives in the sibling timing.json, whose `source` field records which origin produced it. May be null if neither source is available." }, "duration_ms": { - "type": [ - "integer", - "null" - ], + "type": ["integer", "null"], "description": "From the harness's task completion event, a native duration field, or enough persisted transcript timestamps to derive wall-clock time. Canonical timing lives in the sibling timing.json. May be null when the harness does not expose reliable timing." }, "run_index": { @@ -109,70 +86,19 @@ "description": "Ordered multi-turn evidence and scripted-delivery outcome. Absent for one-shot runs." }, "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 + 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." - } - }, + "$ref": "#/definitions/codebase", "description": "The codebase this run's environment was built from. Absent for a fixture-only run." } }, "definitions": { "conversation": { "type": "object", - "required": [ - "status", - "delivered_followups", - "events" - ], + "required": ["status", "delivered_followups", "events"], "additionalProperties": false, "properties": { "status": { "type": "string", - "enum": [ - "completed", - "stopped" - ] + "enum": ["completed", "stopped"] }, "delivered_followups": { "type": "integer", @@ -180,10 +106,7 @@ }, "stop_reason": { "type": "string", - "enum": [ - "agent_did_not_ask", - "agent_response_mismatch" - ] + "enum": ["agent_did_not_ask", "agent_response_mismatch"] }, "stopped_before_followup": { "type": "integer", @@ -194,15 +117,9 @@ "minItems": 2, "items": { "oneOf": [ - { - "$ref": "#/definitions/userMessage" - }, - { - "$ref": "#/definitions/assistantMessage" - }, - { - "$ref": "#/definitions/conversationTool" - } + { "$ref": "#/definitions/userMessage" }, + { "$ref": "#/definitions/assistantMessage" }, + { "$ref": "#/definitions/conversationTool" } ] } } @@ -210,46 +127,23 @@ "allOf": [ { "if": { - "properties": { - "status": { - "const": "stopped" - } - }, - "required": [ - "status" - ] + "properties": { "status": { "const": "stopped" } }, + "required": ["status"] }, "then": { - "required": [ - "stop_reason", - "stopped_before_followup" - ] + "required": ["stop_reason", "stopped_before_followup"] } }, { "if": { - "properties": { - "status": { - "const": "completed" - } - }, - "required": [ - "status" - ] + "properties": { "status": { "const": "completed" } }, + "required": ["status"] }, "then": { "not": { "anyOf": [ - { - "required": [ - "stop_reason" - ] - }, - { - "required": [ - "stopped_before_followup" - ] - } + { "required": ["stop_reason"] }, + { "required": ["stopped_before_followup"] } ] } } @@ -258,96 +152,69 @@ }, "userMessage": { "type": "object", - "required": [ - "type", - "ordinal", - "round", - "text" - ], + "required": ["type", "ordinal", "round", "text"], "additionalProperties": false, "properties": { - "type": { - "const": "user_message" - }, - "ordinal": { - "type": "integer", - "minimum": 0 - }, - "round": { - "type": "integer", - "minimum": 1 - }, - "text": { - "type": "string" - } + "type": { "const": "user_message" }, + "ordinal": { "type": "integer", "minimum": 0 }, + "round": { "type": "integer", "minimum": 1 }, + "text": { "type": "string" } } }, "assistantMessage": { "type": "object", - "required": [ - "type", - "ordinal", - "round", - "text" - ], + "required": ["type", "ordinal", "round", "text"], "additionalProperties": false, "properties": { - "type": { - "const": "assistant_message" - }, - "ordinal": { - "type": "integer", - "minimum": 0 - }, - "round": { - "type": "integer", - "minimum": 1 - }, - "text": { - "type": "string" - } + "type": { "const": "assistant_message" }, + "ordinal": { "type": "integer", "minimum": 0 }, + "round": { "type": "integer", "minimum": 1 }, + "text": { "type": "string" } } }, - "conversationTool": { + "codebase": { "type": "object", - "required": [ - "type", - "ordinal", - "round", - "name" - ], + "required": ["kind", "source", "branch"], "additionalProperties": false, "properties": { - "type": { - "const": "tool_invocation" - }, - "ordinal": { - "type": "integer", - "minimum": 0 + "kind": { + "type": "string", + "enum": ["git", "path"], + "description": "Whether the codebase came from a repository URL or a directory on the host that ran it." }, - "round": { - "type": "integer", - "minimum": 1 + "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." }, - "name": { - "type": "string" + "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." }, - "args": { - "type": [ - "object", - "string", - "array", - "null" - ] + "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." }, - "result": { - "type": [ - "string", - "object", - "null" - ] + "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"], + "additionalProperties": false, + "properties": { + "type": { "const": "tool_invocation" }, + "ordinal": { "type": "integer", "minimum": 0 }, + "round": { "type": "integer", "minimum": 1 }, + "name": { "type": "string" }, + "args": { "type": ["object", "string", "array", "null"] }, + "result": { "type": ["string", "object", "null"] } + } } } } From 1459a499ef2547790f322978f613d7443981ef0c Mon Sep 17 00:00:00 2001 From: samiamorwas Date: Mon, 17 Aug 2026 02:18:45 -0400 Subject: [PATCH 11/11] test(codebase): assert the cited origin in the host's own spelling The origin-citation test registered the remote with the host's native path separators but asserted against the forward-slash form. On Linux those are one string, so the mismatch was invisible; on Windows the assertion compared a backslash path against a slash path and failed. Git stores a remote URL byte-for-byte and eval-magic cites it unchanged, so the fix is to hold both ends to the host's own spelling. That also makes the assertion load-bearing on Windows: any separator normalization between the source repo and conditions.json now shows up here rather than passing. Co-Authored-By: Claude Opus 5 --- tests/run/codebase.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/run/codebase.rs b/tests/run/codebase.rs index c6fb5f6..146cfe3 100644 --- a/tests/run/codebase.rs +++ b/tests/run/codebase.rs @@ -294,10 +294,12 @@ 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( - &local, - &["remote", "add", "origin", &upstream.to_string_lossy()], - ); + // 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)); @@ -317,7 +319,7 @@ fn a_path_codebase_is_recorded_as_host_local_with_its_origin_for_citation() { 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"], wire_path(&upstream)); + assert_eq!(recorded["origin_url"], origin_url); } /// The ticket's last acceptance criterion: an eval declaring no codebase keeps