From 0a13e813593563a710042ccd1d5056b7e57eea7b Mon Sep 17 00:00:00 2001 From: samiamorwas Date: Tue, 1 Sep 2026 01:24:14 -0400 Subject: [PATCH] fix(run): hide framework-staged files from the codebase's own tooling (#296) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Staged skills live inside the task repository, under the harness's skills dir. A codebase whose lint or format step globs the whole tree therefore reported eval-magic's own artifacts as project failures — and only in the arm that stages a skill, because the control arm has nothing to find. That is asymmetric by construction. Any `command_check` running the project's checks was biased against `with_skill`, and worse, the agent under test runs those same checks: it saw a permanently red step listing files it did not create and must not touch. A run meant to measure honest self-verification was measuring the framework instead. So `run` now writes a delimited block into the project's own ignore files, naming the paths the runner placed: # >>> eval-magic framework files >>> /.eval-magic-outputs/ /.claude/skills/ /.claude/settings.local.json # <<< eval-magic framework files <<< The paths come from the selected harness descriptor — `skills_dir` and the file its guard stages (`hooks_file`, or `plugin_file` for a plugin engine) — plus the framework outputs dir, so a BYOH harness contributes its own without configuring anything. The block is written into every environment: both arms, every repetition, `--no-stage` and `--dry-run` alike. An entry present in one arm only would trade one asymmetry for another. It lands before the baseline commit, so it never appears in a run's diff-scope. Which ignore files receive it is detected from the codebase's tooling through a new packaged `ignore-profiles/` family, embedded by `build.rs` the way `guard-profiles/` already is: prettier, eslint, stylelint, markdownlint, and docker, each declaring its markers and whether its file may be created when the project has none. ESLint's may not — ESLint 9's flat config no longer reads `.eslintignore`, so creating one would be inert. Both profile families now detect through one walk, `src/core/tree_profiles.rs`. `codebase.ignore_files` names the files outright when detection cannot find them; `[]` opts out. Paths may not escape the environment. `.gitignore` is deliberately never a target. The baseline force-adds harness config dirs, so an entry there would hide nothing from Git — but it would hide the staged skills from every `.gitignore`-aware tool the agent uses, such as `rg`, damaging the treatment arm instead of protecting it. Nothing records the effective list in an artifact either: the written file is committed into each environment's baseline, so `git show refs/eval-magic/baseline:.prettierignore` is the evidence, per arm and per run. Before / after, the shipped Weeknight fixture (`.prettierrc.json`, no `.prettierignore`), `npm run lint` in `env-g1-with_skill`: [warn] .claude/skills/slow-powers-eval-1-with_skill__demo/SKILL.md [warn] Code style issues found in the above file. exit 1 All matched files use Prettier code style! exit 0 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_018H2oD4ry5huonXVyrpDGFj --- build.rs | 34 +- docs/developer_overview.md | 4 + docs/guides/byoh.md | 5 + docs/guides/codebase.md | 67 ++++ harnesses/template.toml | 3 + ignore-profiles/tool-docker.toml | 8 + ignore-profiles/tool-eslint.toml | 9 + ignore-profiles/tool-markdownlint.toml | 15 + ignore-profiles/tool-prettier.toml | 16 + ignore-profiles/tool-stylelint.toml | 6 + schema/evals.schema.json | 10 + src/adapters/descriptor_adapter.rs | 13 + src/adapters/guard.rs | 15 +- src/adapters/harness.rs | 56 +++ src/cli/args.rs | 8 + src/cli/run/orchestrate/build.rs | 18 +- src/cli/run/orchestrate/build/ignore.rs | 46 +++ src/core/mod.rs | 1 + src/core/tree_profiles.rs | 250 +++++++++++++ src/core/types.rs | 16 + src/sandbox/guard_profiles.rs | 108 +----- src/validation/evals.rs | 29 ++ src/validation/evals/codebase_tests.rs | 67 ++++ src/workspace/mod.rs | 2 + src/workspace/tool_ignore.rs | 479 ++++++++++++++++++++++++ tests/cli/docs/codebase.rs | 19 + tests/run/ignore_files.rs | 156 ++++++++ tests/run/main.rs | 1 + 28 files changed, 1357 insertions(+), 104 deletions(-) create mode 100644 ignore-profiles/tool-docker.toml create mode 100644 ignore-profiles/tool-eslint.toml create mode 100644 ignore-profiles/tool-markdownlint.toml create mode 100644 ignore-profiles/tool-prettier.toml create mode 100644 ignore-profiles/tool-stylelint.toml create mode 100644 src/cli/run/orchestrate/build/ignore.rs create mode 100644 src/core/tree_profiles.rs create mode 100644 src/workspace/tool_ignore.rs create mode 100644 tests/run/ignore_files.rs diff --git a/build.rs b/build.rs index d3316ae..18f0ae1 100644 --- a/build.rs +++ b/build.rs @@ -23,37 +23,49 @@ fn main() { fs::write(out_dir.join("guide_topics.rs"), generated) .expect("failed to write generated guide topic table"); - let profile_dir = manifest_dir.join("guard-profiles"); - println!("cargo:rerun-if-changed={}", profile_dir.display()); - let generated = render_guard_profiles(&profile_dir); - fs::write(out_dir.join("guard_profiles.rs"), generated) - .expect("failed to write generated guard profile table"); + embed_profiles( + &manifest_dir.join("guard-profiles"), + &out_dir.join("guard_profiles.rs"), + "PACKAGED_GUARD_PROFILES", + ); + embed_profiles( + &manifest_dir.join("ignore-profiles"), + &out_dir.join("ignore_profiles.rs"), + "PACKAGED_IGNORE_PROFILES", + ); } -fn render_guard_profiles(profile_dir: &Path) -> String { +/// Render one directory of TOML profiles as a `&[(filename, body)]` table the +/// crate `include!`s. Both packaged profile families — guard command policy and +/// tool-ignore targets — ship this way, so a new profile is a new file and +/// nothing else. +fn embed_profiles(profile_dir: &Path, out_file: &Path, table_name: &str) { + println!("cargo:rerun-if-changed={}", profile_dir.display()); let mut profiles: Vec = fs::read_dir(profile_dir) .unwrap_or_else(|err| panic!("failed to read {}: {err}", profile_dir.display())) - .map(|entry| entry.expect("failed to read guard profile entry").path()) + .map(|entry| entry.expect("failed to read profile entry").path()) .filter(|path| path.extension().and_then(|value| value.to_str()) == Some("toml")) .collect(); profiles.sort(); assert!( !profiles.is_empty(), - "guard-profiles must contain a TOML profile" + "{} must contain a TOML profile", + profile_dir.display() ); - let mut generated = String::from("const PACKAGED_GUARD_PROFILES: &[(&str, &str)] = &[\n"); + let mut generated = format!("const {table_name}: &[(&str, &str)] = &[\n"); for path in profiles { let name = path .file_name() .and_then(|value| value.to_str()) - .unwrap_or_else(|| panic!("guard profile path is not UTF-8: {}", path.display())); + .unwrap_or_else(|| panic!("profile path is not UTF-8: {}", path.display())); let body = fs::read_to_string(&path) .unwrap_or_else(|err| panic!("failed to read {}: {err}", path.display())); generated.push_str(&format!(" ({name:?}, {body:?}),\n")); } generated.push_str("];\n"); - generated + fs::write(out_file, generated) + .unwrap_or_else(|err| panic!("failed to write {}: {err}", out_file.display())); } fn discover_guides(guide_dir: &Path) -> Vec { diff --git a/docs/developer_overview.md b/docs/developer_overview.md index aee2ff2..0963ea2 100644 --- a/docs/developer_overview.md +++ b/docs/developer_overview.md @@ -54,6 +54,10 @@ preconditions, handoffs, and recovery commands. - `schema/` contains the JSON schemas for user input and generated artifacts. - `harnesses/` contains built-in descriptors, descriptor scaffolding, and embedded harness assets. - `guard-profiles/` contains packaged command-policy defaults discovered and embedded by `build.rs`. +- `ignore-profiles/` contains packaged tool-ignore targets, discovered and embedded the same way: + which ignore file a detected formatter or linter reads, so `run` can hide the framework's own + staged files from a sourced codebase's tooling. Both families detect through + `src/core/tree_profiles.rs`. - `profiles/` contains shared prompt profiles. - `tests/cli/` covers CLI and packaging contracts; `tests/run/` covers campaign behavior across the run boundary. Focused unit tests normally live beside the implementation. diff --git a/docs/guides/byoh.md b/docs/guides/byoh.md index b377c6a..62a3fe3 100644 --- a/docs/guides/byoh.md +++ b/docs/guides/byoh.md @@ -128,6 +128,11 @@ path must be normalized, `/`-separated, and relative to the task repository. Its also appear in `config_dirs`, keeping discovery, sibling filtering, and task-repository baselining on one descriptor surface. +`skills_dir` and the file your `[guard]` section stages (`hooks_file`, or `plugin_file` for a +plugin engine) are also what `run` hides from a sourced codebase's own linters and formatters, by +writing them into the project's ignore files. Declaring them correctly is all a descriptor has to +do; see `eval-magic docs codebase`. + ## Layer descriptors by field Descriptors load in this order: diff --git a/docs/guides/codebase.md b/docs/guides/codebase.md index 32e1c8d..b71801d 100644 --- a/docs/guides/codebase.md +++ b/docs/guides/codebase.md @@ -172,6 +172,73 @@ cleanup. An explicit `--stage-name` remains stricter and refuses to clobber an o The effective `exclude_skill_sources` value is recorded with each codebase in `conditions.json`, every task in `dispatch.json`, every `run.json`, `benchmark.json`, and promoted `BASELINE.md`. +## Framework files stay out of the project's own tooling + +Staged skills live *inside* the task repository, under the harness's skills dir. A project whose +lint or format step globs the whole tree would otherwise report eval-magic's own artifacts as +project failures — and only in the arm that stages a skill, because the control arm has no staged +skills to find. That biases every `command_check` running the project's checks, and it hands the +agent under test a red check listing files it did not create and must not touch. + +So `run` writes a delimited block into the project's own ignore files, naming the paths the runner +placed: + +``` +# >>> eval-magic framework files >>> +# Staged by `eval-magic run` so this project's own tooling does not report them. +# See `eval-magic docs codebase`. +/.eval-magic-outputs/ +/.claude/skills/ +/.claude/settings.local.json +# <<< eval-magic framework files <<< +``` + +The paths come from the selected harness descriptor — its `skills_dir` and the file its write guard +stages — plus the framework outputs directory, so a BYOH harness gets its own paths without +configuring anything. The block is written into **every** environment: both arms, every repetition, +revision mode, `--no-stage`, and `--dry-run`. An entry present in one arm only would trade one +asymmetry for another. + +Which ignore files get the block is detected from the codebase's own tooling: + +| Detected | Ignore file | Created when the project has none | +| --- | --- | --- | +| Prettier | `.prettierignore` | yes | +| ESLint | `.eslintignore` | no — ESLint 9's flat config no longer reads it | +| Stylelint | `.stylelintignore` | yes | +| markdownlint | `.markdownlintignore` | yes | +| Docker | `.dockerignore` | yes | + +Detection reads config-file markers and `package.json` dependencies anywhere in the tree. The list +is short because the ignore-file convention is: Python, Rust, and Go formatters have no ignore file +and already skip dot-directories, so they never see the staged skills in the first place. + +Name the ignore files yourself when the project's are somewhere detection will not look, or when +you want none at all: + +```json +{ + "codebase": { + "url": "https://github.com/slowdini/example-project", + "ref": "v1.4.0", + "ignore_files": ["tooling/.prettierignore"] + } +} +``` + +A declared list replaces detection rather than extending it, and each path is created if missing. +Paths are relative to the environment root and may not escape it. `"ignore_files": []` opts out +entirely, leaving every ignore file exactly as the codebase wrote it. + +`.gitignore` is never a target. The baseline commit force-adds harness config dirs, so an entry +there would hide nothing from Git — but it *would* hide the staged skills from every +`.gitignore`-aware tool the agent uses, such as `rg`, which would damage the treatment arm instead +of protecting it. + +Unlike `exclude_skill_sources`, no artifact records this: the written file is committed into each +environment's baseline, so `git show refs/eval-magic/baseline:.prettierignore` inside any task +environment is the evidence, per arm and per run. `run` also prints the files it wrote. + ## What the environment contains Each dispatch gets its own private environment holding: diff --git a/harnesses/template.toml b/harnesses/template.toml index 93bf49e..fa31c6e 100644 --- a/harnesses/template.toml +++ b/harnesses/template.toml @@ -27,6 +27,9 @@ label = "{label}" ## additional_project_skill_dirs. They participate in sourced-codebase shadow detection and ## codebase.exclude_skill_sources but never receive staged skills. Each first path segment must ## also appear in config_dirs. +## skills_dir and the [guard] staged file are also what `run` writes into a sourced codebase's own +## ignore files (.prettierignore and friends), so the project's linters never report eval-magic's +## staged skills — see `eval-magic docs codebase`. ## VERIFY: which directory does the harness actually scan for skills? Quote the doc or the ## observed behavior in the notes file. # skills_dir = ".{label}/skills" diff --git a/ignore-profiles/tool-docker.toml b/ignore-profiles/tool-docker.toml new file mode 100644 index 0000000..6a083dc --- /dev/null +++ b/ignore-profiles/tool-docker.toml @@ -0,0 +1,8 @@ +# Not a linter: .dockerignore keeps the staged skills and the framework's own +# outputs out of the build context a `docker build` command_check sends. +id = "tool/docker" +ignore_file = ".dockerignore" +create_if_missing = true +markers = ["Dockerfile", ".dockerignore"] +marker_patterns = ["Dockerfile.*"] +package_json_dependencies = [] diff --git a/ignore-profiles/tool-eslint.toml b/ignore-profiles/tool-eslint.toml new file mode 100644 index 0000000..e49ae04 --- /dev/null +++ b/ignore-profiles/tool-eslint.toml @@ -0,0 +1,9 @@ +# ESLint 9's flat config dropped .eslintignore entirely, so creating one would +# be inert noise in a modern project. A legacy project that already has the +# file still gets the framework entries appended to it. +id = "tool/eslint" +ignore_file = ".eslintignore" +create_if_missing = false +markers = [".eslintignore"] +marker_patterns = [".eslintrc*"] +package_json_dependencies = ["eslint"] diff --git a/ignore-profiles/tool-markdownlint.toml b/ignore-profiles/tool-markdownlint.toml new file mode 100644 index 0000000..36aa73a --- /dev/null +++ b/ignore-profiles/tool-markdownlint.toml @@ -0,0 +1,15 @@ +# markdownlint-cli reads .markdownlintignore; markdownlint-cli2 does not (it +# takes globs in its own config), so its markers are deliberately absent — a +# file that tool never reads would be noise, not protection. +id = "tool/markdownlint" +ignore_file = ".markdownlintignore" +create_if_missing = true +markers = [ + ".markdownlintignore", + ".markdownlint.json", + ".markdownlint.jsonc", + ".markdownlint.yaml", + ".markdownlint.yml", +] +marker_patterns = [".markdownlintrc*"] +package_json_dependencies = ["markdownlint-cli"] diff --git a/ignore-profiles/tool-prettier.toml b/ignore-profiles/tool-prettier.toml new file mode 100644 index 0000000..c5c69bb --- /dev/null +++ b/ignore-profiles/tool-prettier.toml @@ -0,0 +1,16 @@ +# Prettier reads .prettierignore (and .gitignore) but ignores nothing by +# default, so `prettier --check .` walks straight into the staged skills dir. +id = "tool/prettier" +ignore_file = ".prettierignore" +create_if_missing = true +markers = [ + ".prettierrc", + ".prettierrc.json", + ".prettierrc.json5", + ".prettierrc.yaml", + ".prettierrc.yml", + ".prettierrc.toml", + ".prettierignore", +] +marker_patterns = ["prettier.config.*"] +package_json_dependencies = ["prettier"] diff --git a/ignore-profiles/tool-stylelint.toml b/ignore-profiles/tool-stylelint.toml new file mode 100644 index 0000000..a904a6d --- /dev/null +++ b/ignore-profiles/tool-stylelint.toml @@ -0,0 +1,6 @@ +id = "tool/stylelint" +ignore_file = ".stylelintignore" +create_if_missing = true +markers = [".stylelintignore"] +marker_patterns = [".stylelintrc*", "stylelint.config.*"] +package_json_dependencies = ["stylelint"] diff --git a/schema/evals.schema.json b/schema/evals.schema.json index ca73b7f..bb37b08 100644 --- a/schema/evals.schema.json +++ b/schema/evals.schema.json @@ -64,6 +64,11 @@ "type": "boolean", "default": false, "description": "Move project-local skill roots discoverable by the selected harness out of every comparison environment before staging. Root instruction files and other harness configuration remain visible." + }, + "ignore_files": { + "type": "array", + "items": { "type": "string", "minLength": 1 }, + "description": "Ignore files, relative to the task environment root, that run writes its framework-file block into, so the project's own linters and formatters never report eval-magic's staged skills, guard files, or outputs. Omit it to detect the project's tooling automatically (Prettier, ESLint, Stylelint, markdownlint, Docker); an empty array opts out entirely. Paths may not be absolute or escape the environment." } } }, @@ -86,6 +91,11 @@ "type": "boolean", "default": false, "description": "Move project-local skill roots discoverable by the selected harness out of every comparison environment before staging. Root instruction files and other harness configuration remain visible." + }, + "ignore_files": { + "type": "array", + "items": { "type": "string", "minLength": 1 }, + "description": "Ignore files, relative to the task environment root, that run writes its framework-file block into, so the project's own linters and formatters never report eval-magic's staged skills, guard files, or outputs. Omit it to detect the project's tooling automatically (Prettier, ESLint, Stylelint, markdownlint, Docker); an empty array opts out entirely. Paths may not be absolute or escape the environment." } } }, diff --git a/src/adapters/descriptor_adapter.rs b/src/adapters/descriptor_adapter.rs index 6636572..ffff3ea 100644 --- a/src/adapters/descriptor_adapter.rs +++ b/src/adapters/descriptor_adapter.rs @@ -126,6 +126,19 @@ impl HarnessAdapter for DescriptorAdapter { }) } + fn framework_ignore_paths(&self) -> Vec { + let mut paths = vec![format!("/{}/", crate::sandbox::GUARD_DENIALS_DIR)]; + if let Some(skills_dir) = &self.descriptor.skills_dir { + paths.push(format!("/{skills_dir}/")); + } + if let Some(guard) = &self.descriptor.guard + && let Some(staged) = crate::adapters::guard::guard_staged_file(guard) + { + paths.push(format!("/{staged}")); + } + paths + } + fn project_skill_dirs(&self, repo_root: &Path) -> Vec { self.descriptor .skills_dir diff --git a/src/adapters/guard.rs b/src/adapters/guard.rs index b0097c0..6d8f9f2 100644 --- a/src/adapters/guard.rs +++ b/src/adapters/guard.rs @@ -336,6 +336,16 @@ fn append_guard_denial(path: &Path, record: &GuardDenialRecord) -> io::Result<() file.write_all(&line) } +/// The env-relative file this guard engine stages: the merged hook config for +/// `json-hooks`, the project plugin for the plugin engines. One place decides +/// it, so a new engine cannot teach half the codebase where its file lives. +pub(crate) fn guard_staged_file(guard: &GuardSection) -> Option<&str> { + match guard.engine { + GuardEngine::JsonHooks => guard.hooks_file.as_deref(), + GuardEngine::OpencodePlugin | GuardEngine::ClinePlugin => guard.plugin_file.as_deref(), + } +} + /// The hook-surface dir the install created outside the skills dir, which /// teardown prunes when restoring the original file leaves it empty. Derived /// from the data: the engine's staged file's parent dir (`hooks_file` for @@ -347,10 +357,7 @@ pub(crate) fn hook_cleanup_dir( skills_dir_rel: Option<&str>, stage_root: &Path, ) -> Option { - let hook_file = match guard.engine { - GuardEngine::JsonHooks => guard.hooks_file.as_deref(), - GuardEngine::OpencodePlugin | GuardEngine::ClinePlugin => guard.plugin_file.as_deref(), - }?; + let hook_file = guard_staged_file(guard)?; let (parent, _) = hook_file.rsplit_once('/')?; if let Some(skills) = skills_dir_rel { // Component-wise ancestry: `.a` owns `.a/b/skills` but not `.ab/skills`. diff --git a/src/adapters/harness.rs b/src/adapters/harness.rs index 2f347ef..b5241f5 100644 --- a/src/adapters/harness.rs +++ b/src/adapters/harness.rs @@ -81,6 +81,19 @@ pub trait HarnessAdapter { self.skills_dir(repo_root).into_iter().collect() } + /// Env-relative paths the runner itself places in every task environment, + /// as gitignore-style patterns. + /// + /// Staged skills sit *inside* the task repository, so a codebase whose lint + /// or format step globs the whole tree reports the framework's artifacts as + /// project failures — and only in the arm that stages a skill. `run` writes + /// these patterns into the project's own ignore files + /// ([`crate::workspace::tool_ignore`]) to keep that from happening. The + /// baseline is the framework outputs dir every harness produces. + fn framework_ignore_paths(&self) -> Vec { + vec![format!("/{}/", crate::sandbox::GUARD_DENIALS_DIR)] + } + // ── Run-option capabilities (defaulted) ────────────────────────────────── /// The run options the generic `run` preflight may accept for this @@ -550,6 +563,49 @@ mod tests { ); } + #[test] + fn framework_ignore_paths_cover_the_staged_skills_the_guard_file_and_the_outputs_dir() { + assert_eq!( + adapter_for(Harness::resolve("claude-code").unwrap()).framework_ignore_paths(), + vec![ + "/.eval-magic-outputs/".to_string(), + "/.claude/skills/".to_string(), + "/.claude/settings.local.json".to_string(), + ] + ); + // A plugin-engine harness contributes its plugin file, not a hooks file. + assert_eq!( + adapter_for(Harness::resolve("opencode").unwrap()).framework_ignore_paths(), + vec![ + "/.eval-magic-outputs/".to_string(), + "/.opencode/skills/".to_string(), + "/.opencode/plugins/slow-powers-eval-guard.js".to_string(), + ] + ); + assert_eq!( + adapter_for(Harness::resolve("codex").unwrap()).framework_ignore_paths(), + vec![ + "/.eval-magic-outputs/".to_string(), + "/.agents/skills/".to_string(), + "/.codex/hooks.json".to_string(), + ] + ); + } + + #[test] + fn framework_ignore_paths_omit_what_a_bare_descriptor_never_stages() { + let descriptor = + crate::adapters::descriptor::load_descriptor("label = \"bare\"\n", "test.toml") + .unwrap(); + let adapter = + crate::adapters::descriptor_adapter::DescriptorAdapter::from_descriptor(descriptor); + + assert_eq!( + adapter.framework_ignore_paths(), + vec!["/.eval-magic-outputs/".to_string()] + ); + } + #[test] fn only_codex_and_opencode_rewrite_frontmatter() { assert!(!adapter_for(Harness::resolve("claude-code").unwrap()).rewrites_frontmatter_name()); diff --git a/src/cli/args.rs b/src/cli/args.rs index 3fccd47..db541c9 100644 --- a/src/cli/args.rs +++ b/src/cli/args.rs @@ -602,6 +602,14 @@ pub(crate) enum Commands { /// selected harness's project skill roots symmetrically before staging. See /// `eval-magic docs codebase` for configuration and provenance, and /// `eval-magic docs isolation` for operator-source remedies and verification. + /// + /// Staged skills, guard files, and framework outputs sit inside the task + /// repository, so every environment also gets the project's own ignore files + /// (`.prettierignore` and friends, detected from the codebase's tooling) + /// taught to skip them — identically in both arms, so a project lint step + /// cannot fail in the treatment arm alone. Set the codebase's `ignore_files` + /// to name those files yourself, or to `[]` to opt out. See + /// `eval-magic docs codebase`. Run(RunArgs), /// Run every task in a prepared iteration through its harness CLI. /// diff --git a/src/cli/run/orchestrate/build.rs b/src/cli/run/orchestrate/build.rs index 9834434..bf8d87f 100644 --- a/src/cli/run/orchestrate/build.rs +++ b/src/cli/run/orchestrate/build.rs @@ -1,7 +1,8 @@ //! Phases 3 & 4 — build every `(eval, condition)` dispatch task and write //! `conditions.json` / `dispatch-manifest.md` / per-task prompts / `dispatch.json` //! ([`write_dispatch`]), then arm the write guard (auto-armed; `--no-guard` -//! opts out) and run the harness's shadow preflight ([`post_build`]). +//! opts out), hide the framework's own staged files from the codebase's tooling +//! ([`ignore`]), and run the harness's shadow preflight ([`post_build`]). use std::collections::HashMap; use std::fs; @@ -26,6 +27,7 @@ use super::{Resolved, RunOptions, Staged}; use crate::cli::command_target_args; use crate::core::fs::{artifact_path, write_json}; +mod ignore; mod roster; use roster::condition_roster; @@ -482,6 +484,20 @@ pub(super) fn post_build( eprintln!("{notice}"); } + // Staged skills, guard files, and framework outputs sit inside the task + // repository, so the project's own linters and formatters are taught to + // skip them before the baseline commit freezes the environment. + let (ignore_files, ignore_warnings) = ignore::hide_framework_files(ctx, r, &targets)?; + for warning in &ignore_warnings { + eprintln!("⚠ {warning}"); + } + if !ignore_files.is_empty() { + println!( + " ignore files: {} — framework files hidden from the project's own tooling", + ignore_files.join(", ") + ); + } + // Establish the repository boundary after all task-visible framework files // exist, but before project-local skill discovery inspects ancestor state. // Recreating `.git` also resets explicit iteration rebuilds to one clean, diff --git a/src/cli/run/orchestrate/build/ignore.rs b/src/cli/run/orchestrate/build/ignore.rs new file mode 100644 index 0000000..339dd88 --- /dev/null +++ b/src/cli/run/orchestrate/build/ignore.rs @@ -0,0 +1,46 @@ +//! Hide the framework's own staged files from the sourced codebase's tooling. + +use crate::adapters::adapter_for; +use crate::core::RunContext; +use crate::workspace::{IgnorePlan, apply_framework_ignore_entries}; + +use super::super::super::RunError; +use super::super::Resolved; +use super::super::envs::EnvTarget; + +/// Teach each task environment's own ignore files to skip the paths the runner +/// placed, and report what was written. +/// +/// Applied to *every* environment — both arms, every repetition, `--no-stage` +/// and `--dry-run` alike. A codebase whose lint or format step globs the tree +/// would otherwise report the staged skills as project failures, and only in +/// the arm that has them; an entry written in one arm alone would trade that +/// asymmetry for another. +pub(super) fn hide_framework_files( + ctx: &RunContext, + r: &Resolved, + targets: &[EnvTarget], +) -> Result<(Vec, Vec), RunError> { + let mut written: Vec = Vec::new(); + let mut warnings: Vec = Vec::new(); + let framework_paths = adapter_for(ctx.harness).framework_ignore_paths(); + for target in targets { + let codebase = r.codebase_for(&target.eval_ids)?; + let outcome = apply_framework_ignore_entries( + &target.root, + &IgnorePlan { + declared: codebase.declared.ignore_files(), + framework_paths: &framework_paths, + }, + )?; + written.extend(outcome.written); + warnings.extend(outcome.warnings); + } + // Every environment holds the same codebase shape, so an undeduplicated + // report would repeat itself once per run cell. + for list in [&mut written, &mut warnings] { + list.sort(); + list.dedup(); + } + Ok((written, warnings)) +} diff --git a/src/core/mod.rs b/src/core/mod.rs index 7446d42..7f49c46 100644 --- a/src/core/mod.rs +++ b/src/core/mod.rs @@ -16,6 +16,7 @@ pub mod fs; pub mod git; pub mod grading; pub mod runtime; +pub mod tree_profiles; pub mod types; pub use capabilities::HarnessRunCapabilities; diff --git a/src/core/tree_profiles.rs b/src/core/tree_profiles.rs new file mode 100644 index 0000000..fee16fa --- /dev/null +++ b/src/core/tree_profiles.rs @@ -0,0 +1,250 @@ +//! Marker-driven profile detection over a task tree. +//! +//! Two packaged profile families ask the same question of a task environment — +//! "which ecosystems and tools does this codebase actually use?" — and answer it +//! from the same three signals: an exact filename, a single-`*` filename +//! pattern, and a `package.json` dependency. The guard's command policy +//! ([`crate::sandbox::guard_profiles`]) and the framework-ignore writer +//! ([`crate::workspace::tool_ignore`]) share this one walk so a marker added for +//! one is understood by the other. + +use std::collections::BTreeSet; +use std::fs; +use std::io; +use std::path::Path; + +/// One packaged profile, as detection sees it. +pub trait TreeProfile { + /// Stable identifier, e.g. `language/rust` or `tool/prettier`. + fn id(&self) -> &str; + /// Filenames that identify this profile exactly. + fn markers(&self) -> &[String]; + /// Filename patterns with at most one `*`, e.g. `requirements*.txt`. + fn marker_patterns(&self) -> &[String]; + /// `package.json` dependency names that identify this profile. + fn package_json_dependencies(&self) -> &[String]; +} + +/// Every profile whose markers appear anywhere in `root`, sorted and deduped. +pub fn detect<'a>( + root: &Path, + profiles: impl IntoIterator, +) -> io::Result> { + let profiles: Vec<&dyn TreeProfile> = profiles.into_iter().collect(); + let mut detected = BTreeSet::new(); + visit(root, &profiles, &mut detected)?; + Ok(detected.into_iter().collect()) +} + +fn visit( + path: &Path, + profiles: &[&dyn TreeProfile], + detected: &mut BTreeSet, +) -> io::Result<()> { + for entry in fs::read_dir(path)? { + let entry = entry?; + let file_type = entry.file_type()?; + let name = entry.file_name(); + let name = name.to_string_lossy(); + if file_type.is_dir() { + if !excluded_directory(&name) { + visit(&entry.path(), profiles, detected)?; + } + continue; + } + if !file_type.is_file() { + continue; + } + + for profile in profiles { + if profile.markers().iter().any(|marker| marker == &name) + || profile + .marker_patterns() + .iter() + .any(|pattern| marker_matches(pattern, &name)) + { + detected.insert(profile.id().to_string()); + } + } + if name == "package.json" { + detect_package_json_profiles(&entry.path(), profiles, detected); + } + } + Ok(()) +} + +fn marker_matches(pattern: &str, name: &str) -> bool { + let Some((prefix, suffix)) = pattern.split_once('*') else { + return pattern == name; + }; + !suffix.contains('*') && name.starts_with(prefix) && name.ends_with(suffix) +} + +/// Directories detection never descends into: Git internals, framework-owned +/// state, and the build/dependency trees whose vendored copies of a marker say +/// nothing about the project itself. +fn excluded_directory(name: &str) -> bool { + matches!( + name, + ".git" + | ".eval-magic-outputs" + | ".claude" + | ".codex" + | ".agents" + | ".opencode" + | ".cline" + | "target" + | "node_modules" + | ".venv" + ) +} + +fn detect_package_json_profiles( + path: &Path, + profiles: &[&dyn TreeProfile], + detected: &mut BTreeSet, +) { + let Ok(body) = fs::read_to_string(path) else { + return; + }; + let Ok(value) = serde_json::from_str::(&body) else { + return; + }; + for profile in profiles { + if profile + .package_json_dependencies() + .iter() + .any(|dependency| { + [ + "dependencies", + "devDependencies", + "peerDependencies", + "optionalDependencies", + ] + .iter() + .any(|field| { + value + .get(field) + .and_then(|deps| deps.get(dependency)) + .is_some() + }) + }) + { + detected.insert(profile.id().to_string()); + } + } +} + +#[cfg(test)] +mod tests { + use std::fs; + + use tempfile::tempdir; + + use super::{TreeProfile, detect}; + + struct Probe { + id: &'static str, + markers: Vec, + marker_patterns: Vec, + package_json_dependencies: Vec, + } + + impl Probe { + fn new(id: &'static str) -> Self { + Self { + id, + markers: Vec::new(), + marker_patterns: Vec::new(), + package_json_dependencies: Vec::new(), + } + } + + fn markers(mut self, values: &[&str]) -> Self { + self.markers = values.iter().map(|value| value.to_string()).collect(); + self + } + + fn marker_patterns(mut self, values: &[&str]) -> Self { + self.marker_patterns = values.iter().map(|value| value.to_string()).collect(); + self + } + + fn dependencies(mut self, values: &[&str]) -> Self { + self.package_json_dependencies = values.iter().map(|value| value.to_string()).collect(); + self + } + } + + impl TreeProfile for Probe { + fn id(&self) -> &str { + self.id + } + fn markers(&self) -> &[String] { + &self.markers + } + fn marker_patterns(&self) -> &[String] { + &self.marker_patterns + } + fn package_json_dependencies(&self) -> &[String] { + &self.package_json_dependencies + } + } + + #[test] + fn detects_markers_patterns_and_dependencies_recursively_and_sorted() { + let root = tempdir().unwrap(); + fs::create_dir_all(root.path().join("frontend")).unwrap(); + fs::write( + root.path().join("frontend/package.json"), + r#"{"devDependencies":{"prettier":"3.0.0"}}"#, + ) + .unwrap(); + fs::create_dir_all(root.path().join("backend")).unwrap(); + fs::write(root.path().join("backend/requirements-dev.txt"), "").unwrap(); + fs::write(root.path().join("Cargo.toml"), "").unwrap(); + + let profiles = [ + Probe::new("tool/prettier").dependencies(&["prettier"]), + Probe::new("language/python").marker_patterns(&["requirements*.txt"]), + Probe::new("language/rust").markers(&["Cargo.toml"]), + ]; + let detected = detect( + root.path(), + profiles.iter().map(|probe| probe as &dyn TreeProfile), + ) + .unwrap(); + + assert_eq!( + detected, + ["language/python", "language/rust", "tool/prettier"] + ); + } + + #[test] + fn framework_and_build_directories_are_never_walked() { + let root = tempdir().unwrap(); + for dir in [ + ".claude", + ".git", + "node_modules", + "target", + ".eval-magic-outputs", + ] { + fs::create_dir_all(root.path().join(dir)).unwrap(); + fs::write(root.path().join(dir).join("Cargo.toml"), "").unwrap(); + } + + let profiles = [Probe::new("language/rust").markers(&["Cargo.toml"])]; + let detected = detect( + root.path(), + profiles.iter().map(|probe| probe as &dyn TreeProfile), + ) + .unwrap(); + + assert!( + detected.is_empty(), + "walked an excluded directory: {detected:?}" + ); + } +} diff --git a/src/core/types.rs b/src/core/types.rs index d37e6d2..1995e4d 100644 --- a/src/core/types.rs +++ b/src/core/types.rs @@ -207,11 +207,15 @@ pub enum CodebaseSource { reference: String, #[serde(default)] exclude_skill_sources: bool, + #[serde(default, skip_serializing_if = "Option::is_none")] + ignore_files: Option>, }, Path { path: String, #[serde(default)] exclude_skill_sources: bool, + #[serde(default, skip_serializing_if = "Option::is_none")] + ignore_files: Option>, }, } @@ -228,6 +232,18 @@ impl CodebaseSource { } => *exclude_skill_sources, } } + + /// The ignore files `run` writes its framework block into, as declared. + /// + /// `None` leaves detection in charge; `Some` replaces it outright, and an + /// empty slice is the opt-out. See [`crate::workspace::tool_ignore`]. + pub fn ignore_files(&self) -> Option<&[String]> { + match self { + Self::Git { ignore_files, .. } | Self::Path { ignore_files, .. } => { + ignore_files.as_deref() + } + } + } } /// Whether a source came from a repository URL or a directory on this host. diff --git a/src/sandbox/guard_profiles.rs b/src/sandbox/guard_profiles.rs index c498d5b..e805c36 100644 --- a/src/sandbox/guard_profiles.rs +++ b/src/sandbox/guard_profiles.rs @@ -1,7 +1,6 @@ //! Packaged command-policy profiles and task-tree auto-detection. -use std::collections::{BTreeSet, HashMap}; -use std::fs; +use std::collections::HashMap; use std::io; use std::path::Path; use std::sync::LazyLock; @@ -9,6 +8,7 @@ use std::sync::LazyLock; use serde::Deserialize; use crate::core::GuardPolicyConfig; +use crate::core::tree_profiles::{TreeProfile, detect}; include!(concat!(env!("OUT_DIR"), "/guard_profiles.rs")); @@ -25,6 +25,21 @@ struct GuardProfile { allow_commands: Vec, } +impl TreeProfile for GuardProfile { + fn id(&self) -> &str { + &self.id + } + fn markers(&self) -> &[String] { + &self.markers + } + fn marker_patterns(&self) -> &[String] { + &self.marker_patterns + } + fn package_json_dependencies(&self) -> &[String] { + &self.package_json_dependencies + } +} + static PROFILES: LazyLock> = LazyLock::new(|| { let mut profiles = HashMap::new(); for (path, body) in PACKAGED_GUARD_PROFILES { @@ -76,95 +91,12 @@ pub(crate) fn expand_policy(policy: &GuardPolicyConfig) -> Result io::Result> { - let mut detected = BTreeSet::new(); - visit(root, &mut detected)?; - Ok(detected.into_iter().collect()) -} - -fn visit(path: &Path, detected: &mut BTreeSet) -> io::Result<()> { - for entry in fs::read_dir(path)? { - let entry = entry?; - let file_type = entry.file_type()?; - let name = entry.file_name(); - let name = name.to_string_lossy(); - if file_type.is_dir() { - if !excluded_directory(&name) { - visit(&entry.path(), detected)?; - } - continue; - } - if !file_type.is_file() { - continue; - } - - for profile in PROFILES.values() { - if profile.markers.iter().any(|marker| marker == &name) - || profile - .marker_patterns - .iter() - .any(|pattern| marker_matches(pattern, &name)) - { - detected.insert(profile.id.clone()); - } - } - if name == "package.json" { - detect_package_json_profiles(&entry.path(), detected); - } - } - Ok(()) -} - -fn marker_matches(pattern: &str, name: &str) -> bool { - let Some((prefix, suffix)) = pattern.split_once('*') else { - return pattern == name; - }; - !suffix.contains('*') && name.starts_with(prefix) && name.ends_with(suffix) -} - -fn excluded_directory(name: &str) -> bool { - matches!( - name, - ".git" - | ".eval-magic-outputs" - | ".claude" - | ".codex" - | ".agents" - | ".opencode" - | ".cline" - | "target" - | "node_modules" - | ".venv" + detect( + root, + PROFILES.values().map(|profile| profile as &dyn TreeProfile), ) } -fn detect_package_json_profiles(path: &Path, detected: &mut BTreeSet) { - let Ok(body) = fs::read_to_string(path) else { - return; - }; - let Ok(value) = serde_json::from_str::(&body) else { - return; - }; - for profile in PROFILES.values() { - if profile.package_json_dependencies.iter().any(|dependency| { - [ - "dependencies", - "devDependencies", - "peerDependencies", - "optionalDependencies", - ] - .iter() - .any(|field| { - value - .get(field) - .and_then(|deps| deps.get(dependency)) - .is_some() - }) - }) { - detected.insert(profile.id.clone()); - } - } -} - #[cfg(test)] mod tests { use std::fs; diff --git a/src/validation/evals.rs b/src/validation/evals.rs index d16a1e0..03ce668 100644 --- a/src/validation/evals.rs +++ b/src/validation/evals.rs @@ -296,9 +296,38 @@ fn validate_codebase(source: &str, label: &str, value: &Value) -> Result<(), Val ))); } } + // `run` writes these paths inside a task environment, so one that leaves it + // would reach the operator's own tree. The schema can state "non-empty + // string" and no more; containment is a semantic rule. + for entry in fields + .get("ignore_files") + .and_then(Value::as_array) + .into_iter() + .flatten() + .filter_map(Value::as_str) + { + if entry.trim().is_empty() { + return Err(invalid(format!( + "{label}: 'ignore_files' entries must contain non-whitespace text" + ))); + } + if !is_contained_relative_path(entry) { + return Err(invalid(format!( + "{label}: 'ignore_files' entry {entry:?} must be a relative path inside the task \ + environment (no leading '/', no '..' segment)" + ))); + } + } Ok(()) } +/// True when `path` names something inside the directory it is resolved +/// against: relative, and with no `..` hop out of it. Split on `/` alone, +/// because that is what the writer splits on. +fn is_contained_relative_path(path: &str) -> bool { + !path.starts_with('/') && path.split('/').all(|segment| segment != "..") +} + fn validate_environment_name( source: &str, eval_id: &str, diff --git a/src/validation/evals/codebase_tests.rs b/src/validation/evals/codebase_tests.rs index bb3c39f..fc31ff8 100644 --- a/src/validation/evals/codebase_tests.rs +++ b/src/validation/evals/codebase_tests.rs @@ -99,6 +99,7 @@ fn accepts_a_top_level_git_codebase_as_the_default() { url: "https://example.com/project.git".to_string(), reference: "main".to_string(), exclude_skill_sources: false, + ignore_files: None, }) ); } @@ -116,6 +117,7 @@ fn accepts_a_per_eval_path_codebase_overriding_the_default() { Some(CodebaseSource::Path { path: "../projects/legacy-service".to_string(), exclude_skill_sources: false, + ignore_files: None, }) ); } @@ -132,6 +134,7 @@ fn accepts_a_top_level_path_codebase() { Some(CodebaseSource::Path { path: "/srv/projects/legacy-service".to_string(), exclude_skill_sources: false, + ignore_files: None, }) ); } @@ -150,6 +153,70 @@ fn accepts_codebase_skill_source_exclusion() { assert_eq!(declared["exclude_skill_sources"], true); } +#[test] +fn accepts_declared_ignore_files() { + let mut config = base(); + config["codebase"] = json!({ + "path": "/srv/projects/legacy-service", + "ignore_files": ["config/.prettierignore", ".stylelintignore"] + }); + + let parsed = validate_evals_config(&config, "evals.json").unwrap(); + + assert_eq!( + parsed.codebase.unwrap().ignore_files(), + Some( + [ + "config/.prettierignore".to_string(), + ".stylelintignore".to_string() + ] + .as_slice() + ) + ); +} + +#[test] +fn an_empty_ignore_files_list_is_the_opt_out_not_the_default() { + let mut config = base(); + config["codebase"] = json!({ "path": ".", "ignore_files": [] }); + + let parsed = validate_evals_config(&config, "evals.json").unwrap(); + + assert_eq!(parsed.codebase.unwrap().ignore_files(), Some([].as_slice())); +} + +#[test] +fn an_absent_ignore_files_list_leaves_detection_in_charge() { + let parsed = validate_evals_config(&base(), "evals.json").unwrap(); + + assert_eq!(parsed.codebase.unwrap().ignore_files(), None); +} + +/// The runner writes these paths inside a task environment, so anything that +/// leaves it — an absolute path, a `..` hop, a blank entry — is refused before +/// a run can touch the host. +#[test] +fn rejects_ignore_file_paths_that_leave_the_task_environment() { + for path in [ + "/etc/.prettierignore", + "../.prettierignore", + "a/../../b", + " ", + ] { + let mut config = base(); + config["codebase"] = json!({ "path": ".", "ignore_files": [path] }); + + let err = validate_evals_config(&config, "evals.json") + .unwrap_err() + .to_string(); + + assert!( + err.contains("ignore_files"), + "path {path:?} was accepted or reported oddly: {err}" + ); + } +} + /// `minLength: 1` admits `" "`, so the schema cannot carry this on its own. #[test] fn rejects_whitespace_only_codebase_values() { diff --git a/src/workspace/mod.rs b/src/workspace/mod.rs index e717fb1..f544005 100644 --- a/src/workspace/mod.rs +++ b/src/workspace/mod.rs @@ -6,12 +6,14 @@ pub mod promote; pub mod snapshot; pub mod teardown; +pub mod tool_ignore; pub use promote::{NotesStatus, PromoteOptions, PromoteResult, promote_baseline}; pub use snapshot::{snapshot, snapshot_set}; pub use teardown::{ KeptIteration, PROMOTED_MARKER, SNAPSHOT_META, WorkspaceCleanupSummary, cleanup_workspace, }; +pub use tool_ignore::{IgnoreOutcome, IgnorePlan, apply_framework_ignore_entries}; use std::time::{SystemTime, UNIX_EPOCH}; diff --git a/src/workspace/tool_ignore.rs b/src/workspace/tool_ignore.rs new file mode 100644 index 0000000..22b5630 --- /dev/null +++ b/src/workspace/tool_ignore.rs @@ -0,0 +1,479 @@ +//! Framework-ignore entries written into a task environment. +//! +//! Staged skills live *inside* the task repository, under the harness's skills +//! dir. A codebase whose lint or format step globs the whole tree therefore +//! reports the framework's own artifacts as project failures — and only in the +//! arm that stages a skill, which biases the comparison and hands the agent +//! under test a red check it can neither fix nor understand (issue #296). +//! +//! The remedy is the one `.eval-magic-outputs/` already gets from +//! `.git/info/exclude`, extended to the tools that do not read Git's exclude +//! file: write the framework's own paths into the project's ignore files. Which +//! ignore files those are is detected from the codebase's tooling through the +//! packaged profiles below, or declared outright as `codebase.ignore_files`. +//! +//! `.gitignore` is deliberately never a target. The task-repository baseline +//! force-adds harness config dirs, so an entry there would hide nothing from +//! Git — but it *would* hide the staged skills from every `.gitignore`-aware +//! tool the agent uses, which breaks the treatment arm instead of fixing it. + +use std::fs; +use std::io; +use std::path::Path; +use std::sync::LazyLock; + +use serde::Deserialize; + +use crate::core::tree_profiles::{TreeProfile, detect}; + +include!(concat!(env!("OUT_DIR"), "/ignore_profiles.rs")); + +/// Opening line of the block this module owns inside an ignore file. +const BLOCK_START: &str = "# >>> eval-magic framework files >>>"; +/// Closing line of the block this module owns inside an ignore file. +const BLOCK_END: &str = "# <<< eval-magic framework files <<<"; + +/// One packaged tool profile: how to recognize the tool, and which ignore file +/// it reads. +#[derive(Debug, Deserialize)] +struct IgnoreProfile { + id: String, + /// Env-root-relative ignore file this tool honors. + ignore_file: String, + /// Whether the file may be created when the project does not have one. + /// False where creating it would be inert or misleading — ESLint 9's flat + /// config no longer reads `.eslintignore` at all. + #[serde(default)] + create_if_missing: bool, + #[serde(default)] + markers: Vec, + #[serde(default)] + marker_patterns: Vec, + #[serde(default)] + package_json_dependencies: Vec, +} + +impl TreeProfile for IgnoreProfile { + fn id(&self) -> &str { + &self.id + } + fn markers(&self) -> &[String] { + &self.markers + } + fn marker_patterns(&self) -> &[String] { + &self.marker_patterns + } + fn package_json_dependencies(&self) -> &[String] { + &self.package_json_dependencies + } +} + +static PROFILES: LazyLock> = LazyLock::new(|| { + let mut profiles: Vec = Vec::new(); + for (path, body) in PACKAGED_IGNORE_PROFILES { + let profile: IgnoreProfile = toml::from_str(body) + .unwrap_or_else(|error| panic!("invalid ignore profile {path}: {error}")); + assert!( + !profiles.iter().any(|existing| existing.id == profile.id), + "duplicate ignore profile {}", + profile.id + ); + profiles.push(profile); + } + profiles +}); + +/// What to write into one task environment. +pub struct IgnorePlan<'a> { + /// The codebase's `ignore_files` declaration. `Some` replaces detection + /// outright — an empty slice opts out of the whole mechanism. + pub declared: Option<&'a [String]>, + /// Env-relative paths the runner itself places, in the order they should + /// appear in the block. + pub framework_paths: &'a [String], +} + +/// What [`apply_framework_ignore_entries`] did. +#[derive(Debug, Default)] +pub struct IgnoreOutcome { + /// Env-relative ignore files actually written, sorted. + pub written: Vec, + /// Warnings for the CLI to print; the library never prints. + pub warnings: Vec, +} + +/// Write the framework block into every ignore file this environment needs. +/// +/// Idempotent: a second call rewrites the block in place rather than appending +/// a second one, so an environment rebuilt over an existing tree stays clean. +pub fn apply_framework_ignore_entries( + env_root: &Path, + plan: &IgnorePlan, +) -> io::Result { + let mut outcome = IgnoreOutcome::default(); + let block = render_block(plan.framework_paths); + for (relative, create_if_missing) in targets(env_root, plan)? { + let path = relative + .split('/') + .fold(env_root.to_path_buf(), |path, segment| path.join(segment)); + let existing = match fs::symlink_metadata(&path) { + Ok(_) => match existing_ignore_file(env_root, &path)? { + Ok(content) => Some(content), + Err(reason) => { + outcome.warnings.push(format!( + "{relative} {reason}; eval-magic could not hide its own staged files from \ + this project's tooling there" + )); + continue; + } + }, + Err(error) if error.kind() == io::ErrorKind::NotFound => None, + Err(error) => return Err(error), + }; + if existing.is_none() && !create_if_missing { + continue; + } + if let Some(parent) = path.parent() { + fs::create_dir_all(parent)?; + } + fs::write( + &path, + merge_block(existing.as_deref().unwrap_or_default(), &block), + )?; + outcome.written.push(relative); + } + outcome.written.sort(); + outcome.written.dedup(); + Ok(outcome) +} + +/// The ignore file already at `path`, or why it cannot be used. +/// +/// A symlink is followed — a monorepo may legitimately point one ignore file at +/// another — but only after its target is proven to stay inside the task +/// environment, so nothing here can write onto the host through a link the +/// sourced codebase carried in. +fn existing_ignore_file(env_root: &Path, path: &Path) -> io::Result> { + let Ok(resolved) = fs::canonicalize(path) else { + return Ok(Err("does not resolve to a readable file")); + }; + if !resolved.starts_with(fs::canonicalize(env_root)?) { + return Ok(Err("resolves outside the task environment")); + } + if !fs::metadata(&resolved)?.is_file() { + return Ok(Err("is not a regular file")); + } + Ok(Ok(fs::read_to_string(&resolved)?)) +} + +/// The ignore files to write, as `(env-relative path, may create)`. +/// +/// A declaration replaces detection entirely, and a declared path is always +/// created: the author named it, so its absence is not a signal. +fn targets(env_root: &Path, plan: &IgnorePlan) -> io::Result> { + if let Some(declared) = plan.declared { + return Ok(declared.iter().map(|path| (path.clone(), true)).collect()); + } + let detected = detect( + env_root, + PROFILES.iter().map(|profile| profile as &dyn TreeProfile), + )?; + Ok(PROFILES + .iter() + .filter(|profile| detected.iter().any(|id| id == &profile.id)) + .map(|profile| (profile.ignore_file.clone(), profile.create_if_missing)) + .collect()) +} + +/// The block as it appears in every target file, gitignore pattern syntax — +/// which Prettier, ESLint, Stylelint, markdownlint, and Docker all accept. +fn render_block(framework_paths: &[String]) -> String { + let mut block = format!( + "{BLOCK_START}\n\ + # Staged by `eval-magic run` so this project's own tooling does not report them.\n\ + # See `eval-magic docs codebase`.\n" + ); + for path in framework_paths { + block.push_str(path); + block.push('\n'); + } + block.push_str(BLOCK_END); + block.push('\n'); + block +} + +/// `existing` with the framework block replaced in place, or appended after a +/// normalizing final newline when it is not there yet. +fn merge_block(existing: &str, block: &str) -> String { + if let Some(start) = existing.find(BLOCK_START) + && let Some(end) = existing[start..].find(BLOCK_END) + { + let after = start + end + BLOCK_END.len(); + let tail = existing[after..] + .strip_prefix('\n') + .unwrap_or(&existing[after..]); + return format!("{}{block}{tail}", &existing[..start]); + } + if existing.is_empty() { + return block.to_string(); + } + let separator = if existing.ends_with('\n') { "" } else { "\n" }; + format!("{existing}{separator}{block}") +} + +#[cfg(test)] +mod tests { + use std::fs; + use std::path::Path; + + use tempfile::tempdir; + + use super::{IgnorePlan, apply_framework_ignore_entries}; + use crate::core::fs::create_symlink; + + /// Whether this host lets the test process create a symlink at all — a + /// capability, not a platform label. Mirrors the probe in `core::fs`. + fn skip_without_symlinks(scratch: &Path, test: &str) -> bool { + let target = scratch.join("probe-target.txt"); + let link = scratch.join("probe-link.txt"); + let available = + fs::write(&target, "probe").is_ok() && create_symlink(&target, &link).is_ok(); + let _ = fs::remove_file(&link); + let _ = fs::remove_file(&target); + !available + && crate::core::runtime::report_skip( + test, + "this filesystem does not permit symlink creation", + ) + } + + const FRAMEWORK: [&str; 3] = [ + "/.eval-magic-outputs/", + "/.claude/skills/", + "/.claude/settings.local.json", + ]; + + fn apply(root: &Path, declared: Option<&[String]>) -> super::IgnoreOutcome { + let framework_paths: Vec = FRAMEWORK.iter().map(|path| path.to_string()).collect(); + apply_framework_ignore_entries( + root, + &IgnorePlan { + declared, + framework_paths: &framework_paths, + }, + ) + .unwrap() + } + + /// The packaged set is data, and a broken entry would fail silently — a + /// profile nothing can detect, or two profiles fighting over one file. + #[test] + fn every_packaged_profile_is_detectable_and_owns_one_contained_ignore_file() { + let mut seen: Vec<&str> = Vec::new(); + for profile in super::PROFILES.iter() { + let id = &profile.id; + assert!( + !profile.markers.is_empty() + || !profile.marker_patterns.is_empty() + || !profile.package_json_dependencies.is_empty(), + "profile {id} declares no way to detect it" + ); + assert!( + !profile.ignore_file.starts_with('/') + && !profile.ignore_file.split('/').any(|part| part == ".."), + "profile {id} names an ignore file outside the task environment: {}", + profile.ignore_file + ); + assert!( + !seen.contains(&profile.ignore_file.as_str()), + "profile {id} claims {}, which another profile already owns", + profile.ignore_file + ); + seen.push(&profile.ignore_file); + } + assert!(!seen.is_empty(), "no ignore profiles were packaged"); + } + + #[test] + fn a_detected_formatter_gets_its_ignore_file_created_with_every_framework_path() { + let root = tempdir().unwrap(); + fs::write(root.path().join(".prettierrc.json"), "{}").unwrap(); + + let outcome = apply(root.path(), None); + + assert_eq!(outcome.written, [".prettierignore"]); + assert!(outcome.warnings.is_empty(), "{:?}", outcome.warnings); + let body = fs::read_to_string(root.path().join(".prettierignore")).unwrap(); + for path in FRAMEWORK { + assert!(body.contains(path), "missing {path} in:\n{body}"); + } + } + + #[test] + fn a_package_json_dependency_detects_the_tool_too() { + let root = tempdir().unwrap(); + fs::write( + root.path().join("package.json"), + r#"{"devDependencies":{"prettier":"3.0.0"}}"#, + ) + .unwrap(); + + let outcome = apply(root.path(), None); + + assert_eq!(outcome.written, [".prettierignore"]); + } + + #[test] + fn rewriting_replaces_the_block_instead_of_appending_a_second_one() { + let root = tempdir().unwrap(); + fs::write(root.path().join(".prettierrc.json"), "{}").unwrap(); + + apply(root.path(), None); + let first = fs::read_to_string(root.path().join(".prettierignore")).unwrap(); + apply(root.path(), None); + let second = fs::read_to_string(root.path().join(".prettierignore")).unwrap(); + + assert_eq!(first, second); + assert_eq!(second.matches("/.claude/skills/").count(), 1); + } + + #[test] + fn the_projects_own_entries_survive_and_a_missing_final_newline_is_repaired() { + let root = tempdir().unwrap(); + fs::write(root.path().join(".prettierrc.json"), "{}").unwrap(); + fs::write(root.path().join(".prettierignore"), "dist\ncoverage").unwrap(); + + apply(root.path(), None); + + let body = fs::read_to_string(root.path().join(".prettierignore")).unwrap(); + assert!(body.starts_with("dist\ncoverage\n"), "clobbered:\n{body}"); + assert!(body.contains("/.claude/skills/")); + } + + #[test] + fn eslints_ignore_file_is_appended_to_but_never_created() { + let root = tempdir().unwrap(); + fs::write(root.path().join(".eslintrc.json"), "{}").unwrap(); + + let outcome = apply(root.path(), None); + + assert!(outcome.written.is_empty(), "{:?}", outcome.written); + assert!(!root.path().join(".eslintignore").exists()); + + fs::write(root.path().join(".eslintignore"), "vendor\n").unwrap(); + let outcome = apply(root.path(), None); + + assert_eq!(outcome.written, [".eslintignore"]); + let body = fs::read_to_string(root.path().join(".eslintignore")).unwrap(); + assert!(body.starts_with("vendor\n")); + assert!(body.contains("/.claude/skills/")); + } + + #[test] + fn a_declared_list_replaces_detection_and_creates_parent_directories() { + let root = tempdir().unwrap(); + fs::write(root.path().join(".prettierrc.json"), "{}").unwrap(); + let declared = ["config/.prettierignore".to_string()]; + + let outcome = apply(root.path(), Some(&declared)); + + assert_eq!(outcome.written, ["config/.prettierignore"]); + assert!(!root.path().join(".prettierignore").exists()); + let body = fs::read_to_string(root.path().join("config/.prettierignore")).unwrap(); + assert!(body.contains("/.claude/skills/")); + } + + #[test] + fn an_empty_declared_list_opts_out_entirely() { + let root = tempdir().unwrap(); + fs::write(root.path().join(".prettierrc.json"), "{}").unwrap(); + + let outcome = apply(root.path(), Some(&[])); + + assert!(outcome.written.is_empty(), "{:?}", outcome.written); + assert!(!root.path().join(".prettierignore").exists()); + } + + #[test] + fn a_directory_where_an_ignore_file_belongs_is_reported_not_written() { + let root = tempdir().unwrap(); + fs::write(root.path().join(".prettierrc.json"), "{}").unwrap(); + fs::create_dir(root.path().join(".prettierignore")).unwrap(); + + let outcome = apply(root.path(), None); + + assert!(outcome.written.is_empty(), "{:?}", outcome.written); + assert_eq!(outcome.warnings.len(), 1, "{:?}", outcome.warnings); + assert!( + outcome.warnings[0].contains(".prettierignore"), + "{}", + outcome.warnings[0] + ); + } + + #[test] + fn a_symlinked_ignore_file_inside_the_environment_is_written_through() { + let root = tempdir().unwrap(); + if skip_without_symlinks( + root.path(), + "a_symlinked_ignore_file_inside_the_environment_is_written_through", + ) { + return; + } + fs::write(root.path().join(".prettierrc.json"), "{}").unwrap(); + fs::create_dir(root.path().join("config")).unwrap(); + fs::write(root.path().join("config/ignore-rules"), "dist\n").unwrap(); + create_symlink( + Path::new("config/ignore-rules"), + &root.path().join(".prettierignore"), + ) + .unwrap(); + + let outcome = apply(root.path(), None); + + assert_eq!(outcome.written, [".prettierignore"]); + let body = fs::read_to_string(root.path().join("config/ignore-rules")).unwrap(); + assert!(body.starts_with("dist\n"), "clobbered:\n{body}"); + assert!(body.contains("/.claude/skills/")); + } + + #[test] + fn a_symlink_escaping_the_environment_is_reported_not_followed() { + let outside = tempdir().unwrap(); + let root = tempdir().unwrap(); + if skip_without_symlinks( + root.path(), + "a_symlink_escaping_the_environment_is_reported_not_followed", + ) { + return; + } + fs::write(outside.path().join("host.prettierignore"), "dist\n").unwrap(); + fs::write(root.path().join(".prettierrc.json"), "{}").unwrap(); + create_symlink( + &outside.path().join("host.prettierignore"), + &root.path().join(".prettierignore"), + ) + .unwrap(); + + let outcome = apply(root.path(), None); + + assert!(outcome.written.is_empty(), "{:?}", outcome.written); + assert_eq!(outcome.warnings.len(), 1, "{:?}", outcome.warnings); + assert_eq!( + fs::read_to_string(outside.path().join("host.prettierignore")).unwrap(), + "dist\n", + "wrote outside the task environment" + ); + } + + #[test] + fn a_codebase_with_no_detected_tooling_is_left_alone() { + let root = tempdir().unwrap(); + fs::write(root.path().join("Cargo.toml"), "").unwrap(); + + let outcome = apply(root.path(), None); + + assert!(outcome.written.is_empty(), "{:?}", outcome.written); + assert_eq!(fs::read_dir(root.path()).unwrap().count(), 1); + } +} diff --git a/tests/cli/docs/codebase.rs b/tests/cli/docs/codebase.rs index 1512f63..720e896 100644 --- a/tests/cli/docs/codebase.rs +++ b/tests/cli/docs/codebase.rs @@ -37,3 +37,22 @@ fn keeps_declaration_rules_project_choices_caveat_and_provisioning_contract() { .stdout(contains("CLAUDE.md")) .stdout(contains(".opencode/skills")); } + +/// A codebase's own linters must not be able to see the framework's staged +/// files, and an author has to be able to find both the detected set and the +/// override without reading the source. +#[test] +fn keeps_the_framework_ignore_contract_and_its_override() { + skill_eval() + .args(["docs", "codebase"]) + .assert() + .success() + .stdout(contains("ignore_files")) + .stdout(contains(".prettierignore")) + .stdout(contains(".eslintignore")) + .stdout(contains(">>> eval-magic framework files >>>")) + .stdout(contains("both arms")) + .stdout(contains("replaces detection")) + .stdout(contains("`.gitignore` is never a target")) + .stdout(contains("refs/eval-magic/baseline:.prettierignore")); +} diff --git a/tests/run/ignore_files.rs b/tests/run/ignore_files.rs new file mode 100644 index 0000000..b3d0336 --- /dev/null +++ b/tests/run/ignore_files.rs @@ -0,0 +1,156 @@ +//! Framework-staged files stay out of the project's own lint and format scope, +//! identically in both comparison arms (issue #296). + +use crate::codebase_support::{codebase_repo, commit, evals_with_codebase, git}; +use crate::helpers::*; +use std::fs; +use std::path::{Path, PathBuf}; + +/// A codebase that runs Prettier, the shape the shipped Weeknight fixture has: +/// a Prettier config and no `.prettierignore` of its own. +fn prettier_codebase(root: &Path) -> PathBuf { + let repo = codebase_repo(root, "origin", "main"); + fs::write(repo.join(".prettierrc.json"), "{}\n").unwrap(); + commit(&repo, "add prettier config"); + repo +} + +fn run_against(cwd: &Path, skill_dir: &Path, extra: &[&str]) { + skill_eval() + .current_dir(cwd) + .args(["run", "--skill-dir"]) + .arg(skill_dir) + .args([ + "--skill", + "mr-review", + "--mode", + "new-skill", + "--harness", + "claude-code", + "--no-guard", + "--dry-run", + ]) + .args(extra) + .assert() + .success(); +} + +fn prepare(tmp: &Path, codebase_json: &str) -> (PathBuf, PathBuf) { + let (skill_dir, cwd) = setup(tmp, &evals_with_codebase(codebase_json)); + fs::write(skill_dir.join("mr-review/evals/TASK.md"), "task\n").unwrap(); + (skill_dir, cwd) +} + +#[test] +fn both_arms_get_the_same_ignore_file_hiding_the_staged_skills() { + let tmp = tempfile::TempDir::new().unwrap(); + let origin = prettier_codebase(tmp.path()); + let source = format!(r#"{{ "url": "{}", "ref": "main" }}"#, wire_path(&origin)); + let (skill_dir, cwd) = prepare(tmp.path(), &source); + + run_against(&cwd, &skill_dir, &[]); + + let with = cli_env_dir(&cwd, "g1", "with_skill").join(".prettierignore"); + let without = cli_env_dir(&cwd, "g1", "without_skill").join(".prettierignore"); + let with_body = fs::read_to_string(&with).unwrap(); + let without_body = fs::read_to_string(&without).unwrap(); + + assert_eq!( + with_body, without_body, + "the arms disagree about what the project's formatter sees" + ); + for entry in [ + "/.eval-magic-outputs/", + "/.claude/skills/", + "/.claude/settings.local.json", + ] { + assert!( + with_body.contains(entry), + "missing {entry} in:\n{with_body}" + ); + } +} + +#[test] +fn the_ignore_file_is_part_of_the_baseline_so_it_never_shows_up_as_agent_work() { + let tmp = tempfile::TempDir::new().unwrap(); + let origin = prettier_codebase(tmp.path()); + let source = format!(r#"{{ "url": "{}", "ref": "main" }}"#, wire_path(&origin)); + let (skill_dir, cwd) = prepare(tmp.path(), &source); + + run_against(&cwd, &skill_dir, &[]); + + for condition in ["with_skill", "without_skill"] { + let env = cli_env_dir(&cwd, "g1", condition); + assert_eq!( + git(&env, &["status", "--porcelain", "--untracked-files=all"]), + "", + "{condition}: the environment is dirty before dispatch" + ); + let committed = git(&env, &["show", "refs/eval-magic/baseline:.prettierignore"]); + assert!( + committed.contains("/.claude/skills/"), + "{condition}: the ignore file is not in the baseline: {committed}" + ); + } +} + +#[test] +fn an_empty_ignore_files_declaration_leaves_the_codebase_untouched() { + let tmp = tempfile::TempDir::new().unwrap(); + let origin = prettier_codebase(tmp.path()); + let source = format!( + r#"{{ "url": "{}", "ref": "main", "ignore_files": [] }}"#, + wire_path(&origin) + ); + let (skill_dir, cwd) = prepare(tmp.path(), &source); + + run_against(&cwd, &skill_dir, &[]); + + for condition in ["with_skill", "without_skill"] { + assert!( + !cli_env_dir(&cwd, "g1", condition) + .join(".prettierignore") + .exists(), + "{condition}: wrote an ignore file the eval opted out of" + ); + } +} + +#[test] +fn a_declared_ignore_file_is_written_where_the_eval_asked() { + let tmp = tempfile::TempDir::new().unwrap(); + let origin = prettier_codebase(tmp.path()); + let source = format!( + r#"{{ "url": "{}", "ref": "main", "ignore_files": ["tooling/.prettierignore"] }}"#, + wire_path(&origin) + ); + let (skill_dir, cwd) = prepare(tmp.path(), &source); + + run_against(&cwd, &skill_dir, &[]); + + for condition in ["with_skill", "without_skill"] { + let env = cli_env_dir(&cwd, "g1", condition); + assert!( + !env.join(".prettierignore").exists(), + "{condition}: detection ran even though the eval declared its own list" + ); + let body = fs::read_to_string(env.join("tooling/.prettierignore")).unwrap(); + assert!(body.contains("/.claude/skills/"), "{condition}: {body}"); + } +} + +#[test] +fn a_codebase_with_no_matching_tooling_gets_no_ignore_file() { + 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) = prepare(tmp.path(), &source); + + run_against(&cwd, &skill_dir, &[]); + + let env = cli_env_dir(&cwd, "g1", "with_skill"); + for name in [".prettierignore", ".eslintignore", ".dockerignore"] { + assert!(!env.join(name).exists(), "invented {name}"); + } +} diff --git a/tests/run/main.rs b/tests/run/main.rs index 862f68b..5da7217 100644 --- a/tests/run/main.rs +++ b/tests/run/main.rs @@ -28,6 +28,7 @@ mod env_layout; mod git_isolation; mod grouping; mod guard_policy; +mod ignore_files; mod judges; mod lifecycle; mod multi_skill;