diff --git a/docs/claude-notes.md b/docs/claude-notes.md index 42b8ab4..0c33ce1 100644 --- a/docs/claude-notes.md +++ b/docs/claude-notes.md @@ -128,8 +128,9 @@ Each `claude -p` dispatch loads the user/global plugins and skills from its Clau staging slug prevents an on-disk collision but not runtime discovery — an installed plugin exposing a same-named skill is discoverable in *both* arms, so the control arm is not truly skill-absent. `plugin_shadow.rs` detects this in every comparison environment. The shared shadow policy records -one finding per logical skill in schema-v2 `plugin-shadow.json`, including every affected cell, -canonical/discovery paths, source-specific remediation, and the runtime identifier the agent sees. +one finding per logical skill and source class in schema-v3 `plugin-shadow.json`, including every +affected cell, canonical/discovery paths, source-specific remediation, and the runtime identifier +the agent sees. Claude plugin skills use their namespaced `:` runtime ID, direct live skills retain the logical name, and staged subjects use their staging-directory slug. Direct live duplicates record user-before-project precedence; a staged subject with its distinct slug remains selected. diff --git a/docs/guides/byoh.md b/docs/guides/byoh.md index ea18b71..4c2eabf 100644 --- a/docs/guides/byoh.md +++ b/docs/guides/byoh.md @@ -93,6 +93,21 @@ The scaffold and resolved descriptor output are the installed references. Reposi can trace the underlying schema and adapter contracts from `docs/developer_overview.md` in a source checkout. +When a harness discovers project skills from more than its native staging directory, declare the +extra roots beside `skills_dir`: + +```toml +skills_dir = ".cool/skills" +additional_project_skill_dirs = [".claude/skills", ".agents/skills"] +config_dirs = [".cool", ".claude", ".agents"] +``` + +`skills_dir` is the only staging destination. The additional roots participate in sourced-codebase +shadow detection and `codebase.exclude_skill_sources`; eval-magic never stages into them. Every +path must be normalized, `/`-separated, and relative to the task repository. Its first segment must +also appear in `config_dirs`, keeping discovery, sibling filtering, and task-repository baselining +on one descriptor surface. + ## Layer descriptors by field Descriptors load in this order: diff --git a/docs/guides/codebase.md b/docs/guides/codebase.md index 5537ff0..d31fa7a 100644 --- a/docs/guides/codebase.md +++ b/docs/guides/codebase.md @@ -54,6 +54,45 @@ 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. +## Project config and skill sources + +The sourced tree is preserved by default, including harness instructions, settings, plugins, and +project-local skills. For example, `CLAUDE.md`, `AGENTS.md`, `.claude/settings.json`, and +`.opencode/settings.json` remain visible in every comparison arm. + +Preserving project skills can contaminate a comparison when the codebase provides the +subject or one of its staged siblings. `run` records those matches in `plugin-shadow.json` with +`class: "codebase-sourced"`, separately from `class: "operator-environment"` findings caused by +global skills or installed plugins. Subject collisions are comparison-invalid; sibling collisions +follow the symmetric/asymmetric rules in `eval-magic docs isolation`. + +Opt an eval out of only the harness-discoverable project skill roots when the codebase's skills are +not part of the task being measured: + +```json +{ + "codebase": { + "url": "https://github.com/slowdini/example-project", + "ref": "v1.4.0", + "exclude_skill_sources": true + } +} +``` + +The default is `false`. When set to `true`, eval-magic moves every project skill root declared by +the selected harness out of each task environment before staging. It applies equally to both arms, +every repetition, revision mode, and `--no-stage`. Root instruction files and other harness config +remain in place. OpenCode, for example, excludes `.opencode/skills`, `.claude/skills`, and +`.agents/skills` because its descriptor declares all three discovery roots. For a BYOH descriptor +with no project skill roots, the setting is recorded and makes no filesystem change. + +Generated staging slugs are collision-safe: if the codebase owns that exact directory, the +runner backs it up, stages the evaluated copy for that arm, and restores the original during +cleanup. An explicit `--stage-name` remains stricter and refuses to clobber an occupied directory. + +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`. + ## What the environment contains Each dispatch gets its own private environment holding: @@ -172,9 +211,9 @@ head -50 diff.patch The same difference, spelled by Git itself, is `git diff refs/eval-magic/baseline` inside the environment. -The resolved commit appears in `conditions.json`, each `run.json`, `benchmark.json`, and the -`BASELINE.md` written by `promote-baseline` — alongside the skill the run measured, which -is recorded the same way: +The resolved commit and effective skill-source policy appear in `conditions.json`, each `run.json`, +`benchmark.json`, and the `BASELINE.md` written by `promote-baseline` — alongside the skill the run +measured, which is recorded the same way: ```sh jq '.codebases, .skill_source' conditions.json diff --git a/docs/guides/isolation.md b/docs/guides/isolation.md index 3e5f56b..46c24e1 100644 --- a/docs/guides/isolation.md +++ b/docs/guides/isolation.md @@ -20,9 +20,14 @@ Sibling collisions have two outcomes: - A sibling visible in only one arm is comparison-invalid because its effect cannot be separated from the skill under test. -The preflight reports what the environment makes discoverable. Transcript evidence can later show -what a dispatch loaded. Eval-magic does not parse shell templates to infer that a flag or environment -variable isolates the process. +The preflight reports two source classes in schema-v3 `plugin-shadow.json`: + +- `operator-environment` — global skills, enabled plugins, and other sources inherited from the + machine running eval-magic. +- `codebase-sourced` — matching project-local skills preserved from the task codebase. + +Transcript evidence can later show what a dispatch loaded. Eval-magic does not parse shell +templates to infer that a flag or environment variable isolates the process. Apply the remedy to **every eval-agent command**, including every resumed turn of a scripted eval. Isolating only the first round allows the live copy to return on the next round. Judge commands do @@ -80,9 +85,14 @@ label = "claude-code" isolates_live_sources = true ``` +The declaration covers only `operator-environment` findings. It does not claim that skills sourced +from the task codebase are isolated. Use `codebase.exclude_skill_sources: true` for that separate +policy when project skills should not participate; see `eval-magic docs codebase`. + The declaration does not disable detection. `plugin-shadow.json` retains every source and its -intrinsic severity as provenance. `run` presents the finding as informational, and `aggregate` -omits the warning only while no transcript evidence contradicts the declaration. +intrinsic severity as provenance. `run` presents operator-environment findings as informational, +and `aggregate` omits those warnings only while no transcript evidence contradicts the declaration. +Codebase-sourced findings remain warnings regardless of this descriptor setting. Do not set it when: diff --git a/docs/progressive-enhancements.md b/docs/progressive-enhancements.md index 19f9de3..0ac2fa6 100644 --- a/docs/progressive-enhancements.md +++ b/docs/progressive-enhancements.md @@ -347,16 +347,19 @@ global `.opencode`, `.claude`, and `.agents` skill dirs — including skills ins harnesses. A logical eval skill present in any such source can contaminate the with/without comparison when dispatches load that source, even when the staged copy uses a unique slug. -*What it unlocks:* a build-time contamination warning (shared banner + schema-v2 +*What it unlocks:* a build-time contamination warning (shared banner + schema-v3 `plugin-shadow.json` in the iteration dir), which `aggregate` folds into `benchmark.json` validity warnings. The runner scans every matrix environment and the shared policy groups scanner -facts by logical skill, records live/staged sources and affected cells, and assigns role-aware -severity. Subject and asymmetric sibling collisions invalidate the comparison; symmetric sibling -collisions warn. Because the scan runs before dispatch it reports *risk*, so the banner states the -consequence conditionally; the verdict is settled afterwards by the session-surface sub-capability -below. When the resolved descriptor declares `isolates_live_sources = true`, the scan, intrinsic -severity, and artifact are retained, but the banner becomes an informational notice and `aggregate` -omits the findings from validity warnings. Historical unversioned artifacts remain readable. +facts by logical skill and source class, records live/staged sources and affected cells, and assigns +role-aware severity. `operator-environment` findings come from inherited global/plugin sources; +`codebase-sourced` findings come from project roots the harness descriptor declares. Subject and +asymmetric sibling collisions invalidate the comparison; symmetric sibling collisions warn. +Because the scan runs before dispatch it reports *risk*, so the banner states the consequence +conditionally; the verdict is settled afterwards by the session-surface sub-capability below. When +the resolved descriptor declares `isolates_live_sources = true`, operator-source scan facts, +intrinsic severity, and artifact are retained, but the banner becomes informational and `aggregate` +omits those findings. Codebase findings use the eval's separate `exclude_skill_sources` policy and +remain warnings when preserved. Schema-v2 and historical unversioned artifacts remain readable. ### Session surface (sub-capability of transcript ingest) diff --git a/harnesses/opencode.toml b/harnesses/opencode.toml index 611d480..e2e4497 100644 --- a/harnesses/opencode.toml +++ b/harnesses/opencode.toml @@ -9,7 +9,8 @@ label = "opencode" skills_dir = ".opencode/skills" -config_dirs = [".opencode"] +additional_project_skill_dirs = [".claude/skills", ".agents/skills"] +config_dirs = [".opencode", ".claude", ".agents"] [run] supports_guard = true diff --git a/harnesses/template.toml b/harnesses/template.toml index 201e185..1607602 100644 --- a/harnesses/template.toml +++ b/harnesses/template.toml @@ -23,10 +23,15 @@ label = "{label}" ## Where the harness discovers project-local skills. Declaring skills_dir unlocks native ## staging; without it every run is forced to --no-stage. The first path segment of skills_dir ## must appear in config_dirs (it feeds the staging sibling filter and task-repository baseline). +## If the harness also discovers compatibility roots, list them in +## 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. ## 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" -# config_dirs = [".{label}"] +# additional_project_skill_dirs = [".claude/skills", ".agents/skills"] +# config_dirs = [".{label}", ".claude", ".agents"] ## ------------------------------------------------------------------------------------------- ## [dispatch] — the highest-leverage first enhancement: with exec_template declared, the diff --git a/schema/benchmark.schema.json b/schema/benchmark.schema.json index 1b22cce..00fcbb0 100644 --- a/schema/benchmark.schema.json +++ b/schema/benchmark.schema.json @@ -107,6 +107,10 @@ "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." }, + "exclude_skill_sources": { + "type": "boolean", + "description": "Whether project-local skill roots discoverable by the selected harness were removed from the comparison environment before staging." + }, "evals": { "type": "array", "items": { "type": "string" }, diff --git a/schema/evals.schema.json b/schema/evals.schema.json index 1bb1e07..54d287a 100644 --- a/schema/evals.schema.json +++ b/schema/evals.schema.json @@ -41,6 +41,11 @@ "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." + }, + "exclude_skill_sources": { + "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." } } }, @@ -58,6 +63,11 @@ "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." + }, + "exclude_skill_sources": { + "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." } } }, diff --git a/schema/harness-descriptor.schema.json b/schema/harness-descriptor.schema.json index f1865b3..f2135ce 100644 --- a/schema/harness-descriptor.schema.json +++ b/schema/harness-descriptor.schema.json @@ -17,6 +17,12 @@ "minLength": 1, "description": "Project-local staged-skills directory, `/`-separated relative to the repo root (e.g. \".claude/skills\"). Optional: a harness without one cannot stage skills natively, so runs fall back to --no-stage (each SKILL.md inlined into its dispatch prompt) with a preflight warning." }, + "additional_project_skill_dirs": { + "type": "array", + "uniqueItems": true, + "items": { "type": "string", "minLength": 1 }, + "description": "Additional project-local skill roots the harness discovers for compatibility with other harness conventions. These roots participate in codebase shadow detection and opt-in exclusion but never receive staged skills." + }, "config_dirs": { "type": "array", "items": { "type": "string", "minLength": 1 }, diff --git a/schema/plugin-shadow.schema.json b/schema/plugin-shadow.schema.json index 586808f..726d6be 100644 --- a/schema/plugin-shadow.schema.json +++ b/schema/plugin-shadow.schema.json @@ -12,7 +12,7 @@ ], "properties": { "schema_version": { - "const": 2 + "const": 3 }, "config_dir": { "type": "string" @@ -35,12 +35,20 @@ "type": "object", "additionalProperties": false, "required": [ + "class", "skill_name", "role", "severity", "sources" ], "properties": { + "class": { + "enum": [ + "operator-environment", + "codebase-sourced" + ], + "description": "Whether the non-staged source comes from the operator environment or from the sourced task codebase." + }, "skill_name": { "type": "string", "minLength": 1 diff --git a/schema/run-record.schema.json b/schema/run-record.schema.json index 6fee4d9..a1819d9 100644 --- a/schema/run-record.schema.json +++ b/schema/run-record.schema.json @@ -274,6 +274,10 @@ "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." }, + "exclude_skill_sources": { + "type": "boolean", + "description": "Whether project-local skill roots discoverable by the selected harness were removed from the task environment before staging." + }, "dirty": { "type": "boolean", "description": "True when the copy this record describes carries uncommitted work from its source, so revision alone does not name what ran. A codebase is checked out at a commit and is never dirty; a skill is copied as it sits on disk and can be." diff --git a/src/adapters/descriptor.rs b/src/adapters/descriptor.rs index fd025e1..1af679f 100644 --- a/src/adapters/descriptor.rs +++ b/src/adapters/descriptor.rs @@ -59,6 +59,8 @@ pub struct HarnessDescriptor { #[serde(skip_serializing_if = "Option::is_none")] pub skills_dir: Option, #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub additional_project_skill_dirs: Vec, + #[serde(default, skip_serializing_if = "Vec::is_empty")] pub config_dirs: Vec, #[serde(default, skip_serializing_if = "RunSection::is_default")] pub run: RunSection, @@ -641,6 +643,72 @@ timestamp_spread = "timestamp" assert!(d.transcript.is_none()); } + #[test] + fn additional_project_skill_dirs_load_and_reserialize() { + let d = load( + "label = \"demo\"\nskills_dir = \".demo/skills\"\n\ + additional_project_skill_dirs = [\".claude/skills\", \".agents/skills\"]\n\ + config_dirs = [\".demo\", \".claude\", \".agents\"]\n", + ) + .unwrap(); + + assert_eq!( + d.additional_project_skill_dirs, + vec![".claude/skills", ".agents/skills"] + ); + let shown = toml::to_string(&d).unwrap(); + assert!(shown.contains("additional_project_skill_dirs"), "{shown}"); + } + + #[test] + fn rejects_config_dirs_missing_an_additional_project_skill_parent() { + let error = err_of( + "label = \"demo\"\nskills_dir = \".demo/skills\"\n\ + additional_project_skill_dirs = [\".claude/skills\"]\n\ + config_dirs = [\".demo\"]\n", + ); + + assert!(error.contains(".claude"), "{error}"); + assert!(error.contains("additional project skill"), "{error}"); + } + + #[test] + fn rejects_additional_project_skill_dirs_without_a_native_skills_dir() { + let error = err_of( + "label = \"demo\"\nadditional_project_skill_dirs = [\".claude/skills\"]\n\ + config_dirs = [\".claude\"]\n", + ); + + assert!(error.contains("additional_project_skill_dirs"), "{error}"); + assert!(error.contains("skills_dir"), "{error}"); + } + + #[test] + fn rejects_project_skill_dirs_that_escape_or_duplicate_the_native_root() { + for additional in [ + "../skills", + "/tmp/skills", + ".claude/../skills", + ".demo/skills", + ] { + let error = err_of(&format!( + "{MINIMAL}\nadditional_project_skill_dirs = [\"{additional}\"]\n" + )); + assert!(error.contains("project skill"), "{additional}: {error}"); + } + } + + #[test] + fn rejects_backslash_separated_project_skill_dirs() { + let error = err_of( + "label = \"demo\"\nskills_dir = \".demo/skills\"\n\ + additional_project_skill_dirs = [\".claude\\\\skills\"]\n\ + config_dirs = [\".demo\", \".claude\\\\skills\"]\n", + ); + + assert!(error.contains("`/`-separated"), "{error}"); + } + #[test] fn dispatch_environment_loads_and_reserializes() { let d = load(&format!( diff --git a/src/adapters/descriptor/validation.rs b/src/adapters/descriptor/validation.rs index fd97dbe..712a52e 100644 --- a/src/adapters/descriptor/validation.rs +++ b/src/adapters/descriptor/validation.rs @@ -30,6 +30,7 @@ type Check = fn(&HarnessDescriptor) -> Result<(), String>; const CHECKS: &[Check] = &[ check_dispatch_env, check_guard_lockstep, + check_project_skill_dirs, check_skills_dir_requirements, check_slug_shape, check_config_dirs_cover_skills_dir, @@ -44,6 +45,42 @@ const CHECKS: &[Check] = &[ check_skills_block_item, ]; +/// Skill-root paths drive staging cleanup and opt-in codebase source moves, so +/// every one must stay beneath the task repository and name a single normalized +/// location. +fn check_project_skill_dirs(d: &HarnessDescriptor) -> Result<(), String> { + let mut roots = Vec::new(); + if let Some(native) = &d.skills_dir { + roots.push(("skills_dir", native)); + } + roots.extend( + d.additional_project_skill_dirs + .iter() + .map(|path| ("additional_project_skill_dirs", path)), + ); + for (field, path) in roots { + if path.starts_with('/') + || path.contains('\\') + || path + .split('/') + .any(|segment| segment.is_empty() || segment == "." || segment == "..") + { + return Err(format!( + "{field} project skill path must be a relative `/`-separated path without empty, \ + `.` or `..` segments (got \"{path}\")" + )); + } + } + if let Some(native) = &d.skills_dir + && d.additional_project_skill_dirs.contains(native) + { + return Err(format!( + "additional project skill dirs duplicate skills_dir \"{native}\"" + )); + } + Ok(()) +} + /// Check every cross-field invariant, returning the first violation with an /// actionable message. pub(super) fn validate_descriptor( @@ -94,6 +131,14 @@ fn check_guard_lockstep(d: &HarnessDescriptor) -> Result<(), String> { /// Without a skills_dir neither has anywhere to operate. fn check_skills_dir_requirements(d: &HarnessDescriptor) -> Result<(), String> { if d.skills_dir.is_none() { + if !d.additional_project_skill_dirs.is_empty() { + return Err( + "additional_project_skill_dirs is declared but skills_dir is not; exclusion and \ + cleanup record project skill roots in the native skills_dir manifest — declare \ + it, or drop additional_project_skill_dirs" + .into(), + ); + } if d.staging.is_configured() { return Err( "[staging] is configured but skills_dir is not declared; native staging \ @@ -170,6 +215,17 @@ fn check_config_dirs_cover_skills_dir(d: &HarnessDescriptor) -> Result<(), Strin )); } } + for skills_dir in &d.additional_project_skill_dirs { + let top = skills_dir.split('/').next().unwrap_or_default(); + if !d.config_dirs.iter().any(|dir| dir == top) { + return Err(format!( + "config_dirs {:?} misses \"{top}\", the parent of additional project skill dir \ + \"{skills_dir}\" — discovery, sibling filtering, and task-repository baselining \ + must use the same harness config surface", + d.config_dirs + )); + } + } Ok(()) } diff --git a/src/adapters/descriptor_adapter.rs b/src/adapters/descriptor_adapter.rs index bfd0ce5..8ebbb44 100644 --- a/src/adapters/descriptor_adapter.rs +++ b/src/adapters/descriptor_adapter.rs @@ -126,6 +126,18 @@ impl HarnessAdapter for DescriptorAdapter { }) } + fn project_skill_dirs(&self, repo_root: &Path) -> Vec { + self.descriptor + .skills_dir + .iter() + .chain(&self.descriptor.additional_project_skill_dirs) + .map(|dir| { + dir.split('/') + .fold(repo_root.to_path_buf(), |path, segment| path.join(segment)) + }) + .collect() + } + fn run_capabilities(&self) -> HarnessRunCapabilities { HarnessRunCapabilities { supports_guard: self.descriptor.run.supports_guard, diff --git a/src/adapters/harness.rs b/src/adapters/harness.rs index 1516b33..2768145 100644 --- a/src/adapters/harness.rs +++ b/src/adapters/harness.rs @@ -77,6 +77,15 @@ pub trait HarnessAdapter { /// `--no-stage` (each SKILL.md is inlined into its dispatch prompt). fn skills_dir(&self, repo_root: &Path) -> Option; + /// Every project-local skill root this harness may discover. The native + /// staging root comes first, followed by any cross-harness compatibility + /// roots declared by the descriptor. This surface is used for codebase + /// shadow detection and opt-in source exclusion; staging still writes only + /// to [`skills_dir`](Self::skills_dir). + fn project_skill_dirs(&self, repo_root: &Path) -> Vec { + self.skills_dir(repo_root).into_iter().collect() + } + // ── Run-option capabilities (defaulted) ────────────────────────────────── /// The run options the generic `run` preflight may accept for this @@ -532,6 +541,23 @@ mod tests { ); } + #[test] + fn project_skill_dirs_include_cross_harness_roots_declared_by_the_descriptor() { + let root = Path::new("/repo"); + assert_eq!( + adapter_for(Harness::resolve("claude-code").unwrap()).project_skill_dirs(root), + vec![root.join(".claude/skills")] + ); + assert_eq!( + adapter_for(Harness::resolve("opencode").unwrap()).project_skill_dirs(root), + vec![ + root.join(".opencode/skills"), + root.join(".claude/skills"), + root.join(".agents/skills"), + ] + ); + } + #[test] fn only_codex_and_opencode_rewrite_frontmatter() { assert!(!adapter_for(Harness::resolve("claude-code").unwrap()).rewrites_frontmatter_name()); diff --git a/src/adapters/skill_shadow.rs b/src/adapters/skill_shadow.rs index d0da73a..f4fe93c 100644 --- a/src/adapters/skill_shadow.rs +++ b/src/adapters/skill_shadow.rs @@ -15,6 +15,9 @@ use serde::{Deserialize, Serialize}; use crate::core::fs::artifact_path; mod artifact; +#[cfg(test)] +mod codebase_tests; +mod grouping; mod resolution; pub(crate) mod verification; @@ -30,7 +33,16 @@ pub use verification::{ VerificationStatus, }; -pub const PLUGIN_SHADOW_SCHEMA_VERSION: u8 = 2; +pub const PLUGIN_SHADOW_SCHEMA_VERSION: u8 = 3; + +/// Which environment contributed the non-staged side of a finding. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "kebab-case")] +pub enum ShadowFindingClass { + #[default] + OperatorEnvironment, + CodebaseSourced, +} /// How a logical skill participates in the comparison. #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] @@ -333,6 +345,8 @@ pub(crate) fn severity_for( /// Every concrete source associated with one logical eval skill. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct ShadowFinding { + #[serde(default)] + pub class: ShadowFindingClass, pub skill_name: String, pub role: ShadowSkillRole, /// What the collision would mean if the live copy loaded. Set at detection @@ -355,123 +369,6 @@ pub struct PluginShadowReport { pub findings: Vec, } -impl PluginShadowReport { - pub(crate) fn from_sources(config_dir: impl Into, sources: Vec) -> Self { - let mut grouped: BTreeMap> = BTreeMap::new(); - for source in sources { - grouped - .entry(source.skill_name.clone()) - .or_default() - .push(source); - } - let findings = grouped - .into_iter() - .map(|(skill_name, mut sources)| { - sources.sort_by(|a, b| { - ( - &a.runtime_id, - &a.discovery_path, - a.origin == ShadowSourceOrigin::Staged, - ) - .cmp(&( - &b.runtime_id, - &b.discovery_path, - b.origin == ShadowSourceOrigin::Staged, - )) - }); - ShadowFinding { - skill_name, - role: ShadowSkillRole::Subject, - severity: ShadowSeverity::ComparisonInvalid, - sources, - resolved_severity: None, - } - }) - .collect(); - Self { - config_dir: config_dir.into(), - findings, - } - } - - pub(crate) fn from_observed_sources( - config_dir: impl Into, - sources: Vec, - subject_skill_name: &str, - expected_cells: &[(String, String)], - ) -> Self { - let mut merged = Vec::::new(); - for mut source in sources { - if let Some(existing) = merged - .iter_mut() - .find(|existing| existing.same_identity(&source)) - { - for appearance in source.appearances.drain(..) { - existing.add_appearance(appearance); - } - } else { - merged.push(source); - } - } - - let mut report = Self::from_sources(config_dir, merged); - let mut expected_by_group = BTreeMap::<&str, BTreeSet<&str>>::new(); - for (group, condition) in expected_cells { - expected_by_group - .entry(group) - .or_default() - .insert(condition); - } - for finding in &mut report.findings { - finding.role = if finding.skill_name == subject_skill_name { - ShadowSkillRole::Subject - } else { - ShadowSkillRole::Sibling - }; - let live_cells = finding - .sources - .iter() - .filter(|source| source.origin == ShadowSourceOrigin::Live) - .flat_map(|source| &source.appearances) - .map(|appearance| (appearance.group.as_str(), appearance.condition.as_str())) - .collect::>(); - finding.severity = severity_for(finding.role, &live_cells, &expected_by_group); - } - report - } - - pub(crate) fn is_empty(&self) -> bool { - self.findings.is_empty() - } - - #[cfg(test)] - pub(crate) fn source_count(&self) -> usize { - self.findings - .iter() - .map(|finding| finding.sources.len()) - .sum() - } - - #[cfg(test)] - pub(crate) fn sources(&self) -> impl Iterator { - self.findings.iter().flat_map(|finding| &finding.sources) - } - - pub(crate) fn into_sources(self) -> Vec { - self.findings - .into_iter() - .flat_map(|finding| finding.sources) - .collect() - } - - #[cfg(test)] - pub(crate) fn source(&self, index: usize) -> &ShadowSource { - self.sources() - .nth(index) - .expect("shadow source index should exist") - } -} - #[cfg(test)] mod tests { use super::*; @@ -530,7 +427,7 @@ mod tests { .unwrap() .get("shadowed") .is_none(), - "legacy input is normalized to v2 when reserialized" + "legacy input is normalized to v3 when reserialized" ); } @@ -538,7 +435,8 @@ mod tests { fn declared_isolation_is_serialized_with_the_shadow_report() { let artifact = PluginShadowArtifact::new(sample_report(), true); let value = serde_json::to_value(&artifact).unwrap(); - assert_eq!(value["schema_version"], 2); + assert_eq!(value["schema_version"], 3); + assert_eq!(value["findings"][0]["class"], "operator-environment"); assert_eq!(value["isolates_live_sources"], true); assert_eq!( value["findings"][0]["skill_name"], @@ -645,11 +543,12 @@ mod tests { } #[test] - fn v2_artifact_groups_sources_under_one_logical_finding() { + fn v3_artifact_groups_sources_under_one_logical_finding() { let artifact = PluginShadowArtifact::new( PluginShadowReport { config_dir: "/home/u/.config/opencode".into(), findings: vec![ShadowFinding { + class: ShadowFindingClass::OperatorEnvironment, skill_name: "mr-review".into(), role: ShadowSkillRole::Subject, severity: ShadowSeverity::ComparisonInvalid, @@ -687,7 +586,7 @@ mod tests { ); let value = serde_json::to_value(artifact).unwrap(); - assert_eq!(value["schema_version"], 2); + assert_eq!(value["schema_version"], 3); assert_eq!(value["findings"][0]["skill_name"], "mr-review"); assert_eq!( value["findings"][0]["sources"][0]["root"]["relation"], diff --git a/src/adapters/skill_shadow/artifact.rs b/src/adapters/skill_shadow/artifact.rs index fda5106..16b115c 100644 --- a/src/adapters/skill_shadow/artifact.rs +++ b/src/adapters/skill_shadow/artifact.rs @@ -5,8 +5,16 @@ use serde::{Deserialize, Deserializer, Serialize, Serializer}; use super::*; -/// The persisted v2 artifact. Deserialization also accepts the historical, -/// unversioned `shadowed` shape so old iterations remain aggregatable. +mod render; + +pub(crate) use render::format_isolated_shadow_notice; +use render::legacy_shadow_validity_warnings; +pub use render::{ + format_shadow_banner, format_shadow_banner_with_verification, shadow_validity_warnings, +}; + +/// The persisted v3 artifact. Deserialization also accepts schema v2 and the +/// unversioned `shadowed` shape so artifacts from that contract remain aggregatable. #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct PluginShadowArtifact { pub report: PluginShadowReport, @@ -40,6 +48,26 @@ impl PluginShadowArtifact { |sources| legacy_shadow_validity_warnings(sources), ) } + + pub(crate) fn validity_warnings_for_class(&self, class: ShadowFindingClass) -> Vec { + if let Some(sources) = &self.legacy_shadowed { + return if class == ShadowFindingClass::OperatorEnvironment { + legacy_shadow_validity_warnings(sources) + } else { + Vec::new() + }; + } + shadow_validity_warnings(&PluginShadowReport { + config_dir: self.report.config_dir.clone(), + findings: self + .report + .findings + .iter() + .filter(|finding| finding.class == class) + .cloned() + .collect(), + }) + } } #[derive(Serialize)] @@ -70,8 +98,9 @@ impl Serialize for PluginShadowArtifact { } #[derive(Deserialize)] -struct PluginShadowArtifactV2 { - schema_version: u8, +struct VersionedPluginShadowArtifact { + #[serde(rename = "schema_version")] + _schema_version: u8, config_dir: String, findings: Vec, #[serde(default)] @@ -159,12 +188,9 @@ impl<'de> Deserialize<'de> for PluginShadowArtifact { { let value = serde_json::Value::deserialize(deserializer)?; match value.get("schema_version").and_then(|value| value.as_u64()) { - Some(version) if version == u64::from(PLUGIN_SHADOW_SCHEMA_VERSION) => { - let artifact: PluginShadowArtifactV2 = + Some(2 | 3) => { + let artifact: VersionedPluginShadowArtifact = serde_json::from_value(value).map_err(D::Error::custom)?; - if artifact.schema_version != PLUGIN_SHADOW_SCHEMA_VERSION { - return Err(D::Error::custom("unsupported plugin-shadow schema version")); - } Ok(Self { report: PluginShadowReport { config_dir: artifact.config_dir, @@ -203,292 +229,3 @@ impl<'de> Deserialize<'de> for PluginShadowArtifact { fn is_false(value: &bool) -> bool { !*value } - -/// Informational build-time notice for a report whose resolved descriptor -/// asserts that every detected live source is isolated from dispatches. -pub(crate) fn format_isolated_shadow_notice(report: &PluginShadowReport, verifies: bool) -> String { - let count = report.findings.len(); - let finding = if count == 1 { "finding" } else { "findings" }; - let mut lines = vec![ - String::new(), - format!("ℹ Skill-shadow notice: preflight detected {count} live-source {finding}."), - " The resolved descriptor declares `[shadow] isolates_live_sources = true`, so" - .to_string(), - " the findings remain in plugin-shadow.json as informational provenance and".to_string(), - " will not become benchmark.json validity_warnings.".to_string(), - ]; - lines.push(if verifies { - // Saying "eval-magic does not verify this" would now be false for this - // harness: `ingest` checks the assertion against the transcripts. - " `ingest` checks this assertion against what each dispatch reported, and".to_string() - } else { - " eval-magic cannot verify this assertion for this harness; it must cover".to_string() - }); - lines.push(if verifies { - " `aggregate` reports any contradiction.".to_string() - } else { - " every initial and resumed eval-agent dispatch.".to_string() - }); - lines.push(" How to confirm it holds: `eval-magic docs isolation`.".to_string()); - lines.join("\n") -} - -fn source_label(source: &ShadowSource) -> String { - match &source.plugin { - Some(plugin) => format!("enabled plugin '{plugin}'"), - None => format!( - "{} {:?} skill root '{}'", - relation_label(source.root.relation), - source.root.scope, - source.root.path - ), - } -} - -/// Join distinct entries in first-seen order — two cached versions of one -/// installed plugin yield separate sources sharing a label and a remediation, so -/// joining verbatim says the same thing twice. Order-preserving rather than -/// sorted: these strings land in `benchmark.json` and must match the order the -/// banner prints its sources in. -fn join_distinct(values: impl Iterator, separator: &str) -> String { - let mut distinct: Vec = Vec::new(); - for value in values { - if !distinct.contains(&value) { - distinct.push(value); - } - } - distinct.join(separator) -} - -/// One `validity_warnings` entry per grouped logical skill. -/// -/// A finding whose evidence refuted every live source produces **nothing**: the -/// dispatches demonstrably did not load it, so there is no threat to report. -/// Everything else reports, and says which of the three it is — confirmed by -/// transcripts, or detected but unverifiable — because "we saw it happen" and -/// "we could not tell" call for different responses from the operator. -pub fn shadow_validity_warnings(report: &PluginShadowReport) -> Vec { - report - .findings - .iter() - .filter(|finding| { - finding.resolved_severity != Some(verification::ShadowResolvedSeverity::Isolated) - }) - .map(|finding| { - let sources = join_distinct( - finding - .sources - .iter() - .filter(|source| source.origin == ShadowSourceOrigin::Live) - .map(source_label), - ", ", - ); - let remediation = join_distinct( - finding - .sources - .iter() - .filter_map(|source| source.remediation.clone()), - " ", - ); - let role = role_label(finding.role); - let skill = &finding.skill_name; - let lead = match finding.resolved_severity { - Some(resolved) => { - let severity = resolved_severity_label(resolved); - match verification::finding_status(finding) { - verification::VerificationStatus::Confirmed => { - let cells = verification::confirmed_cells(finding).join(", "); - let n = verification::confirming_dispatch_count(finding); - let plural = if n == 1 { "" } else { "s" }; - format!( - "{severity}: staged {role} skill '{skill}' was actually loaded \ - from {sources} in {cells} (verified from {n} dispatch \ - transcript{plural})." - ) - } - _ => unverified_lead(severity, role, skill, &sources, finding), - } - } - None => format!( - "{}: staged {role} skill '{skill}' is also discoverable from {sources}.", - severity_label(finding.severity) - ), - }; - format!("{lead} {remediation} See `eval-magic docs isolation`.") - .trim() - .to_string() - }) - .collect() -} - -/// The lead sentence for a finding evidence could not settle, naming why. -fn unverified_lead( - severity: &str, - role: &str, - skill: &str, - sources: &str, - finding: &ShadowFinding, -) -> String { - let reason = verification::inconclusive_reason(finding) - .unwrap_or_else(|| "no dispatch reported its skill/plugin surface".to_string()); - format!( - "{severity} (unverified): staged {role} skill '{skill}' is discoverable from {sources}, \ - and eval-magic could not verify whether dispatches loaded it — {reason}. Treat the \ - comparison as affected until each dispatch isolates the source or a transcript shows it \ - did not load." - ) -} - -fn legacy_shadow_validity_warnings(sources: &[LegacyShadowSource]) -> Vec { - sources - .iter() - .map(|source| { - format!( - "staged skill '{}' is also provided by {} — each claude -p dispatch could discover \ - both copies, so with/without results may be contaminated. Isolate each dispatch's \ - Claude config: add --setting-sources project,local to drop user-scope plugins, \ - disable the plugin in enabledPlugins settings, or run under a clean \ - CLAUDE_CONFIG_DIR.", - source.skill_name(), - source.source_label(), - ) - }) - .collect() -} - -/// Shared build-time banner. Empty when nothing is shadowed. -/// -/// Nothing has dispatched yet, so this states a *risk*, not a verdict. Whether a -/// dispatch actually loads one of these depends on its own config isolation, -/// which eval-magic reads from the transcript during `ingest` rather than -/// inferring from command templates. Printing "comparison invalid" here would -/// convict a correctly-isolated run before it ran a single task (issue #207). -/// -/// `verifies` reflects whether this harness's transcripts can settle it. -pub fn format_shadow_banner_with_verification( - report: &PluginShadowReport, - verifies: bool, -) -> String { - if report.findings.is_empty() { - return String::new(); - } - let mut lines = vec![ - String::new(), - "⚠ Skill-shadow preflight: live copies of staged eval skills are installed in this" - .to_string(), - " operator environment. Whether a dispatch loads them depends on that dispatch's own" - .to_string(), - " config isolation. At risk unless every dispatch isolates these sources:".to_string(), - ]; - for finding in &report.findings { - lines.push(format!( - " • [{}] {} — {}", - role_label(finding.role), - finding.skill_name, - consequence(finding.severity) - )); - for source in finding - .sources - .iter() - .filter(|source| source.origin == ShadowSourceOrigin::Live) - { - lines.push(format!( - " - {} [{}; runtime id '{}']", - source_label(source), - relation_label(source.root.relation), - source.runtime_id - )); - if !source.appearances.is_empty() { - let cells = source - .appearances - .iter() - .map(|appearance| { - format!( - "{}/{} ({})", - appearance.group, - appearance.condition, - resolution_label(appearance.resolution) - ) - }) - .collect::>() - .join(", "); - lines.push(format!(" expected in: {cells}")); - } - if let Some(remediation) = &source.remediation { - lines.push(format!(" remediation: {remediation}")); - } - } - } - lines.push(" See plugin-shadow.json for canonical paths and full provenance.".to_string()); - lines.push(if verifies { - " `ingest` records what each dispatch actually loaded and `aggregate` reports the \ - verified verdict." - .to_string() - } else { - " This harness's transcripts do not report the session's skill/plugin surface, so \ - eval-magic cannot verify isolation." - .to_string() - }); - lines.push( - " Per-harness isolation recipes, and how to verify one worked: \ - `eval-magic docs isolation`." - .to_string(), - ); - lines.join("\n") -} - -/// Banner for a caller with no harness context. Defaults to "cannot verify", -/// the conservative direction: understating what eval-magic can settle is safe, -/// promising a verdict it will never produce is not. Prefer -/// [`format_shadow_banner_with_verification`] wherever the harness is known. -pub fn format_shadow_banner(report: &PluginShadowReport) -> String { - format_shadow_banner_with_verification(report, false) -} - -/// What the collision would cost if the live copy did load. Conditional mood on -/// purpose — at banner time it has not happened yet. -fn consequence(severity: ShadowSeverity) -> &'static str { - match severity { - ShadowSeverity::Warning => "would weaken the comparison if loaded", - ShadowSeverity::ComparisonInvalid => "would invalidate the comparison if loaded", - } -} - -fn severity_label(severity: ShadowSeverity) -> &'static str { - match severity { - ShadowSeverity::Warning => "warning", - ShadowSeverity::ComparisonInvalid => "comparison invalid", - } -} - -fn resolved_severity_label(severity: verification::ShadowResolvedSeverity) -> &'static str { - match severity { - verification::ShadowResolvedSeverity::Isolated => "isolated", - verification::ShadowResolvedSeverity::Warning => "warning", - verification::ShadowResolvedSeverity::ComparisonInvalid => "comparison invalid", - } -} - -fn role_label(role: ShadowSkillRole) -> &'static str { - match role { - ShadowSkillRole::Subject => "subject", - ShadowSkillRole::Sibling => "sibling", - } -} - -fn relation_label(relation: ShadowRelation) -> &'static str { - match relation { - ShadowRelation::Native => "native", - ShadowRelation::CrossHarness => "cross-harness", - ShadowRelation::Unknown => "unknown", - } -} - -fn resolution_label(resolution: ShadowResolution) -> &'static str { - match resolution { - ShadowResolution::Selected => "selected", - ShadowResolution::Shadowed => "shadowed", - ShadowResolution::Coexisting => "coexisting", - ShadowResolution::Unknown => "unknown", - } -} diff --git a/src/adapters/skill_shadow/artifact/render.rs b/src/adapters/skill_shadow/artifact/render.rs new file mode 100644 index 0000000..4d6d09a --- /dev/null +++ b/src/adapters/skill_shadow/artifact/render.rs @@ -0,0 +1,317 @@ +//! User-facing rendering for persisted shadow reports. + +use super::*; + +/// Informational build-time notice for a report whose resolved descriptor +/// asserts that every detected live source is isolated from dispatches. +pub(crate) fn format_isolated_shadow_notice(report: &PluginShadowReport, verifies: bool) -> String { + let count = report.findings.len(); + let finding = if count == 1 { "finding" } else { "findings" }; + let mut lines = vec![ + String::new(), + format!("ℹ Skill-shadow notice: preflight detected {count} live-source {finding}."), + " The resolved descriptor declares `[shadow] isolates_live_sources = true`, so" + .to_string(), + " the findings remain in plugin-shadow.json as informational provenance and".to_string(), + " will not become benchmark.json validity_warnings.".to_string(), + ]; + lines.push(if verifies { + // Saying "eval-magic does not verify this" would now be false for this + // harness: `ingest` checks the assertion against the transcripts. + " `ingest` checks this assertion against what each dispatch reported, and".to_string() + } else { + " eval-magic cannot verify this assertion for this harness; it must cover".to_string() + }); + lines.push(if verifies { + " `aggregate` reports any contradiction.".to_string() + } else { + " every initial and resumed eval-agent dispatch.".to_string() + }); + lines.push(" How to confirm it holds: `eval-magic docs isolation`.".to_string()); + lines.join("\n") +} + +fn source_label(source: &ShadowSource) -> String { + match &source.plugin { + Some(plugin) => format!("enabled plugin '{plugin}'"), + None => format!( + "{} {:?} skill root '{}'", + relation_label(source.root.relation), + source.root.scope, + source.root.path + ), + } +} + +/// Join distinct entries in first-seen order — two cached versions of one +/// installed plugin yield separate sources sharing a label and a remediation, so +/// joining verbatim says the same thing twice. Order-preserving rather than +/// sorted: these strings land in `benchmark.json` and must match the order the +/// banner prints its sources in. +fn join_distinct(values: impl Iterator, separator: &str) -> String { + let mut distinct: Vec = Vec::new(); + for value in values { + if !distinct.contains(&value) { + distinct.push(value); + } + } + distinct.join(separator) +} + +/// One `validity_warnings` entry per grouped logical skill. +/// +/// A finding whose evidence refuted every live source produces **nothing**: the +/// dispatches demonstrably did not load it, so there is no threat to report. +/// Everything else reports, and says which of the three it is — confirmed by +/// transcripts, or detected but unverifiable — because "we saw it happen" and +/// "we could not tell" call for different responses from the operator. +pub fn shadow_validity_warnings(report: &PluginShadowReport) -> Vec { + report + .findings + .iter() + .filter(|finding| { + finding.resolved_severity != Some(verification::ShadowResolvedSeverity::Isolated) + }) + .map(|finding| { + let sources = join_distinct( + finding + .sources + .iter() + .filter(|source| source.origin == ShadowSourceOrigin::Live) + .map(source_label), + ", ", + ); + let remediation = join_distinct( + finding + .sources + .iter() + .filter_map(|source| source.remediation.clone()), + " ", + ); + let role = role_label(finding.role); + let skill = &finding.skill_name; + let lead = match finding.resolved_severity { + Some(resolved) => { + let severity = resolved_severity_label(resolved); + match verification::finding_status(finding) { + verification::VerificationStatus::Confirmed => { + let cells = verification::confirmed_cells(finding).join(", "); + let n = verification::confirming_dispatch_count(finding); + let plural = if n == 1 { "" } else { "s" }; + format!( + "{severity}: staged {role} skill '{skill}' was actually loaded \ + from {sources} in {cells} (verified from {n} dispatch \ + transcript{plural})." + ) + } + _ => unverified_lead(severity, role, skill, &sources, finding), + } + } + None => format!( + "{}: staged {role} skill '{skill}' is also discoverable from {sources}.", + severity_label(finding.severity) + ), + }; + format!("{lead} {remediation} See `eval-magic docs isolation`.") + .trim() + .to_string() + }) + .collect() +} + +/// The lead sentence for a finding evidence could not settle, naming why. +fn unverified_lead( + severity: &str, + role: &str, + skill: &str, + sources: &str, + finding: &ShadowFinding, +) -> String { + let reason = verification::inconclusive_reason(finding) + .unwrap_or_else(|| "no dispatch reported its skill/plugin surface".to_string()); + format!( + "{severity} (unverified): staged {role} skill '{skill}' is discoverable from {sources}, \ + and eval-magic could not verify whether dispatches loaded it — {reason}. Treat the \ + comparison as affected until each dispatch isolates the source or a transcript shows it \ + did not load." + ) +} + +pub(super) fn legacy_shadow_validity_warnings(sources: &[LegacyShadowSource]) -> Vec { + sources + .iter() + .map(|source| { + format!( + "staged skill '{}' is also provided by {} — each claude -p dispatch could discover \ + both copies, so with/without results may be contaminated. Isolate each dispatch's \ + Claude config: add --setting-sources project,local to drop user-scope plugins, \ + disable the plugin in enabledPlugins settings, or run under a clean \ + CLAUDE_CONFIG_DIR.", + source.skill_name(), + source.source_label(), + ) + }) + .collect() +} + +/// Shared build-time banner. Empty when nothing is shadowed. +/// +/// Nothing has dispatched yet, so this states a *risk*, not a verdict. Whether a +/// dispatch actually loads one of these depends on its own config isolation, +/// which eval-magic reads from the transcript during `ingest` rather than +/// inferring from command templates. Printing "comparison invalid" here would +/// convict a correctly-isolated run before it ran a single task (issue #207). +/// +/// `verifies` reflects whether this harness's transcripts can settle it. +pub fn format_shadow_banner_with_verification( + report: &PluginShadowReport, + verifies: bool, +) -> String { + if report.findings.is_empty() { + return String::new(); + } + let has_operator = report + .findings + .iter() + .any(|finding| finding.class == ShadowFindingClass::OperatorEnvironment); + let has_codebase = report + .findings + .iter() + .any(|finding| finding.class == ShadowFindingClass::CodebaseSourced); + let mut lines = vec![String::new()]; + match (has_operator, has_codebase) { + (true, false) => lines.extend([ + "⚠ Skill-shadow preflight: live copies of evaluated skills are installed in this" + .to_string(), + " operator environment. Whether a dispatch loads them depends on that dispatch's own" + .to_string(), + " config isolation. At risk unless every dispatch isolates these sources:" + .to_string(), + ]), + (false, true) => lines.extend([ + "⚠ Skill-shadow preflight: project-local copies of evaluated skills remain discoverable" + .to_string(), + " from the sourced codebase. At risk unless those sources are excluded or displaced:" + .to_string(), + ]), + (true, true) => lines.extend([ + "⚠ Skill-shadow preflight: evaluated skills have additional discoverable copies in the" + .to_string(), + " operator environment and the sourced codebase. At risk unless every source is isolated" + .to_string(), + " or excluded:".to_string(), + ]), + (false, false) => unreachable!("an empty report returned above"), + } + for finding in &report.findings { + lines.push(format!( + " • [{}] {} — {}", + role_label(finding.role), + finding.skill_name, + consequence(finding.severity) + )); + for source in finding + .sources + .iter() + .filter(|source| source.origin == ShadowSourceOrigin::Live) + { + lines.push(format!( + " - {} [{}; runtime id '{}']", + source_label(source), + relation_label(source.root.relation), + source.runtime_id + )); + if !source.appearances.is_empty() { + let cells = source + .appearances + .iter() + .map(|appearance| { + format!( + "{}/{} ({})", + appearance.group, + appearance.condition, + resolution_label(appearance.resolution) + ) + }) + .collect::>() + .join(", "); + lines.push(format!(" expected in: {cells}")); + } + if let Some(remediation) = &source.remediation { + lines.push(format!(" remediation: {remediation}")); + } + } + } + lines.push(" See plugin-shadow.json for canonical paths and full provenance.".to_string()); + lines.push(if verifies { + " `ingest` records what each dispatch actually loaded and `aggregate` reports the \ + verified verdict." + .to_string() + } else { + " This harness's transcripts do not report the session's skill/plugin surface, so \ + eval-magic cannot verify isolation." + .to_string() + }); + lines.push( + " Per-harness isolation recipes, and how to verify one worked: \ + `eval-magic docs isolation`." + .to_string(), + ); + lines.join("\n") +} + +/// Banner for a caller with no harness context. Defaults to "cannot verify", +/// the conservative direction: understating what eval-magic can settle is safe, +/// promising a verdict it will never produce is not. Prefer +/// [`format_shadow_banner_with_verification`] wherever the harness is known. +pub fn format_shadow_banner(report: &PluginShadowReport) -> String { + format_shadow_banner_with_verification(report, false) +} + +/// What the collision would cost if the live copy did load. Conditional mood on +/// purpose — at banner time it has not happened yet. +fn consequence(severity: ShadowSeverity) -> &'static str { + match severity { + ShadowSeverity::Warning => "would weaken the comparison if loaded", + ShadowSeverity::ComparisonInvalid => "would invalidate the comparison if loaded", + } +} + +fn severity_label(severity: ShadowSeverity) -> &'static str { + match severity { + ShadowSeverity::Warning => "warning", + ShadowSeverity::ComparisonInvalid => "comparison invalid", + } +} + +fn resolved_severity_label(severity: verification::ShadowResolvedSeverity) -> &'static str { + match severity { + verification::ShadowResolvedSeverity::Isolated => "isolated", + verification::ShadowResolvedSeverity::Warning => "warning", + verification::ShadowResolvedSeverity::ComparisonInvalid => "comparison invalid", + } +} + +fn role_label(role: ShadowSkillRole) -> &'static str { + match role { + ShadowSkillRole::Subject => "subject", + ShadowSkillRole::Sibling => "sibling", + } +} + +fn relation_label(relation: ShadowRelation) -> &'static str { + match relation { + ShadowRelation::Native => "native", + ShadowRelation::CrossHarness => "cross-harness", + ShadowRelation::Unknown => "unknown", + } +} + +fn resolution_label(resolution: ShadowResolution) -> &'static str { + match resolution { + ShadowResolution::Selected => "selected", + ShadowResolution::Shadowed => "shadowed", + ShadowResolution::Coexisting => "coexisting", + ShadowResolution::Unknown => "unknown", + } +} diff --git a/src/adapters/skill_shadow/codebase_tests.rs b/src/adapters/skill_shadow/codebase_tests.rs new file mode 100644 index 0000000..b14ba42 --- /dev/null +++ b/src/adapters/skill_shadow/codebase_tests.rs @@ -0,0 +1,104 @@ +use super::*; + +fn sample_report() -> PluginShadowReport { + PluginShadowReport::from_sources( + "/x", + vec![ShadowSource { + kind: ShadowSourceKind::Plugin, + origin: ShadowSourceOrigin::Live, + skill_name: "subject".into(), + runtime_id: "plugin:subject".into(), + plugin: Some("plugin@example".into()), + discovery_path: "/plugins/example/subject".into(), + canonical_path: None, + root: ShadowRoot::unknown("/plugins/example"), + appearances: Vec::new(), + remediation: None, + verification: None, + }], + ) +} + +#[test] +fn validity_warnings_can_be_filtered_by_finding_class() { + let mut report = sample_report(); + report.findings[0].class = ShadowFindingClass::CodebaseSourced; + let artifact = PluginShadowArtifact::new(report, true); + + assert!( + artifact + .validity_warnings_for_class(ShadowFindingClass::OperatorEnvironment) + .is_empty() + ); + assert_eq!( + artifact + .validity_warnings_for_class(ShadowFindingClass::CodebaseSourced) + .len(), + 1 + ); +} + +#[test] +fn codebase_finding_banner_names_the_sourced_codebase_not_operator_environment() { + let mut report = sample_report(); + report.findings[0].class = ShadowFindingClass::CodebaseSourced; + + let banner = format_shadow_banner(&report); + + assert!(banner.contains("sourced codebase"), "{banner}"); + assert!(!banner.contains("operator environment"), "{banner}"); +} + +#[test] +fn v2_artifact_defaults_findings_to_the_operator_environment_class() { + let artifact: PluginShadowArtifact = serde_json::from_value(serde_json::json!({ + "schema_version": 2, + "config_dir": "/home/u/.claude", + "findings": [{ + "skill_name": "mr-review", + "role": "subject", + "severity": "comparison-invalid", + "sources": [] + }] + })) + .unwrap(); + + assert_eq!( + artifact.report.findings[0].class, + ShadowFindingClass::OperatorEnvironment + ); + assert_eq!( + serde_json::to_value(artifact).unwrap()["schema_version"], + PLUGIN_SHADOW_SCHEMA_VERSION + ); +} + +#[test] +fn observed_codebase_sources_keep_their_distinct_finding_class() { + let source = ShadowSource::live_skill( + "subject", + Path::new("/repo/.claude/skills/subject"), + ShadowRoot { + scope: ShadowRootScope::Project, + namespace: ShadowNamespace::Claude, + plugin: None, + path: "/repo/.claude/skills".into(), + relation: ShadowRelation::Native, + }, + "Set `codebase.exclude_skill_sources = true` for this eval.", + ); + + let report = PluginShadowReport::from_observed_sources_with_class( + "/repo", + vec![source], + "subject", + &[("g1".into(), "with_skill".into())], + ShadowFindingClass::CodebaseSourced, + ); + + assert_eq!(report.findings.len(), 1); + assert_eq!( + report.findings[0].class, + ShadowFindingClass::CodebaseSourced + ); +} diff --git a/src/adapters/skill_shadow/grouping.rs b/src/adapters/skill_shadow/grouping.rs new file mode 100644 index 0000000..6ddc06c --- /dev/null +++ b/src/adapters/skill_shadow/grouping.rs @@ -0,0 +1,138 @@ +//! Group concrete discovery sources into logical skill findings. + +use super::*; + +impl PluginShadowReport { + pub(crate) fn from_sources(config_dir: impl Into, sources: Vec) -> Self { + let mut grouped: BTreeMap> = BTreeMap::new(); + for source in sources { + grouped + .entry(source.skill_name.clone()) + .or_default() + .push(source); + } + let findings = grouped + .into_iter() + .map(|(skill_name, mut sources)| { + sources.sort_by(|a, b| { + ( + &a.runtime_id, + &a.discovery_path, + a.origin == ShadowSourceOrigin::Staged, + ) + .cmp(&( + &b.runtime_id, + &b.discovery_path, + b.origin == ShadowSourceOrigin::Staged, + )) + }); + ShadowFinding { + class: ShadowFindingClass::OperatorEnvironment, + skill_name, + role: ShadowSkillRole::Subject, + severity: ShadowSeverity::ComparisonInvalid, + sources, + resolved_severity: None, + } + }) + .collect(); + Self { + config_dir: config_dir.into(), + findings, + } + } + + pub(crate) fn from_observed_sources( + config_dir: impl Into, + sources: Vec, + subject_skill_name: &str, + expected_cells: &[(String, String)], + ) -> Self { + Self::from_observed_sources_with_class( + config_dir, + sources, + subject_skill_name, + expected_cells, + ShadowFindingClass::OperatorEnvironment, + ) + } + + pub(crate) fn from_observed_sources_with_class( + config_dir: impl Into, + sources: Vec, + subject_skill_name: &str, + expected_cells: &[(String, String)], + class: ShadowFindingClass, + ) -> Self { + let mut merged = Vec::::new(); + for mut source in sources { + if let Some(existing) = merged + .iter_mut() + .find(|existing| existing.same_identity(&source)) + { + for appearance in source.appearances.drain(..) { + existing.add_appearance(appearance); + } + } else { + merged.push(source); + } + } + + let mut report = Self::from_sources(config_dir, merged); + let mut expected_by_group = BTreeMap::<&str, BTreeSet<&str>>::new(); + for (group, condition) in expected_cells { + expected_by_group + .entry(group) + .or_default() + .insert(condition); + } + for finding in &mut report.findings { + finding.class = class; + finding.role = if finding.skill_name == subject_skill_name { + ShadowSkillRole::Subject + } else { + ShadowSkillRole::Sibling + }; + let live_cells = finding + .sources + .iter() + .filter(|source| source.origin == ShadowSourceOrigin::Live) + .flat_map(|source| &source.appearances) + .map(|appearance| (appearance.group.as_str(), appearance.condition.as_str())) + .collect::>(); + finding.severity = severity_for(finding.role, &live_cells, &expected_by_group); + } + report + } + + pub(crate) fn is_empty(&self) -> bool { + self.findings.is_empty() + } + + #[cfg(test)] + pub(crate) fn source_count(&self) -> usize { + self.findings + .iter() + .map(|finding| finding.sources.len()) + .sum() + } + + #[cfg(test)] + pub(crate) fn sources(&self) -> impl Iterator { + self.findings.iter().flat_map(|finding| &finding.sources) + } + + pub(crate) fn into_sources(self) -> Vec { + self.findings + .into_iter() + .flat_map(|finding| finding.sources) + .collect() + } + + #[cfg(test)] + pub(crate) fn source(&self, index: usize) -> &ShadowSource { + self.sources() + .nth(index) + .expect("shadow source index should exist") + } +} diff --git a/src/adapters/skill_shadow/verification/tests.rs b/src/adapters/skill_shadow/verification/tests.rs index 215850b..de760a9 100644 --- a/src/adapters/skill_shadow/verification/tests.rs +++ b/src/adapters/skill_shadow/verification/tests.rs @@ -2,8 +2,8 @@ use super::*; use crate::adapters::skill_shadow::{ - ShadowAppearance, ShadowNamespace, ShadowRelation, ShadowResolution, ShadowRoot, - ShadowRootScope, ShadowSkillRole, + ShadowAppearance, ShadowFindingClass, ShadowNamespace, ShadowRelation, ShadowResolution, + ShadowRoot, ShadowRootScope, ShadowSkillRole, }; /// One dispatch's evidence, as the policy sees it. @@ -174,6 +174,7 @@ fn staged_source(skill: &str, dir_name: &str, cells: &[(&str, &str)]) -> ShadowS fn finding(role: ShadowSkillRole, sources: Vec) -> ShadowFinding { ShadowFinding { + class: ShadowFindingClass::OperatorEnvironment, skill_name: sources[0].skill_name.clone(), role, severity: match role { diff --git a/src/cli/args.rs b/src/cli/args.rs index b622ee7..2cce251 100644 --- a/src/cli/args.rs +++ b/src/cli/args.rs @@ -623,13 +623,15 @@ pub(crate) enum Commands { /// and rebuilding an explicit iteration resets prior Git history, branches, /// and remotes before dispatch. /// - /// Before dispatch, a shadow preflight scans every task environment for live - /// copies of the staged skills — installed plugins, global and cross-harness - /// skill directories — and warns when one could contaminate the comparison. It - /// reports what is discoverable, not what a dispatch loaded: eval-magic never - /// reads your command templates, so a remedy you applied is invisible to it. - /// Isolating each dispatch from those sources, and confirming it worked, is - /// `eval-magic docs isolation`. + /// Before dispatch, a shadow preflight scans every task environment for other + /// discoverable copies of evaluated skills. Schema-v3 `plugin-shadow.json` + /// distinguishes operator-environment sources (installed plugins, global and + /// cross-harness directories) from project skills preserved from a sourced + /// codebase. A codebase keeps its instructions, harness config, and project + /// skills by default; set its `exclude_skill_sources` field to remove only the + /// 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. Run(RunArgs), /// Run every task in a prepared iteration through its harness CLI. /// @@ -789,15 +791,18 @@ pub(crate) enum Commands { /// `validity_warnings` (including incomplete timing sample counts, one per /// task in `guard-denials.json`, and one per task in /// `permission-denials.json` whose refusals were not the guard's own, plus - /// grouped findings in schema-v2 `plugin-shadow.json` (legacy unversioned - /// reports remain readable) unless it records the resolved descriptor's - /// `isolates_live_sources = true` assertion), and raw per-run files/lines/hunks - /// from `diff-scope.json`. Each run's changed-file list and its `diff.patch` - /// stay in the run directory. Shadow findings retain their intrinsic warning or - /// comparison-invalid severity, per-cell appearances, resolution, and - /// remediation. A timing metric with `n: 0` is unavailable, not a measured - /// zero. The top-level `diff_scope` field is omitted for compatible older - /// iterations that predate metric capture. + /// grouped findings in schema-v3 `plugin-shadow.json` (v2 and legacy + /// unversioned reports remain readable). Findings distinguish + /// `operator-environment` sources from `codebase-sourced` project skills. + /// The resolved descriptor's `isolates_live_sources = true` assertion + /// suppresses only operator-environment findings; codebase findings require + /// the eval's separate `codebase.exclude_skill_sources` policy. The benchmark + /// also carries raw per-run files/lines/hunks from `diff-scope.json`. Each run's + /// changed-file list and its `diff.patch` stay in the run directory. Shadow + /// findings retain their intrinsic warning or comparison-invalid severity, + /// per-cell appearances, resolution, and remediation. A timing metric with + /// `n: 0` is unavailable, not a measured zero. The top-level `diff_scope` field + /// is omitted for compatible older iterations that predate metric capture. /// /// Read `validity_warnings` before trusting the delta. Raw `diff_scope` entries /// are diagnostic context rather than an optimization target: smaller is not diff --git a/src/cli/commands/workspace.rs b/src/cli/commands/workspace.rs index 0abda92..4b07b1a 100644 --- a/src/cli/commands/workspace.rs +++ b/src/cli/commands/workspace.rs @@ -3,6 +3,7 @@ use std::path::Path; +use crate::adapters::adapter_for; use crate::cli::args::{CommonArgs, PromoteBaselineArgs, SnapshotArgs}; use crate::cli::{ command_target_args, iteration_dir, resolve_iteration, run_context_from, staged_env_roots, @@ -97,6 +98,9 @@ pub(crate) fn run_teardown(args: CommonArgs) -> anyhow::Result<()> { if let Ok(dir) = iteration_dir(&ctx, args.iteration) { for env in staged_env_roots(&dir) { torn |= sandbox::teardown_guard(&env); + if adapter_for(ctx.harness).skills_dir(&env).is_some() { + crate::cli::run::staging::cleanup_staged_skills(&env, ctx.harness)?; + } } } let ws = workspace::cleanup_workspace(&ctx.workspace_root, &ctx.skill_name); diff --git a/src/cli/run/dispatch.rs b/src/cli/run/dispatch.rs index db8cef3..3f4d449 100644 --- a/src/cli/run/dispatch.rs +++ b/src/cli/run/dispatch.rs @@ -15,8 +15,8 @@ use serde::{Deserialize, Serialize}; use crate::adapters::{CliManifestContext, adapter_for}; use crate::core::fs::artifact_path; use crate::core::{ - AvailableSkill, Eval, GuardPolicyConfig, Harness, POSIX_TOOLING_REQUIREMENT, ResponderPolicy, - ScriptedTurn, SkillSource, SourceRecord, + AvailableSkill, CodebaseRecord, Eval, GuardPolicyConfig, Harness, POSIX_TOOLING_REQUIREMENT, + ResponderPolicy, ScriptedTurn, SkillSource, }; use super::RunError; @@ -59,7 +59,7 @@ pub struct DispatchTask { /// 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, + pub codebase: Option, /// The skill under test this task stages, as the run resolved it. #[serde(default, skip_serializing_if = "Option::is_none")] pub skill_source: Option, @@ -116,7 +116,7 @@ pub struct DispatchTaskOpts<'a> { /// 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 SourceRecord>, + pub codebase: Option<&'a CodebaseRecord>, /// The skill under test this task stages, if any. pub skill_source: Option<&'a SkillSource>, /// The responder policy this eval declares, if any. diff --git a/src/cli/run/orchestrate/mod.rs b/src/cli/run/orchestrate/mod.rs index 4a93b51..5acf13b 100644 --- a/src/cli/run/orchestrate/mod.rs +++ b/src/cli/run/orchestrate/mod.rs @@ -14,12 +14,13 @@ use std::collections::{BTreeMap, BTreeSet}; use std::path::{Path, PathBuf}; +use crate::adapters::skill_shadow::ShadowSource; use crate::adapters::{CliDispatchContext, adapter_for}; use crate::cli::command_target_args; use crate::core::fs::artifact_path; use crate::core::{ - CodebaseSource, CodebaseUse, Eval, GuardPolicyConfig, Mode, RunContext, SkillSource, - SourceKind, SourceRecord, + CodebaseRecord, CodebaseSource, CodebaseUse, Eval, GuardPolicyConfig, Mode, RunContext, + SkillSource, SourceKind, SourceRecord, }; use crate::source::ResolvedSource; @@ -154,26 +155,29 @@ impl RunSkill { 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) -> SourceRecord { - SourceRecord { - kind: match self.declared { - CodebaseSource::Git { .. } => SourceKind::Git, - CodebaseSource::Path { .. } => SourceKind::Path, + fn record(&self) -> CodebaseRecord { + CodebaseRecord { + source: SourceRecord { + kind: match self.declared { + CodebaseSource::Git { .. } => SourceKind::Git, + CodebaseSource::Path { .. } => SourceKind::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, + // Materialization checks out a commit, so the environment never + // carries uncommitted work however the source directory looked. + dirty: false, }, - 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, - // Materialization checks out a commit, so the environment never - // carries uncommitted work however the source directory looked. - dirty: false, + exclude_skill_sources: self.declared.exclude_skill_sources(), } } @@ -226,6 +230,9 @@ struct Staged { bootstrap_content: Option, plan_mode_content: Option, guard_policies: std::collections::HashMap, + /// Matching project skill sources inventoried from each sourced codebase + /// before exclusion or staging changes its discovery roots. + codebase_shadow_sources: std::collections::HashMap>, } /// Build the iteration workspace and dispatch plan for a run. diff --git a/src/cli/run/orchestrate/resolve.rs b/src/cli/run/orchestrate/resolve.rs index c100e8a..21e4ccc 100644 --- a/src/cli/run/orchestrate/resolve.rs +++ b/src/cli/run/orchestrate/resolve.rs @@ -47,11 +47,11 @@ fn resolve_codebases( } let spec = match declared { - CodebaseSource::Git { url, reference } => SourceSpec::Git { + CodebaseSource::Git { url, reference, .. } => SourceSpec::Git { url: url.clone(), reference: reference.clone(), }, - CodebaseSource::Path { path } => SourceSpec::Path { path: path.clone() }, + CodebaseSource::Path { path, .. } => SourceSpec::Path { path: path.clone() }, }; let source = resolve_source(&spec, &base_dir, "codebase") .map_err(|error| RunError::msg(format!("eval '{}': {error}", eval.id)))?; diff --git a/src/cli/run/orchestrate/shadow_preflight.rs b/src/cli/run/orchestrate/shadow_preflight.rs index c482fa2..968f78c 100644 --- a/src/cli/run/orchestrate/shadow_preflight.rs +++ b/src/cli/run/orchestrate/shadow_preflight.rs @@ -1,19 +1,130 @@ //! Assemble the harness-neutral skill-shadow report across every comparison cell. use std::collections::BTreeSet; +use std::fs; +use std::path::Path; -use crate::adapters::adapter_for; use crate::adapters::skill_shadow::{ - PluginShadowArtifact, PluginShadowReport, ShadowAppearance, ShadowResolution, ShadowRoot, - ShadowSource, format_isolated_shadow_notice, format_shadow_banner_with_verification, + PluginShadowArtifact, PluginShadowReport, ShadowAppearance, ShadowFindingClass, + ShadowNamespace, ShadowRelation, ShadowResolution, ShadowRoot, ShadowRootScope, ShadowSource, + format_isolated_shadow_notice, format_shadow_banner_with_verification, }; -use crate::core::RunContext; +use crate::adapters::{HarnessAdapter, adapter_for}; +use crate::core::fs::artifact_path; +use crate::core::{Harness, RunContext}; use crate::pipeline::shadow_verification::write_verified; use super::envs::EnvTarget; use super::{Resolved, RunOptions, Staged}; use crate::cli::run::RunError; +/// Inventory evaluated skill names from every project root the selected harness +/// discovers. This runs immediately after codebase provisioning, before opt-in +/// exclusion or eval staging can remove/replace a source. +pub(super) fn scan_codebase_skill_sources( + repo_root: &Path, + harness: Harness, + evaluated_names: &[&str], +) -> Vec { + let adapter = adapter_for(harness); + let native = adapter.skills_dir(repo_root); + let evaluated = evaluated_names.iter().copied().collect::>(); + let mut sources = Vec::new(); + for root in adapter.project_skill_dirs(repo_root) { + let Ok(entries) = fs::read_dir(&root) else { + continue; + }; + for entry in entries.flatten() { + let path = entry.path(); + if !path.is_dir() || !path.join("SKILL.md").is_file() { + continue; + } + let folder_name = entry.file_name().to_string_lossy().into_owned(); + let frontmatter_name = frontmatter_name(&path.join("SKILL.md")); + let Some(skill_name) = frontmatter_name + .filter(|name| evaluated.contains(name.as_str())) + .or_else(|| { + evaluated + .contains(folder_name.as_str()) + .then_some(folder_name) + }) + else { + continue; + }; + let namespace = project_namespace(&root); + sources.push(ShadowSource::live_skill( + skill_name, + &path, + ShadowRoot { + scope: ShadowRootScope::Project, + namespace, + plugin: None, + path: artifact_path(&root), + relation: if native.as_ref() == Some(&root) { + ShadowRelation::Native + } else { + ShadowRelation::CrossHarness + }, + }, + format!( + "Set `codebase.exclude_skill_sources = true` for this eval, or move or rename '{}'.", + path.display() + ), + )); + } + } + sources.sort_by(|a, b| { + (&a.skill_name, &a.discovery_path).cmp(&(&b.skill_name, &b.discovery_path)) + }); + sources +} + +fn frontmatter_name(skill_md: &Path) -> Option { + let raw = fs::read_to_string(skill_md).ok()?; + let mut lines = raw.lines(); + (lines.next()?.trim() == "---").then_some(())?; + let mut found = None; + for line in lines { + if line.trim() == "---" { + return found; + } + if line.starts_with(' ') || line.starts_with('\t') { + continue; + } + let Some((key, value)) = line.split_once(':') else { + continue; + }; + if key.trim() == "name" { + let value = value.trim(); + let unquoted = if value.len() >= 2 + && ((value.starts_with('"') && value.ends_with('"')) + || (value.starts_with('\'') && value.ends_with('\''))) + { + value[1..value.len() - 1].trim() + } else { + value + }; + found = (!unquoted.is_empty()).then(|| unquoted.to_string()); + } + } + None +} + +fn project_namespace(skills_dir: &Path) -> ShadowNamespace { + match skills_dir + .parent() + .and_then(Path::file_name) + .and_then(|name| name.to_str()) + { + Some(".claude") => ShadowNamespace::Claude, + Some(".agents") => ShadowNamespace::Agents, + Some(".opencode") => ShadowNamespace::Opencode, + Some(".cline") => ShadowNamespace::Cline, + Some(".codex") => ShadowNamespace::Codex, + _ => ShadowNamespace::Unknown, + } +} + pub(super) fn run( ctx: &RunContext, opts: &RunOptions, @@ -36,8 +147,10 @@ pub(super) fn run( .into_iter() .collect::>(); let mut config_dir = None; - let mut shadowed_names = BTreeSet::new(); - let mut scans = Vec::with_capacity(targets.len()); + let mut operator_shadowed_names = BTreeSet::new(); + let mut operator_scans = Vec::with_capacity(targets.len()); + let mut codebase_shadowed_names = BTreeSet::new(); + let mut codebase_scans = Vec::with_capacity(targets.len()); for target in targets { let Some((condition, _)) = target.conditions.first() else { continue; @@ -57,32 +170,121 @@ pub(super) fn run( }) .unwrap_or_default(); for source in &mut sources { - shadowed_names.insert(source.skill_name.clone()); + operator_shadowed_names.insert(source.skill_name.clone()); + source.add_appearance(appearance.clone()); + } + operator_scans.push((target, appearance.clone(), sources)); + + let mut codebase_sources = staged + .codebase_shadow_sources + .get(&target.root) + .cloned() + .unwrap_or_default(); + omit_sources_displaced_by_staging(ctx, opts, r, staged, target, &mut codebase_sources); + for source in &mut codebase_sources { + codebase_shadowed_names.insert(source.skill_name.clone()); source.add_appearance(appearance.clone()); } - scans.push((target, appearance, sources)); + codebase_scans.push((target, appearance, codebase_sources)); } - if shadowed_names.is_empty() { + if operator_shadowed_names.is_empty() && codebase_shadowed_names.is_empty() { return Ok(()); } - let mut observed_sources = Vec::new(); + let operator_sources = collect_observed_sources( + ctx, + opts, + r, + staged, + adapter, + operator_scans, + &operator_shadowed_names, + ); + let codebase_sources = collect_observed_sources( + ctx, + opts, + r, + staged, + adapter, + codebase_scans, + &codebase_shadowed_names, + ); + let operator_report = PluginShadowReport::from_observed_sources( + config_dir.clone().unwrap_or_default(), + operator_sources, + &ctx.skill_name, + &expected_cells, + ); + let codebase_report = PluginShadowReport::from_observed_sources_with_class( + config_dir.clone().unwrap_or_default(), + codebase_sources, + &ctx.skill_name, + &expected_cells, + ShadowFindingClass::CodebaseSourced, + ); + let mut findings = operator_report.findings.clone(); + findings.extend(codebase_report.findings.clone()); + findings.sort_by(|a, b| { + let class_key = |class| match class { + ShadowFindingClass::OperatorEnvironment => 0, + ShadowFindingClass::CodebaseSourced => 1, + }; + (class_key(a.class), &a.skill_name).cmp(&(class_key(b.class), &b.skill_name)) + }); + let artifact = PluginShadowArtifact::new( + PluginShadowReport { + config_dir: config_dir.unwrap_or_default(), + findings, + }, + adapter.isolates_live_sources(), + ); + let verifies = adapter.surfaces_session_surface(); + write_verified(&r.iteration_dir.join("plugin-shadow.json"), &artifact) + .map_err(|e| RunError::Message(e.to_string()))?; + if artifact.isolates_live_sources { + if !operator_report.is_empty() { + eprintln!( + "{}", + format_isolated_shadow_notice(&operator_report, verifies) + ); + } + if !codebase_report.is_empty() { + eprintln!( + "{}", + format_shadow_banner_with_verification(&codebase_report, verifies) + ); + } + } else { + eprintln!( + "{}", + format_shadow_banner_with_verification(&artifact.report, verifies) + ); + } + Ok(()) +} + +type Scan<'a> = (&'a EnvTarget, ShadowAppearance, Vec); + +fn collect_observed_sources( + ctx: &RunContext, + opts: &RunOptions, + r: &Resolved, + staged: &Staged, + adapter: &dyn HarnessAdapter, + scans: Vec>, + shadowed_names: &BTreeSet, +) -> Vec { + let mut observed = Vec::new(); for (target, appearance, mut sources) in scans { let Some(skills_dir) = adapter.skills_dir(&target.root) else { adapter.resolve_shadow_sources(&target.root, &mut sources); - observed_sources.extend(sources); + observed.extend(sources); continue; }; if !opts.no_stage { let (condition, condition_skill_path) = &target.conditions[0]; - let condition_slug = if *condition == r.cond_a { - staged.cond_a_slug.as_deref() - } else if *condition == r.cond_b { - staged.cond_b_slug.as_deref() - } else { - None - }; + let condition_slug = condition_slug(r, staged, condition); if condition_skill_path.is_some() && shadowed_names.contains(&ctx.skill_name) && let Some(slug) = condition_slug @@ -113,29 +315,51 @@ pub(super) fn run( } } adapter.resolve_shadow_sources(&target.root, &mut sources); - observed_sources.extend(sources); + observed.extend(sources); } + observed +} - let report = PluginShadowReport::from_observed_sources( - config_dir.unwrap_or_default(), - observed_sources, - &ctx.skill_name, - &expected_cells, - ); - let artifact = PluginShadowArtifact::new(report, adapter.isolates_live_sources()); - let verifies = adapter.surfaces_session_surface(); - write_verified(&r.iteration_dir.join("plugin-shadow.json"), &artifact) - .map_err(|e| RunError::Message(e.to_string()))?; - if artifact.isolates_live_sources { - eprintln!( - "{}", - format_isolated_shadow_notice(&artifact.report, verifies) - ); +fn condition_slug<'a>(r: &Resolved, staged: &'a Staged, condition: &str) -> Option<&'a str> { + if condition == r.cond_a { + staged.cond_a_slug.as_deref() + } else if condition == r.cond_b { + staged.cond_b_slug.as_deref() } else { - eprintln!( - "{}", - format_shadow_banner_with_verification(&artifact.report, verifies) + None + } +} + +/// A source backed up because staging owns its exact discovery path cannot be +/// loaded during this run. Other project sources remain findings. +fn omit_sources_displaced_by_staging( + ctx: &RunContext, + opts: &RunOptions, + r: &Resolved, + staged: &Staged, + target: &EnvTarget, + sources: &mut Vec, +) { + if opts.no_stage { + return; + } + let adapter = adapter_for(ctx.harness); + let Some(skills_dir) = adapter.skills_dir(&target.root) else { + return; + }; + let mut displaced = BTreeSet::new(); + let (condition, condition_skill_path) = &target.conditions[0]; + if condition_skill_path.is_some() + && let Some(slug) = condition_slug(r, staged, condition) + { + displaced.insert(artifact_path(&skills_dir.join(slug))); + } + if ctx.stage_siblings { + displaced.extend( + ctx.sibling_skill_names + .iter() + .map(|name| artifact_path(&skills_dir.join(name))), ); } - Ok(()) + sources.retain(|source| !displaced.contains(&source.discovery_path)); } diff --git a/src/cli/run/orchestrate/stage.rs b/src/cli/run/orchestrate/stage.rs index 3af2203..b9be06c 100644 --- a/src/cli/run/orchestrate/stage.rs +++ b/src/cli/run/orchestrate/stage.rs @@ -5,6 +5,7 @@ use std::collections::HashMap; use std::fs; use std::path::{Path, PathBuf}; +use crate::adapters::adapter_for; use crate::core::RunContext; use crate::core::fs::copy_entry_materialized; use crate::sandbox::guard_profiles::{detect_profiles, expand_policy}; @@ -14,8 +15,9 @@ use super::super::RunError; use super::super::dispatch::get_skill_description; use super::super::fixtures::{FixtureClaims, copy_fixtures}; use super::super::staging::{ - StageSiblingOpts, StageSkillOpts, cleanup_staged_skills, register_staged_skill_for_cleanup, - skills_dir_for_harness, stage_sibling_skills, stage_skill_for_harness, + StageSiblingOpts, StageSkillOpts, cleanup_staged_skills, exclude_codebase_skill_sources, + register_staged_skill_for_cleanup, skills_dir_for_harness, stage_sibling_skills, + stage_skill_for_harness, }; use super::super::util::{harness_label, resolve_plan_mode_profile}; use super::envs::{EnvLayoutInput, env_targets}; @@ -98,6 +100,10 @@ pub(super) fn stage_conditions( // environment sharing a codebase is provisioned from one materialization. let mut materialized: HashMap = HashMap::new(); let mut guard_policies = HashMap::new(); + let mut codebase_shadow_sources = HashMap::new(); + let evaluated_names = std::iter::once(ctx.skill_name.as_str()) + .chain(ctx.sibling_skill_names.iter().map(String::as_str)) + .collect::>(); for target in &targets { // Disarm a prior run's guard before re-staging, so a crashed run can't leave @@ -106,6 +112,13 @@ pub(super) fn stage_conditions( teardown_guard(&target.root); let codebase = r.codebase_for(&target.eval_ids)?; + if target.root.exists() && adapter_for(ctx.harness).skills_dir(&target.root).is_some() { + // Undo only runner-owned staging/exclusion from the reused environment + // before it is reused or replaced. Never run this legacy cleanup + // against a freshly provisioned codebase: it may legitimately own + // a directory whose name uses the historical staging prefix. + cleanup_staged_skills(&target.root, ctx.harness)?; + } 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 @@ -125,18 +138,28 @@ pub(super) fn stage_conditions( fs::create_dir_all(&target.root)?; } - if !opts.no_stage { - cleanup_staged_skills(&target.root, ctx.harness)?; - if ctx.stage_siblings { - stage_sibling_skills(&StageSiblingOpts { - skill_under_test: &ctx.skill_name, - skills_source_dir: &skills, - repo_root: &target.root, - harness: ctx.harness, - })?; + if let Some(codebase) = codebase { + let inventoried = super::shadow_preflight::scan_codebase_skill_sources( + &target.root, + ctx.harness, + &evaluated_names, + ); + if codebase.declared.exclude_skill_sources() { + exclude_codebase_skill_sources(&target.root, &ctx.skill_name, ctx.harness)?; + } else if !inventoried.is_empty() { + codebase_shadow_sources.insert(target.root.clone(), inventoried); } } + if !opts.no_stage && ctx.stage_siblings { + stage_sibling_skills(&StageSiblingOpts { + skill_under_test: &ctx.skill_name, + skills_source_dir: &skills, + repo_root: &target.root, + harness: ctx.harness, + })?; + } + for (cond_name, cond_skill_path) in &target.conditions { // Refuse to clobber a pre-existing --stage-name dir in this env. if let Some(stage_name) = opts.stage_name @@ -210,6 +233,7 @@ pub(super) fn stage_conditions( bootstrap_content, plan_mode_content, guard_policies, + codebase_shadow_sources, }) } diff --git a/src/cli/run/staging/codebase.rs b/src/cli/run/staging/codebase.rs new file mode 100644 index 0000000..e31dc65 --- /dev/null +++ b/src/cli/run/staging/codebase.rs @@ -0,0 +1,68 @@ +//! Reversible removal of project skill roots from sourced task codebases. + +use super::*; + +/// One project skill root moved aside for a codebase that opted out of skill +/// sources. `path` is relative to the environment root and is accepted during +/// cleanup only when the resolved harness descriptor still declares it. +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct ExcludedRoot { + pub path: String, + pub backup_path: String, +} + +/// Move every project-local skill root discoverable by `harness` out of a +/// sourced task environment. The native root is recreated only to hold the +/// staging manifest (and, when enabled, the eval skill); root instruction files +/// and all other harness config remain in place. +pub fn exclude_codebase_skill_sources( + repo_root: &Path, + staged_under_test: &str, + harness: Harness, +) -> Result<(), RunError> { + let adapter = adapter_for(harness); + let Some(skills_dir) = adapter.skills_dir(repo_root) else { + return Ok(()); + }; + let mut manifest = load_or_create_manifest(&skills_dir, staged_under_test)?; + + for root in adapter.project_skill_dirs(repo_root) { + if !root.exists() { + continue; + } + let relative = root.strip_prefix(repo_root).map_err(|_| { + RunError::msg(format!( + "project skill root {} escapes task environment {}", + root.display(), + repo_root.display() + )) + })?; + let backup_root = make_backup_root()?; + let backup_path = backup_root.join("skill-root"); + copy_entry_materialized(&root, &backup_path)?; + manifest.excluded_roots.push(ExcludedRoot { + path: relative.to_string_lossy().into_owned(), + backup_path: backup_path.to_string_lossy().into_owned(), + }); + remove_path(&root)?; + } + + if !manifest.excluded_roots.is_empty() { + fs::create_dir_all(&skills_dir)?; + write_json(&skills_dir.join(STAGED_SIBLING_MANIFEST), &manifest)?; + } + Ok(()) +} + +pub(super) fn is_managed_backup_path(path: &Path, expected_leaf: &str) -> bool { + path.file_name().and_then(|name| name.to_str()) == Some(expected_leaf) + && path + .parent() + .and_then(Path::file_name) + .and_then(|name| name.to_str()) + .is_some_and(|name| name.starts_with("slow-powers-eval-backup-")) + && path + .parent() + .and_then(Path::parent) + .is_some_and(|parent| parent == std::env::temp_dir()) +} diff --git a/src/cli/run/staging/mod.rs b/src/cli/run/staging/mod.rs index 0613908..f518e22 100644 --- a/src/cli/run/staging/mod.rs +++ b/src/cli/run/staging/mod.rs @@ -23,6 +23,10 @@ use crate::workspace::SNAPSHOT_META; use super::RunError; use crate::core::fs::{copy_entry_materialized, write_json}; +mod codebase; +pub use codebase::exclude_codebase_skill_sources; +use codebase::{ExcludedRoot, is_managed_backup_path}; + /// Prefix for the conspicuous staged-skill slug. The prefix scan in /// [`cleanup_staged_skills`] keys on it to remove staged dirs. pub const STAGED_SKILL_PREFIX: &str = "slow-powers-eval-"; @@ -51,6 +55,8 @@ pub struct SiblingManifest { #[serde(skip_serializing_if = "Option::is_none")] pub skills_dir_preexisting: Option, pub created_entries: Vec, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub excluded_roots: Vec, } /// Options for staging a single skill. `harness` defaults to Claude Code via @@ -168,7 +174,16 @@ pub fn stage_skill_for_harness(opts: &StageSkillOpts) -> Result Result<(), RunError> { - let manifest_path = skills_dir_for_harness(repo_root, harness).join(STAGED_SIBLING_MANIFEST); - let mut manifest: SiblingManifest = if manifest_path.exists() { - serde_json::from_str(&fs::read_to_string(&manifest_path)?)? - } else { - SiblingManifest { - created_at: now_iso8601(), - staged_under_test: name.to_string(), - skills_dir_preexisting: Some(true), - created_entries: Vec::new(), - } - }; + let skills_dir = skills_dir_for_harness(repo_root, harness); + let manifest_path = skills_dir.join(STAGED_SIBLING_MANIFEST); + let mut manifest = load_or_create_manifest(&skills_dir, name)?; if manifest.created_entries.iter().any(|e| e.name == name) { return Ok(()); } @@ -244,8 +251,9 @@ pub fn register_staged_skill_for_cleanup( /// pre-existing entry, and write the manifest. pub fn stage_sibling_skills(opts: &StageSiblingOpts) -> Result { let skills_dir = skills_dir_for_harness(opts.repo_root, opts.harness); - let skills_dir_preexisting = skills_dir.exists(); + let mut manifest = load_or_create_manifest(&skills_dir, opts.skill_under_test)?; fs::create_dir_all(&skills_dir)?; + write_json(&skills_dir.join(STAGED_SIBLING_MANIFEST), &manifest)?; let mut siblings: Vec = Vec::new(); for entry in fs::read_dir(opts.skills_source_dir)? { @@ -262,33 +270,10 @@ pub fn stage_sibling_skills(opts: &StageSiblingOpts) -> Result Result Result { + let manifest_path = skills_dir.join(STAGED_SIBLING_MANIFEST); + if manifest_path.exists() { + return Ok(serde_json::from_str(&fs::read_to_string(manifest_path)?)?); + } + Ok(SiblingManifest { + created_at: now_iso8601(), + staged_under_test: staged_under_test.to_string(), + skills_dir_preexisting: Some(skills_dir.exists()), + created_entries: Vec::new(), + excluded_roots: Vec::new(), + }) +} + +/// Register one runner-owned destination and preserve the entry it displaced. +/// Re-staging the same runner entry replaces only the staged copy and retains +/// the original first backup. +fn prepare_created_entry( + skills_dir: &Path, + name: &str, + manifest: &mut SiblingManifest, +) -> Result<(), RunError> { + let target = skills_dir.join(name); + if manifest + .created_entries + .iter() + .any(|entry| entry.name == name) + { + if target.exists() { + remove_path(&target)?; + } + return Ok(()); + } + + let mut entry = CreatedEntry { + name: name.to_string(), + preexisting: target.exists(), + backup_path: None, + }; + if target.exists() { + let backup_root = make_backup_root()?; + let backup_path = backup_root.join(name); + copy_entry_materialized(&target, &backup_path)?; + entry.backup_path = Some(backup_path.display().to_string()); + } + manifest.created_entries.push(entry); + fs::create_dir_all(skills_dir)?; + write_json(&skills_dir.join(STAGED_SIBLING_MANIFEST), manifest)?; + if target.exists() { + remove_path(&target)?; + } + Ok(()) +} + /// Remove the staged skills (prefix-scanned + manifest-listed) and restore any /// pre-existing siblings the runner displaced. pub fn cleanup_staged_skills(repo_root: &Path, harness: Harness) -> Result<(), RunError> { @@ -339,6 +382,46 @@ pub fn cleanup_staged_skills(repo_root: &Path, harness: Harness) -> Result<(), R } }; + if !manifest.excluded_roots.is_empty() { + let allowed_roots = adapter_for(harness).project_skill_dirs(repo_root); + for excluded in &manifest.excluded_roots { + let target = repo_root.join(&excluded.path); + if !allowed_roots.contains(&target) { + return Err(RunError::msg(format!( + "staging manifest names undeclared project skill root {}", + target.display() + ))); + } + let backup = Path::new(&excluded.backup_path); + if !is_managed_backup_path(backup, "skill-root") { + return Err(RunError::msg(format!( + "staging manifest names unmanaged exclusion backup {}", + backup.display() + ))); + } + } + fs::remove_dir_all(&skills_dir)?; + for excluded in &manifest.excluded_roots { + let target = repo_root.join(&excluded.path); + let backup = Path::new(&excluded.backup_path); + if backup.exists() { + if target.exists() { + remove_path(&target)?; + } + copy_entry_materialized(backup, &target)?; + if let Some(parent) = backup.parent() { + fs::remove_dir_all(parent)?; + } + } + } + if !skills_dir.exists() + && let Some(harness_dir) = skills_dir.parent() + { + prune_if_empty(harness_dir)?; + } + return Ok(()); + } + // The runner created the harness skills dir this run, so it holds none of the // user's own skills — remove the whole staged tree (including any stray, // non-prefixed dirs left behind), then prune an emptied parent. diff --git a/src/cli/run/staging/tests/cleanup.rs b/src/cli/run/staging/tests/cleanup.rs index 9b26c25..269f4f9 100644 --- a/src/cli/run/staging/tests/cleanup.rs +++ b/src/cli/run/staging/tests/cleanup.rs @@ -169,3 +169,123 @@ fn leaves_preexisting_skills_dir_in_place() { assert_eq!(read(&skills_dir.join("user-owned/SKILL.md")), "USER"); assert!(!skills_dir.join("alpha").exists()); } + +#[test] +fn codebase_skill_exclusion_moves_all_discovery_roots_and_cleanup_restores_them() { + let tmp = TempDir::new().unwrap(); + write( + &tmp.path().join(".opencode/skills/native/SKILL.md"), + "NATIVE", + ); + write( + &tmp.path().join(".claude/skills/claude-compat/SKILL.md"), + "CLAUDE", + ); + write( + &tmp.path().join(".agents/skills/agents-compat/SKILL.md"), + "AGENTS", + ); + write(&tmp.path().join(".opencode/settings.json"), "{}"); + write(&tmp.path().join("CLAUDE.md"), "claude instructions"); + write(&tmp.path().join("AGENTS.md"), "agent instructions"); + + exclude_codebase_skill_sources(tmp.path(), "subject", Harness::resolve("opencode").unwrap()) + .unwrap(); + + assert!(!tmp.path().join(".opencode/skills/native").exists()); + assert!(!tmp.path().join(".claude/skills").exists()); + assert!(!tmp.path().join(".agents/skills").exists()); + assert_eq!(read(&tmp.path().join(".opencode/settings.json")), "{}"); + assert_eq!(read(&tmp.path().join("CLAUDE.md")), "claude instructions"); + assert_eq!(read(&tmp.path().join("AGENTS.md")), "agent instructions"); + + cleanup_staged_skills(tmp.path(), Harness::resolve("opencode").unwrap()).unwrap(); + + assert_eq!( + read(&tmp.path().join(".opencode/skills/native/SKILL.md")), + "NATIVE" + ); + assert_eq!( + read(&tmp.path().join(".claude/skills/claude-compat/SKILL.md")), + "CLAUDE" + ); + assert_eq!( + read(&tmp.path().join(".agents/skills/agents-compat/SKILL.md")), + "AGENTS" + ); +} + +#[test] +fn cleanup_rejects_undeclared_excluded_root_before_removing_native_skills() { + let tmp = TempDir::new().unwrap(); + let harness = Harness::resolve("opencode").unwrap(); + write( + &tmp.path().join(".opencode/skills/native/SKILL.md"), + "NATIVE", + ); + let backup = make_backup_root().unwrap().join("skill-root"); + write(&backup.join("subject/SKILL.md"), "SUBJECT"); + let manifest = SiblingManifest { + created_at: "test".into(), + staged_under_test: "subject".into(), + skills_dir_preexisting: Some(true), + created_entries: Vec::new(), + excluded_roots: vec![ExcludedRoot { + path: "undeclared/skills".into(), + backup_path: backup.display().to_string(), + }], + }; + write_json( + &tmp.path() + .join(".opencode/skills") + .join(STAGED_SIBLING_MANIFEST), + &manifest, + ) + .unwrap(); + + let error = cleanup_staged_skills(tmp.path(), harness).unwrap_err(); + + assert!(error.to_string().contains("undeclared project skill root")); + assert_eq!( + read(&tmp.path().join(".opencode/skills/native/SKILL.md")), + "NATIVE" + ); +} + +#[test] +fn cleanup_rejects_unmanaged_exclusion_backup_before_removing_native_skills() { + let tmp = TempDir::new().unwrap(); + let harness = Harness::resolve("opencode").unwrap(); + write( + &tmp.path().join(".opencode/skills/native/SKILL.md"), + "NATIVE", + ); + let backup = tmp.path().join("unmanaged/skill-root"); + write(&backup.join("subject/SKILL.md"), "SUBJECT"); + let manifest = SiblingManifest { + created_at: "test".into(), + staged_under_test: "subject".into(), + skills_dir_preexisting: Some(true), + created_entries: Vec::new(), + excluded_roots: vec![ExcludedRoot { + path: ".opencode/skills".into(), + backup_path: backup.display().to_string(), + }], + }; + write_json( + &tmp.path() + .join(".opencode/skills") + .join(STAGED_SIBLING_MANIFEST), + &manifest, + ) + .unwrap(); + + let error = cleanup_staged_skills(tmp.path(), harness).unwrap_err(); + + assert!(error.to_string().contains("unmanaged exclusion backup")); + assert_eq!( + read(&tmp.path().join(".opencode/skills/native/SKILL.md")), + "NATIVE" + ); + assert!(backup.exists()); +} diff --git a/src/cli/run/staging/tests/stage.rs b/src/cli/run/staging/tests/stage.rs index a865496..25471c1 100644 --- a/src/cli/run/staging/tests/stage.rs +++ b/src/cli/run/staging/tests/stage.rs @@ -56,6 +56,38 @@ fn overwrites_existing_staged_skill_at_same_slug() { assert_eq!(read(&staged), "second"); } +#[test] +fn generated_slug_collision_is_backed_up_and_restored() { + let tmp = TempDir::new().unwrap(); + let slug = "slow-powers-eval-1-with_skill__s"; + let existing = tmp.path().join(".claude/skills").join(slug); + write(&existing.join("SKILL.md"), "CODEBASE OWNED"); + + stage_skill_for_cc(&StageSkillOpts { + content: "STAGED", + iteration: 1, + condition: "with_skill", + skill_name: "s", + repo_root: tmp.path(), + ..Default::default() + }) + .unwrap(); + + assert_eq!(read(&existing.join("SKILL.md")), "STAGED"); + let manifest = read_manifest(&tmp.path().join(".claude/skills")); + let entry = manifest + .created_entries + .iter() + .find(|entry| entry.name == slug) + .expect("the staged subject is registered for cleanup"); + assert!(entry.preexisting); + assert!(entry.backup_path.is_some()); + + cleanup_staged_skills(tmp.path(), Harness::resolve("claude-code").unwrap()).unwrap(); + + assert_eq!(read(&existing.join("SKILL.md")), "CODEBASE OWNED"); +} + #[test] fn copies_sibling_assets_from_assets_dir() { let tmp = TempDir::new().unwrap(); diff --git a/src/core/types.rs b/src/core/types.rs index 3ed8cb5..8e2600d 100644 --- a/src/core/types.rs +++ b/src/core/types.rs @@ -210,12 +210,31 @@ pub enum CodebaseSource { /// tracked a moving branch could not be re-run against what it measured. #[serde(rename = "ref")] reference: String, + #[serde(default)] + exclude_skill_sources: bool, }, Path { path: String, + #[serde(default)] + exclude_skill_sources: bool, }, } +impl CodebaseSource { + pub fn exclude_skill_sources(&self) -> bool { + match self { + Self::Git { + exclude_skill_sources, + .. + } + | Self::Path { + exclude_skill_sources, + .. + } => *exclude_skill_sources, + } + } +} + /// Whether a source came from a repository URL or a directory on this host. #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] @@ -277,11 +296,19 @@ pub struct SkillSource { /// 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 -/// [`SourceRecord`], having exactly one. +/// [`CodebaseRecord`], having exactly one. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct CodebaseRecord { + #[serde(flatten)] + pub source: SourceRecord, + #[serde(default)] + pub exclude_skill_sources: bool, +} + #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct CodebaseUse { #[serde(flatten)] - pub codebase: SourceRecord, + pub codebase: CodebaseRecord, pub evals: Vec, } @@ -438,7 +465,7 @@ pub struct RunRecord { /// 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, + pub codebase: Option, /// The skill under test this run staged. Grading reads `run.json` and nothing /// else, so a result can only be tied to a skill revision if the record names /// one. Appended last, and omitted when absent. @@ -884,6 +911,22 @@ mod tests { assert!(out.get("run_nonce").is_none()); } + #[test] + fn codebase_use_records_effective_skill_source_exclusion() { + let value = serde_json::json!({ + "kind": "path", + "source": "../fixture", + "branch": "work", + "exclude_skill_sources": true, + "evals": ["e1"] + }); + + let record: CodebaseUse = serde_json::from_value(value).unwrap(); + let rendered = serde_json::to_value(record).unwrap(); + + assert_eq!(rendered["exclude_skill_sources"], true); + } + #[test] fn timing_source_kebab_roundtrips() { let v = serde_json::to_value(TimingSource::CompletionEvent).unwrap(); diff --git a/src/pipeline/aggregate.rs b/src/pipeline/aggregate.rs index 00730bf..42a3678 100644 --- a/src/pipeline/aggregate.rs +++ b/src/pipeline/aggregate.rs @@ -574,6 +574,9 @@ fn collect_shadow_warnings( .as_ref() .is_some_and(|verification| verification.assertion_contradicted); if artifact.isolates_live_sources && !contradicted { + warnings.extend(artifact.validity_warnings_for_class( + crate::adapters::skill_shadow::ShadowFindingClass::CodebaseSourced, + )); return; } if contradicted { diff --git a/src/pipeline/record_runs.rs b/src/pipeline/record_runs.rs index 349f49e..39fd489 100644 --- a/src/pipeline/record_runs.rs +++ b/src/pipeline/record_runs.rs @@ -31,7 +31,7 @@ use serde::Deserialize; use crate::adapters::{PermissionDenial, TranscriptSummary, adapter_for}; use crate::core::fs::write_json; use crate::core::{ - ConversationEvent, ConversationRecord, Harness, RunRecord, SkillSource, SourceRecord, + CodebaseRecord, ConversationEvent, ConversationRecord, Harness, RunRecord, SkillSource, TimingRecord, TimingSource, }; use crate::pipeline::error::PipelineError; @@ -85,7 +85,7 @@ struct DispatchTask { /// 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, + codebase: Option, skill_source: Option, } diff --git a/src/pipeline/record_runs/tests/assembly.rs b/src/pipeline/record_runs/tests/assembly.rs index 37cc180..dbd93f5 100644 --- a/src/pipeline/record_runs/tests/assembly.rs +++ b/src/pipeline/record_runs/tests/assembly.rs @@ -70,7 +70,8 @@ fn carries_the_codebase_from_dispatch_task_into_each_run_record() { "source": "https://example.com/project.git", "ref": "main", "revision": "a1b2c3d4e5f60718293a4b5c6d7e8f9012345678", - "branch": "main" + "branch": "main", + "exclude_skill_sources": true }); fs::write( iter.join("dispatch.json"), diff --git a/src/pipeline/shadow_verification.rs b/src/pipeline/shadow_verification.rs index d74af82..8033ba9 100644 --- a/src/pipeline/shadow_verification.rs +++ b/src/pipeline/shadow_verification.rs @@ -10,11 +10,11 @@ use std::collections::{BTreeMap, BTreeSet}; use std::fs; use std::path::Path; -use crate::adapters::skill_shadow::PluginShadowArtifact; use crate::adapters::skill_shadow::verification::{ DispatchEvidence, EvidenceIndex, ReportVerification, VerificationStatus, finding_status, verify_finding, }; +use crate::adapters::skill_shadow::{PluginShadowArtifact, ShadowFindingClass}; use crate::core::fs::write_json; use crate::pipeline::error::PipelineError; use crate::pipeline::io::now_iso8601; @@ -98,12 +98,17 @@ pub(crate) fn verify_iteration(iteration_dir: &Path) -> Result<(), PipelineError .collect(); let index = SurfaceIndex(&surfaces); - let (mut refuted, mut confirmed, mut unverified) = (0, 0, 0); + let (mut refuted, mut confirmed, mut unverified, mut confirmed_operator) = (0, 0, 0, 0); for finding in &mut artifact.report.findings { finding.resolved_severity = Some(verify_finding(finding, &index, &expected_by_group)); match finding_status(finding) { VerificationStatus::Refuted => refuted += 1, - VerificationStatus::Confirmed => confirmed += 1, + VerificationStatus::Confirmed => { + confirmed += 1; + if finding.class == ShadowFindingClass::OperatorEnvironment { + confirmed_operator += 1; + } + } VerificationStatus::Unverified => unverified += 1, } } @@ -118,7 +123,7 @@ pub(crate) fn verify_iteration(iteration_dir: &Path) -> Result<(), PipelineError unverified_findings: unverified, // A declared assertion that evidence contradicts: the suppressed // findings were real, which is worth saying louder than the assertion. - assertion_contradicted: artifact.isolates_live_sources && confirmed > 0, + assertion_contradicted: artifact.isolates_live_sources && confirmed_operator > 0, }); write_verified(&shadow_path, &artifact) @@ -143,9 +148,9 @@ pub(crate) fn write_verified( mod tests { use super::*; use crate::adapters::skill_shadow::{ - PluginShadowReport, ShadowAppearance, ShadowFinding, ShadowNamespace, ShadowRelation, - ShadowResolution, ShadowResolvedSeverity, ShadowRoot, ShadowRootScope, ShadowSeverity, - ShadowSkillRole, ShadowSource, ShadowSourceKind, ShadowSourceOrigin, + PluginShadowReport, ShadowAppearance, ShadowFinding, ShadowFindingClass, ShadowNamespace, + ShadowRelation, ShadowResolution, ShadowResolvedSeverity, ShadowRoot, ShadowRootScope, + ShadowSeverity, ShadowSkillRole, ShadowSource, ShadowSourceKind, ShadowSourceOrigin, }; use crate::adapters::{LoadedPlugin, SessionSurface}; use crate::pipeline::session_surface::RoundSurface; @@ -184,6 +189,7 @@ mod tests { PluginShadowReport { config_dir: "/home/u/.claude".into(), findings: vec![ShadowFinding { + class: ShadowFindingClass::OperatorEnvironment, skill_name: "mr-review".into(), role: ShadowSkillRole::Subject, severity: ShadowSeverity::ComparisonInvalid, @@ -308,6 +314,33 @@ mod tests { ); } + #[test] + fn a_loaded_codebase_source_does_not_contradict_operator_isolation() { + let dir = TempDir::new().unwrap(); + let mut artifact = shadow_artifact(true); + artifact.report.findings[0].class = ShadowFindingClass::CodebaseSourced; + write_both( + dir.path(), + &artifact, + &surface_report(vec![ + task("with_skill", &["slow-powers@slowdini"], true), + task("without_skill", &[], true), + ]), + ); + + verify_iteration(dir.path()).unwrap(); + + let artifact = read_back(dir.path()); + assert_eq!(artifact.verification.unwrap().confirmed_findings, 1); + assert!( + !read_back(dir.path()) + .verification + .unwrap() + .assertion_contradicted, + "operator isolation does not make claims about the sourced codebase" + ); + } + #[test] fn a_missing_surface_report_leaves_the_artifact_untouched() { let dir = TempDir::new().unwrap(); diff --git a/src/validation/evals.rs b/src/validation/evals.rs index b24cce6..6f3338d 100644 --- a/src/validation/evals.rs +++ b/src/validation/evals.rs @@ -819,6 +819,7 @@ mod tests { Some(CodebaseSource::Git { url: "https://example.com/project.git".to_string(), reference: "main".to_string(), + exclude_skill_sources: false, }) ); } @@ -835,6 +836,7 @@ mod tests { parsed.evals[0].codebase, Some(CodebaseSource::Path { path: "../fixtures/legacy-service".to_string(), + exclude_skill_sources: false, }) ); } @@ -850,10 +852,25 @@ mod tests { parsed.codebase, Some(CodebaseSource::Path { path: "/srv/projects/legacy-service".to_string(), + exclude_skill_sources: false, }) ); } + #[test] + fn accepts_codebase_skill_source_exclusion() { + let mut config = base(); + config["codebase"] = json!({ + "path": "/srv/projects/legacy-service", + "exclude_skill_sources": true + }); + + let parsed = validate_evals_config(&config, "evals.json").unwrap(); + let declared = serde_json::to_value(parsed.codebase.unwrap()).unwrap(); + + assert_eq!(declared["exclude_skill_sources"], true); + } + /// `minLength: 1` admits `" "`, so the schema cannot carry this on its own. #[test] fn rejects_whitespace_only_codebase_values() { diff --git a/src/validation/schema.rs b/src/validation/schema.rs index 2a4d275..ed3ae1e 100644 --- a/src/validation/schema.rs +++ b/src/validation/schema.rs @@ -298,11 +298,12 @@ mod tests { } #[test] - fn validates_v2_plugin_shadow_artifacts() { + fn validates_v3_plugin_shadow_artifacts() { let artifact = json!({ - "schema_version": 2, + "schema_version": 3, "config_dir": "/home/u/.config/opencode", "findings": [{ + "class": "operator-environment", "skill_name": "mr-review", "role": "subject", "severity": "comparison-invalid", diff --git a/src/workspace/promote.rs b/src/workspace/promote.rs index c65b5f5..9c23320 100644 --- a/src/workspace/promote.rs +++ b/src/workspace/promote.rs @@ -313,21 +313,25 @@ fn codebase_rows(conditions: Option<&ConditionsRecord>) -> String { } else { "Codebase".to_string() }; - let mut cell = used.codebase.source.clone(); - if let Some(reference) = &used.codebase.reference { + let source = &used.codebase.source; + let mut cell = source.source.clone(); + if let Some(reference) = &source.reference { cell.push('@'); cell.push_str(reference); } - if let Some(revision) = &used.codebase.revision { + if let Some(revision) = &source.revision { let short: String = revision.chars().take(7).collect(); cell.push_str(&format!(" ({short})")); } - if used.codebase.host_local { + if source.host_local { cell.push_str(" — host-local path, not reproducible from this config alone"); - if let Some(origin) = &used.codebase.origin_url { + if let Some(origin) = &source.origin_url { cell.push_str(&format!("; origin {origin}")); } } + if used.codebase.exclude_skill_sources { + cell.push_str("; project skill sources excluded"); + } format!("| {label} | {cell} |") }) .collect::>() diff --git a/src/workspace/promote/tests.rs b/src/workspace/promote/tests.rs index 53d2ae5..291cda3 100644 --- a/src/workspace/promote/tests.rs +++ b/src/workspace/promote/tests.rs @@ -422,6 +422,37 @@ fn provenance_names_the_codebase_and_the_commit_it_resolved_to() { assert!(provenance.contains("a1b2c3d"), "{provenance}"); } +#[test] +fn provenance_names_when_codebase_skill_sources_were_excluded() { + let f = fixture(1); + let mut conditions: Value = serde_json::from_str(CONDITIONS_WITH_PROVENANCE).unwrap(); + conditions["codebases"] = serde_json::json!([{ + "kind": "git", + "source": "https://example.com/project.git", + "ref": "v1.4.0", + "revision": "a1b2c3d4e5f60718293a4b5c6d7e8f9012345678", + "branch": "v1.4.0", + "exclude_skill_sources": 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("project skill sources excluded"), + "{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] diff --git a/tests/cli/aggregate/shadow.rs b/tests/cli/aggregate/shadow.rs index e1d066b..db6465c 100644 --- a/tests/cli/aggregate/shadow.rs +++ b/tests/cli/aggregate/shadow.rs @@ -253,6 +253,64 @@ fn aggregate_suppresses_declared_isolated_shadows_for_every_harness() { } } +#[test] +fn aggregate_keeps_codebase_shadow_warnings_when_operator_sources_are_isolated() { + 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); + 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}), + ); + } + fs::write( + iteration_dir.join("plugin-shadow.json"), + serde_json::to_string(&json!({ + "schema_version": 3, + "config_dir": "/home/u/.claude", + "isolates_live_sources": true, + "findings": [{ + "class": "codebase-sourced", + "skill_name": "mr-review", + "role": "subject", + "severity": "comparison-invalid", + "sources": [{ + "kind": "skill", + "origin": "live", + "skill_name": "mr-review", + "runtime_id": "mr-review", + "discovery_path": "/repo/.claude/skills/mr-review", + "root": { + "scope": "project", + "namespace": "claude", + "path": "/repo/.claude/skills", + "relation": "native" + }, + "remediation": "Set `codebase.exclude_skill_sources = true` for this eval." + }] + }] + })) + .unwrap(), + ) + .unwrap(); + + agg_cmd(&cwd, &skill_dir).assert().success(); + + let warnings = read_benchmark(&iteration_dir)["validity_warnings"] + .as_array() + .unwrap() + .clone(); + assert!(warnings.iter().any(|warning| { + warning.as_str().is_some_and(|text| { + text.contains("mr-review") && text.contains("codebase.exclude_skill_sources") + }) + })); +} + /// `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`. @@ -320,6 +378,7 @@ fn aggregate_echoes_the_resolved_codebases_into_the_benchmark() { "ref": "v1.4.0", "revision": "a1b2c3d4e5f60718293a4b5c6d7e8f9012345678", "branch": "v1.4.0", + "exclude_skill_sources": true, "evals": ["e1"] }]), ); @@ -345,4 +404,5 @@ fn aggregate_echoes_the_resolved_codebases_into_the_benchmark() { "a1b2c3d4e5f60718293a4b5c6d7e8f9012345678" ); assert_eq!(b["codebases"][0]["evals"][0], "e1"); + assert_eq!(b["codebases"][0]["exclude_skill_sources"], true); } diff --git a/tests/cli/basics.rs b/tests/cli/basics.rs index 49637ef..22de034 100644 --- a/tests/cli/basics.rs +++ b/tests/cli/basics.rs @@ -306,6 +306,9 @@ fn aggregate_help_documents_declared_shadow_isolation() { .success() .stdout(contains("plugin-shadow.json")) .stdout(contains("isolates_live_sources")) + .stdout(contains("schema-v3")) + .stdout(contains("codebase-sourced")) + .stdout(contains("exclude_skill_sources")) .stdout(contains("validity_warnings")) .stdout(contains("eval-magic docs isolation")); } diff --git a/tests/cli/docs.rs b/tests/cli/docs.rs index 9474d5c..2616ce1 100644 --- a/tests/cli/docs.rs +++ b/tests/cli/docs.rs @@ -140,6 +140,7 @@ fn docs_byoh_keeps_the_authoring_workflow() { .stdout(contains("harness init")) .stdout(contains("harness lint")) .stdout(contains("--probe")) + .stdout(contains("additional_project_skill_dirs")) .stdout(contains("Upstreaming your descriptor")); } @@ -156,6 +157,9 @@ fn docs_isolation_keeps_remedies_and_verification() { .stdout(contains("OPENCODE_DISABLE_EXTERNAL_SKILLS")) .stdout(contains("resumed")) .stdout(contains("isolates_live_sources")) + .stdout(contains("operator-environment")) + .stdout(contains("codebase-sourced")) + .stdout(contains("codebase.exclude_skill_sources")) .stdout(contains("claude plugin list")) .stdout(contains("`comparison-invalid`")) .stdout(contains("\"subtype\":\"init\"")); @@ -186,7 +190,11 @@ fn docs_codebase_keeps_the_declaration_rules_caveat_and_provisioning_contract() .stdout(contains("independent working tree")) .stdout(contains("diff-scope.json")) .stdout(contains("diff.patch")) - .stdout(contains(".gitignore")); + .stdout(contains(".gitignore")) + .stdout(contains("exclude_skill_sources")) + .stdout(contains("codebase-sourced")) + .stdout(contains("CLAUDE.md")) + .stdout(contains(".opencode/skills")); } #[test] diff --git a/tests/run/claude_cli.rs b/tests/run/claude_cli.rs index 6a69a2e..6ed11f9 100644 --- a/tests/run/claude_cli.rs +++ b/tests/run/claude_cli.rs @@ -250,7 +250,7 @@ fn cli_plugin_shadow_preflight_reads_per_env_project_settings() { "preflight detected the project-enabled plugin shadow by scanning the staged env" ); let artifact = read_json(&iteration_dir(&cwd).join("plugin-shadow.json")); - assert_eq!(artifact["schema_version"], 2); + assert_eq!(artifact["schema_version"], 3); assert!( artifact.get("isolates_live_sources").is_none(), "false isolation assertions stay omitted" @@ -291,7 +291,7 @@ fn declared_shadow_isolation_records_findings_as_informational_provenance() { assert!(!stderr.contains("Plugin-shadow warning"), "{stderr}"); let artifact = read_json(&iteration_dir(&cwd).join("plugin-shadow.json")); - assert_eq!(artifact["schema_version"], 2); + assert_eq!(artifact["schema_version"], 3); assert_eq!(artifact["isolates_live_sources"], true); assert_eq!(artifact["findings"][0]["skill_name"], "mr-review"); assert_eq!(artifact["findings"][0]["severity"], "comparison-invalid"); diff --git a/tests/run/cline.rs b/tests/run/cline.rs index 736b012..9783a3f 100644 --- a/tests/run/cline.rs +++ b/tests/run/cline.rs @@ -209,7 +209,7 @@ fn cline_warns_when_live_global_skill_shadows_staged_skill() { .stderr(contains("cross-harness")); let report = read_json(&iteration_dir(&cwd).join("plugin-shadow.json")); - assert_eq!(report["schema_version"], 2); + assert_eq!(report["schema_version"], 3); assert_eq!(report["findings"][0]["skill_name"], "mr-review"); let sources = report["findings"][0]["sources"].as_array().unwrap(); let mut namespaces: Vec<&str> = sources diff --git a/tests/run/codebase.rs b/tests/run/codebase.rs index 90ed618..e6f939e 100644 --- a/tests/run/codebase.rs +++ b/tests/run/codebase.rs @@ -4,77 +4,11 @@ //! because the property under test spans resolution, provisioning, staging, and //! the fixture overlay — it is only true of a whole prepared workspace. +use crate::codebase_support::{ + an_object_file, codebase_repo, commit, evals_with_codebase, git, link_count, +}; 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() { @@ -320,64 +254,6 @@ fn a_path_codebase_is_recorded_as_host_local_with_its_origin_for_citation() { assert_eq!(recorded["origin_url"], origin_url); } -/// The ticket's last acceptance criterion: an eval declaring no codebase keeps -/// the environment it has always had. -#[test] -fn a_fixture_only_eval_still_gets_the_repository_it_always_had() { - let tmp = tempfile::TempDir::new().unwrap(); - let (skill_dir, cwd) = setup(tmp.path(), DEFAULT_EVALS); - - skill_eval() - .current_dir(&cwd) - .args(["run", "--skill-dir"]) - .arg(&skill_dir) - .args(["--skill", "mr-review", "--mode", "new-skill", "--dry-run"]) - .assert() - .success(); - - let env = cli_env_dir(&cwd, "g1", "with_skill"); - assert_eq!(git(&env, &["symbolic-ref", "--short", "HEAD"]), "work"); - assert_eq!(git(&env, &["rev-list", "--count", "HEAD"]), "1"); - assert_eq!(git(&env, &["remote"]), ""); - assert_eq!(git(&env, &["status", "--porcelain"]), ""); - assert_eq!( - git(&env, &["rev-parse", "refs/eval-magic/baseline"]), - git(&env, &["rev-parse", "HEAD"]) - ); -} - -/// The number of hard links to `file` — the mechanism `git clone --local` uses -/// to share the cache's object store with an environment instead of copying -/// it. Straight from filesystem metadata. -fn link_count(file: &Path) -> u32 { - use std::os::unix::fs::MetadataExt; - fs::metadata(file).unwrap().nlink() as u32 -} - -/// A file from `repo`'s object store — a loose object or a pack — that a local -/// clone shares with its source by hard link. `objects/info` is skipped: it -/// holds per-repository metadata (an exclude file), not objects, and is never -/// shared. -fn an_object_file(repo: &Path) -> PathBuf { - fn walk(dir: &Path) -> Option { - for entry in fs::read_dir(dir).unwrap() { - let path = entry.unwrap().path(); - if path.is_dir() { - if path.file_name().is_some_and(|name| name == "info") { - continue; - } - if let Some(found) = walk(&path) { - return Some(found); - } - } else { - return Some(path); - } - } - None - } - walk(&repo.join(".git/objects")).expect("a materialized repository has objects") -} - /// One cached materialization provisions every environment of a /// multi-run campaign, and the provisioning is a local clone — each /// environment's object store is hard-linked to the cache's, not copied. diff --git a/tests/run/codebase_compat.rs b/tests/run/codebase_compat.rs new file mode 100644 index 0000000..c1c8d32 --- /dev/null +++ b/tests/run/codebase_compat.rs @@ -0,0 +1,28 @@ +//! Compatibility behavior for evals that do not declare a sourced codebase. + +use crate::codebase_support::git; +use crate::helpers::*; + +#[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/codebase_harness_config.rs b/tests/run/codebase_harness_config.rs new file mode 100644 index 0000000..5ab45f3 --- /dev/null +++ b/tests/run/codebase_harness_config.rs @@ -0,0 +1,415 @@ +//! Cross-harness project-skill preservation, exclusion, and collision behavior. + +use crate::codebase_support::{ + add_project_skill_roots, codebase_repo, commit, evals_with_codebase, +}; +use crate::helpers::*; +use std::fs; +use std::path::{Path, PathBuf}; + +#[test] +fn sourced_project_skills_and_instructions_are_preserved_by_default_for_every_harness() { + for (harness, roots) in [ + ("claude-code", vec![".claude/skills"]), + ("cline", vec![".cline/skills"]), + ("codex", vec![".agents/skills"]), + ( + "opencode", + vec![".opencode/skills", ".claude/skills", ".agents/skills"], + ), + ] { + let tmp = tempfile::TempDir::new().unwrap(); + let origin = codebase_repo(tmp.path(), "origin", "main"); + add_project_skill_roots(&origin, &roots); + 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", + "--harness", + harness, + "--no-guard", + "--dry-run", + ]) + .assert() + .success(); + + for condition in ["with_skill", "without_skill"] { + let env = cli_env_dir(&cwd, "g1", condition); + for root in &roots { + assert!( + env.join(root).join("mr-review/SKILL.md").exists(), + "{harness}/{condition}: {root} subject source was removed" + ); + assert!( + env.join(root) + .join("slow-powers-eval-codebase-owned/SKILL.md") + .exists(), + "{harness}/{condition}: a codebase-owned prefixed skill was removed" + ); + } + assert_eq!( + fs::read_to_string(env.join("CLAUDE.md")).unwrap(), + "claude instructions\n" + ); + assert_eq!( + fs::read_to_string(env.join("AGENTS.md")).unwrap(), + "agent instructions\n" + ); + } + + let shadow = read_json(&iteration_dir(&cwd).join("plugin-shadow.json")); + let codebase_findings: Vec<_> = shadow["findings"] + .as_array() + .unwrap() + .iter() + .filter(|finding| finding["class"] == "codebase-sourced") + .collect(); + assert_eq!( + codebase_findings.len(), + 1, + "{harness}: expected one grouped codebase finding: {shadow}" + ); + assert_eq!(codebase_findings[0]["skill_name"], "mr-review"); + let live_sources: Vec<_> = codebase_findings[0]["sources"] + .as_array() + .unwrap() + .iter() + .filter(|source| source["origin"] == "live") + .collect(); + assert_eq!( + live_sources.len(), + roots.len() * 2, + "{harness}: every project root should appear in both task environments" + ); + assert!( + live_sources + .iter() + .all(|source| source["appearances"].as_array().unwrap().len() == 1), + "{harness}: each concrete environment source should name its own cell" + ); + } +} + +#[test] +fn opencode_excludes_all_project_skill_sources_under_no_stage_but_keeps_config() { + let tmp = tempfile::TempDir::new().unwrap(); + let origin = codebase_repo(tmp.path(), "origin", "main"); + let roots = [".opencode/skills", ".claude/skills", ".agents/skills"]; + add_project_skill_roots(&origin, &roots); + let source = format!( + r#"{{ "url": "{}", "ref": "main", "exclude_skill_sources": true }}"#, + 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", + "--harness", + "opencode", + "--no-stage", + "--no-guard", + "--dry-run", + ]) + .assert() + .success(); + + for condition in ["with_skill", "without_skill"] { + let env = cli_env_dir(&cwd, "g1", condition); + for root in roots { + assert!( + !env.join(root).join("mr-review").exists(), + "{condition}: {root} remained discoverable" + ); + assert!( + !env.join(root) + .join("slow-powers-eval-codebase-owned") + .exists(), + "{condition}: {root} retained another codebase skill" + ); + } + assert_eq!( + fs::read_to_string(env.join("CLAUDE.md")).unwrap(), + "claude instructions\n" + ); + assert_eq!( + fs::read_to_string(env.join("AGENTS.md")).unwrap(), + "agent instructions\n" + ); + assert_eq!( + fs::read_to_string(env.join(".opencode/settings.json")).unwrap(), + "{}\n" + ); + } + + let conditions = read_json(&iteration_dir(&cwd).join("conditions.json")); + assert_eq!(conditions["codebases"][0]["exclude_skill_sources"], true); + let dispatch = read_json(&iteration_dir(&cwd).join("dispatch.json")); + assert!( + dispatch["tasks"] + .as_array() + .unwrap() + .iter() + .all(|task| task["codebase"]["exclude_skill_sources"] == true) + ); + let backup_roots: Vec = ["with_skill", "without_skill"] + .into_iter() + .flat_map(|condition| { + let manifest = read_json( + &cli_env_dir(&cwd, "g1", condition) + .join(".opencode/skills") + .join(STAGED_MANIFEST), + ); + manifest["excluded_roots"] + .as_array() + .unwrap() + .iter() + .map(|root| { + Path::new(root["backup_path"].as_str().unwrap()) + .parent() + .unwrap() + .to_path_buf() + }) + .collect::>() + }) + .collect(); + let shadow_path = iteration_dir(&cwd).join("plugin-shadow.json"); + if shadow_path.exists() { + let shadow = read_json(&shadow_path); + assert!( + shadow["findings"] + .as_array() + .unwrap() + .iter() + .all(|finding| finding["class"] != "codebase-sourced"), + "excluded project roots must not produce codebase findings: {shadow}" + ); + } + + skill_eval() + .current_dir(&cwd) + .args(["teardown", "--skill-dir"]) + .arg(&skill_dir) + .args(["--skill", "mr-review", "--harness", "opencode"]) + .assert() + .success(); + assert!( + backup_roots.iter().all(|root| !root.exists()), + "teardown left exclusion backups behind: {backup_roots:?}" + ); +} + +#[test] +fn generated_slug_collision_is_displaced_only_in_the_staged_arm() { + let tmp = tempfile::TempDir::new().unwrap(); + let origin = codebase_repo(tmp.path(), "origin", "main"); + let slug = "slow-powers-eval-1-with_skill__mr-review"; + fs::create_dir_all(origin.join(".claude/skills").join(slug)).unwrap(); + fs::write( + origin.join(".claude/skills").join(slug).join("SKILL.md"), + "---\nname: mr-review\ndescription: codebase collision\n---\n\nCODEBASE\n", + ) + .unwrap(); + commit(&origin, "add exact staged-slug collision"); + 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", + "--no-guard", + "--dry-run", + ]) + .assert() + .success(); + + let with_root = cli_env_dir(&cwd, "g1", "with_skill"); + let without_root = cli_env_dir(&cwd, "g1", "without_skill"); + assert!( + fs::read_to_string(with_root.join(".claude/skills").join(slug).join("SKILL.md")) + .unwrap() + .contains("body"), + "the staged arm should contain the evaluated skill" + ); + assert!( + fs::read_to_string( + without_root + .join(".claude/skills") + .join(slug) + .join("SKILL.md") + ) + .unwrap() + .contains("CODEBASE"), + "the control arm should retain the sourced copy" + ); + let manifest = read_json(&with_root.join(".claude/skills").join(STAGED_MANIFEST)); + let staged_entry = manifest["created_entries"] + .as_array() + .unwrap() + .iter() + .find(|entry| entry["name"] == slug) + .unwrap(); + assert_eq!(staged_entry["preexisting"], true); + + let shadow = read_json(&iteration_dir(&cwd).join("plugin-shadow.json")); + let finding = shadow["findings"] + .as_array() + .unwrap() + .iter() + .find(|finding| finding["class"] == "codebase-sourced") + .unwrap(); + let live_sources: Vec<_> = finding["sources"] + .as_array() + .unwrap() + .iter() + .filter(|source| source["origin"] == "live") + .collect(); + assert_eq!(live_sources.len(), 1, "{finding}"); + assert_eq!( + live_sources[0]["appearances"][0]["condition"], + "without_skill" + ); +} + +#[test] +fn revision_mode_excludes_codebase_skill_sources_from_both_arms() { + let tmp = tempfile::TempDir::new().unwrap(); + let origin = codebase_repo(tmp.path(), "origin", "main"); + add_project_skill_roots(&origin, &[".claude/skills"]); + let source = format!( + r#"{{ "url": "{}", "ref": "main", "exclude_skill_sources": true }}"#, + 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(["snapshot", "--skill-dir"]) + .arg(&skill_dir) + .args(["--skill", "mr-review", "--label", "baseline"]) + .assert() + .success(); + + skill_eval() + .current_dir(&cwd) + .args(["run", "--skill-dir"]) + .arg(&skill_dir) + .args([ + "--skill", + "mr-review", + "--mode", + "revision", + "--no-guard", + "--dry-run", + ]) + .assert() + .success(); + + for condition in ["old_skill", "new_skill"] { + let env = iteration_dir(&cwd).join(format!("env-g1-{condition}")); + assert!( + !env.join(".claude/skills/mr-review").exists(), + "{condition}: the codebase subject source remained discoverable" + ); + let staged = staged_entries(&env.join(".claude/skills")); + assert_eq!(staged.len(), 1, "{condition}: {staged:?}"); + assert!(staged[0].contains(condition), "{condition}: {staged:?}"); + assert_eq!( + fs::read_to_string(env.join("CLAUDE.md")).unwrap(), + "claude instructions\n" + ); + assert_eq!( + fs::read_to_string(env.join("AGENTS.md")).unwrap(), + "agent instructions\n" + ); + } + + let shadow_path = iteration_dir(&cwd).join("plugin-shadow.json"); + if shadow_path.exists() { + let shadow = read_json(&shadow_path); + assert!( + shadow["findings"] + .as_array() + .unwrap() + .iter() + .all(|finding| finding["class"] != "codebase-sourced"), + "excluded revision arms must not report codebase sources: {shadow}" + ); + } +} + +#[test] +fn exclusion_is_a_noop_for_a_byoh_harness_without_project_skill_roots() { + let tmp = tempfile::TempDir::new().unwrap(); + let origin = codebase_repo(tmp.path(), "origin", "main"); + let source = format!( + r#"{{ "url": "{}", "ref": "main", "exclude_skill_sources": true }}"#, + 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(); + let harness_dir = cwd.join(".eval-magic/harnesses"); + fs::create_dir_all(&harness_dir).unwrap(); + fs::write( + harness_dir.join("cool.toml"), + "label = \"cool-custom-harness\"\n", + ) + .unwrap(); + + skill_eval() + .current_dir(&cwd) + .args(["run", "--skill-dir"]) + .arg(&skill_dir) + .args([ + "--skill", + "mr-review", + "--mode", + "new-skill", + "--harness", + "cool-custom-harness", + "--no-guard", + "--dry-run", + ]) + .assert() + .success(); + + for condition in ["with_skill", "without_skill"] { + assert!( + cli_env_dir(&cwd, "g1", condition) + .join("src/lib.rs") + .exists() + ); + } + let dispatch = read_json(&iteration_dir(&cwd).join("dispatch.json")); + assert!( + dispatch["tasks"] + .as_array() + .unwrap() + .iter() + .all(|task| task["codebase"]["exclude_skill_sources"] == true) + ); +} diff --git a/tests/run/codebase_support.rs b/tests/run/codebase_support.rs new file mode 100644 index 0000000..5cc4cd2 --- /dev/null +++ b/tests/run/codebase_support.rs @@ -0,0 +1,123 @@ +use std::fs; +use std::path::{Path, PathBuf}; +use std::process::Command; + +pub 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. +pub 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 +} + +pub 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`. +pub 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"] + }} + ] + }}"# + ) +} + +pub fn add_project_skill_roots(repo: &Path, roots: &[&str]) { + for root in roots { + fs::create_dir_all(repo.join(root).join("mr-review")).unwrap(); + fs::write( + repo.join(root).join("mr-review/SKILL.md"), + "---\nname: mr-review\ndescription: codebase copy\n---\n\nCODEBASE\n", + ) + .unwrap(); + fs::create_dir_all(repo.join(root).join("slow-powers-eval-codebase-owned")).unwrap(); + fs::write( + repo.join(root) + .join("slow-powers-eval-codebase-owned/SKILL.md"), + "CODEBASE PREFIXED SKILL\n", + ) + .unwrap(); + } + fs::write(repo.join("CLAUDE.md"), "claude instructions\n").unwrap(); + fs::write(repo.join("AGENTS.md"), "agent instructions\n").unwrap(); + fs::create_dir_all(repo.join(".opencode")).unwrap(); + fs::write(repo.join(".opencode/settings.json"), "{}\n").unwrap(); + commit(repo, "add project skill sources and instructions"); +} + +/// The number of hard links to `file` — the mechanism `git clone --local` uses +/// to share the cache's object store with an environment instead of copying it. +pub fn link_count(file: &Path) -> u32 { + use std::os::unix::fs::MetadataExt; + fs::metadata(file).unwrap().nlink() as u32 +} + +/// A file from `repo`'s object store — a loose object or a pack — that a local +/// clone shares with its source by hard link. `objects/info` is skipped because +/// it holds per-repository metadata rather than objects. +pub fn an_object_file(repo: &Path) -> PathBuf { + fn walk(dir: &Path) -> Option { + for entry in fs::read_dir(dir).unwrap() { + let path = entry.unwrap().path(); + if path.is_dir() { + if path.file_name().is_some_and(|name| name == "info") { + continue; + } + if let Some(found) = walk(&path) { + return Some(found); + } + } else { + return Some(path); + } + } + None + } + walk(&repo.join(".git/objects")).expect("a materialized repository has objects") +} diff --git a/tests/run/codex.rs b/tests/run/codex.rs index db0f5d1..10d0665 100644 --- a/tests/run/codex.rs +++ b/tests/run/codex.rs @@ -457,7 +457,7 @@ fn codex_warns_when_user_skill_shadows_staged_skill() { .stderr(contains("Move or rename")); let report = read_json(&iteration_dir(&cwd).join("plugin-shadow.json")); - assert_eq!(report["schema_version"], 2); + assert_eq!(report["schema_version"], 3); assert_eq!(report["findings"][0]["skill_name"], "mr-review"); assert_eq!(report["findings"][0]["role"], "subject"); let sources = report["findings"][0]["sources"].as_array().unwrap(); diff --git a/tests/run/main.rs b/tests/run/main.rs index b960739..000db3f 100644 --- a/tests/run/main.rs +++ b/tests/run/main.rs @@ -15,6 +15,9 @@ mod claude_cli; mod cline; mod cline_permission_denials; mod codebase; +mod codebase_compat; +mod codebase_harness_config; +mod codebase_support; mod codex; mod codex_guard; mod codex_permission_denials; diff --git a/tests/run/opencode.rs b/tests/run/opencode.rs index c57eb1c..327169f 100644 --- a/tests/run/opencode.rs +++ b/tests/run/opencode.rs @@ -367,7 +367,7 @@ fn opencode_warns_when_live_skill_shadows_staged_skill() { .stderr(contains("OPENCODE_DISABLE_CLAUDE_CODE_SKILLS")); let report = read_json(&iteration_dir(&cwd).join("plugin-shadow.json")); - assert_eq!(report["schema_version"], 2); + assert_eq!(report["schema_version"], 3); assert_eq!(report["findings"][0]["skill_name"], "mr-review"); let sources = report["findings"][0]["sources"].as_array().unwrap(); let live = sources