diff --git a/eng/skill-validator/src/Commands/ValidateCommand.cs b/eng/skill-validator/src/Commands/ValidateCommand.cs index 05150948cd..59519e3f32 100644 --- a/eng/skill-validator/src/Commands/ValidateCommand.cs +++ b/eng/skill-validator/src/Commands/ValidateCommand.cs @@ -31,6 +31,9 @@ public static RootCommand Create() var reporterOpt = new Option("--reporter") { Description = "Reporter (console, json, junit, markdown). Can be repeated.", AllowMultipleArgumentsPerToken = true }; var noOverfittingCheckOpt = new Option("--no-overfitting-check") { Description = "Disable LLM-based overfitting analysis (on by default)" }; var overfittingFixOpt = new Option("--overfitting-fix") { Description = "Generate a fixed eval.yaml with improved rubric items/assertions" }; + var noiseSkillsDirOpt = new Option("--noise-skills-dir") { Description = "Directory containing skills to load as noise. Enables the noise test: re-runs scenarios with all noise skills loaded and measures degradation." }; + var noiseMaxDegradationOpt = new Option("--noise-max-degradation") { Description = "Maximum acceptable average quality degradation (0-1) in noise test (only positive degradations count)", DefaultValueFactory = _ => 0.2 }; + var noiseMaxScenarioDegradationOpt = new Option("--noise-max-scenario-degradation") { Description = "Maximum acceptable quality degradation (0-1) for any single noise-test scenario", DefaultValueFactory = _ => 0.4 }; var command = new RootCommand("Validate that agent skills meaningfully improve agent performance") { @@ -54,6 +57,9 @@ public static RootCommand Create() reporterOpt, noOverfittingCheckOpt, overfittingFixOpt, + noiseSkillsDirOpt, + noiseMaxDegradationOpt, + noiseMaxScenarioDegradationOpt, }; command.SetAction(async (parseResult, _) => @@ -99,6 +105,9 @@ public static RootCommand Create() TestsDir = parseResult.GetValue(testsDirOpt), OverfittingCheck = !parseResult.GetValue(noOverfittingCheckOpt), OverfittingFix = parseResult.GetValue(overfittingFixOpt), + NoiseSkillsDir = parseResult.GetValue(noiseSkillsDirOpt), + NoiseDegradationLimit = parseResult.GetValue(noiseMaxDegradationOpt), + NoiseMaxScenarioDegradation = parseResult.GetValue(noiseMaxScenarioDegradationOpt), }; return await Run(config); @@ -164,12 +173,21 @@ public static async Task Run(ValidatorConfig config) if (allSkills.Count == 0) { - Console.Error.WriteLine("No skills found in the specified paths."); + var searched = string.Join(", ", config.SkillPaths.Select(p => $"\"{Path.GetFullPath(p)}\"")); + Console.Error.WriteLine($"No skills found in the specified paths: {searched}"); return 1; } Console.WriteLine($"Found {allSkills.Count} skill(s)\n"); + // Discover noise skills when --noise-skills-dir is provided + var noiseSkills = new List(); + if (config.NoiseSkillsDir is not null) + { + noiseSkills.AddRange(await SkillDiscovery.DiscoverSkillsRecursive(config.NoiseSkillsDir, config.TestsDir)); + Console.WriteLine($"Noise test enabled: discovered {noiseSkills.Count} noise skill(s) from {config.NoiseSkillsDir}"); + } + // Check per-plugin aggregate description size var aggregateFailures = CheckAggregateDescriptionLimits(allSkills); if (aggregateFailures.Count > 0) @@ -258,7 +276,7 @@ public static async Task Run(ValidatorConfig config) // Evaluate skills spinner.Start($"Evaluating {allSkills.Count} skill(s)..."); var skillTasks = allSkills.Select(skill => - skillLimit.RunAsync(() => EvaluateSkill(skill, config, usePairwise, spinner))); + skillLimit.RunAsync(() => EvaluateSkill(skill, config, usePairwise, spinner, noiseSkills))); var settled = await Task.WhenAll(skillTasks.Select(async t => { try { return (Result: await t, Error: (Exception?)null); } @@ -362,7 +380,8 @@ internal static List CheckAggregateDescriptionLimits(IReadOnlyList noiseSkills) { var prefix = $"[{skill.Name}]"; var log = (string msg) => spinner.Log($"{prefix} {msg}"); @@ -415,6 +434,12 @@ internal static List CheckAggregateDescriptionLimits(IReadOnlyList 0) + { + return await EvaluateSkillNoise(skill, noiseSkills, config, profile, spinner); + } + // Launch overfitting check in parallel with scenario execution var workDir = Path.GetTempPath(); Task overfittingTask = Task.FromResult(null); @@ -680,6 +705,222 @@ private static async Task ExecuteRun( return new RunExecutionResult(baseline, withSkillResult, pairwise, skillActivation); } + // --- Noise-only evaluation: skill-only vs all-skills (no pure-agent baseline) --- + + private static async Task EvaluateSkillNoise( + SkillInfo skill, + IReadOnlyList noiseSkills, + ValidatorConfig config, + SkillProfile profile, + Spinner spinner) + { + var prefix = $"[{skill.Name}]"; + var log = (string msg) => spinner.Log($"{prefix} {msg}"); + + NoiseTestResult noiseResult; + try + { + noiseResult = await ExecuteNoiseTest(skill, noiseSkills, config, spinner); + } + catch (Exception ex) + { + log($"\u26a0\ufe0f Noise test failed: {ex.Message}"); + return new SkillVerdict + { + SkillName = skill.Name, + SkillPath = skill.Path, + Passed = false, + Scenarios = [], + OverallImprovementScore = 0, + Reason = $"Noise test execution failed: {ex.Message}", + FailureKind = "noise_degradation", + ProfileWarnings = profile.Warnings, + }; + } + + var verdict = new SkillVerdict + { + SkillName = skill.Name, + SkillPath = skill.Path, + Passed = noiseResult.Passed, + Scenarios = [], + OverallImprovementScore = 0, + Reason = noiseResult.Reason, + FailureKind = noiseResult.Passed ? null : "noise_degradation", + ProfileWarnings = profile.Warnings, + NoiseTestResult = noiseResult, + }; + + if (!noiseResult.Passed) + { + log($"\x1b[33m\u26a0\ufe0f Noise test: quality degraded by {noiseResult.OverallDegradation * 100:F1}% with {noiseResult.TotalSkillsLoaded} skills loaded\x1b[0m"); + } + else + { + log($"\u2705 Noise test passed ({noiseResult.TotalSkillsLoaded} skills loaded, degradation: {noiseResult.OverallDegradation * 100:F1}%)"); + } + + var noiseNotActivated = noiseResult.Scenarios.Where(s => s.SkillActivation is { Activated: false }).ToList(); + if (noiseNotActivated.Count > 0) + { + var names = string.Join(", ", noiseNotActivated.Select(s => s.ScenarioName)); + log($"\x1b[33m\u26a0\ufe0f Skills NOT activated in noise scenario(s): {names}\x1b[0m"); + } + + log($"{(verdict.Passed ? "✅" : "❌")} Done (noise degradation: {noiseResult.OverallDegradation * 100:F1}%)"); + return verdict; + } + + // --- Noise test: run scenarios with all discovered skills loaded --- + + private static async Task ExecuteNoiseTest( + SkillInfo targetSkill, + IReadOnlyList allSkills, + ValidatorConfig config, + Spinner spinner) + { + var prefix = $"[{targetSkill.Name}/noise]"; + var log = (string msg) => spinner.Log($"{prefix} {msg}"); + + var otherSkills = allSkills.Where(s => !string.Equals(s.Path, targetSkill.Path, StringComparison.OrdinalIgnoreCase)).ToList(); + int totalLoaded = otherSkills.Count + 1; // target + others + + log($"🔊 Running noise test with {totalLoaded} skills loaded..."); + + var noiseScenarios = new List(); + using var scenarioLimit = new ConcurrencyLimiter(config.ParallelScenarios); + + var tasks = targetSkill.EvalConfig!.Scenarios + .Where(s => s.ExpectActivation) // only test positive scenarios + .Select(scenario => scenarioLimit.RunAsync(async () => + { + var tag = $"[{targetSkill.Name}/noise/{scenario.Name}]"; + var scenarioLog = (string msg) => spinner.Log($"{tag} {msg}"); + + scenarioLog($"running skill-only vs all-skills ({config.Runs} run(s))..."); + + using var runLimit = new ConcurrencyLimiter(config.ParallelRuns); + + var runResults = await Task.WhenAll(Enumerable.Range(0, config.Runs).Select(runIndex => + runLimit.RunAsync(async () => + { + // Run with target skill only + var skillOnlyMetrics = await AgentRunner.RunAgent(new RunOptions( + scenario, targetSkill, targetSkill.EvalPath, config.Model, config.Verbose, scenarioLog)); + + // Run with all skills loaded + var allSkillsMetrics = await AgentRunner.RunAgent(new RunOptions( + scenario, targetSkill, targetSkill.EvalPath, config.Model, config.Verbose, scenarioLog, AdditionalSkills: otherSkills)); + + // Evaluate assertions on both + if (scenario.Assertions is { Count: > 0 }) + { + skillOnlyMetrics.AssertionResults = await AssertionEvaluator.EvaluateAssertions( + scenario.Assertions, skillOnlyMetrics.AgentOutput, skillOnlyMetrics.WorkDir); + allSkillsMetrics.AssertionResults = await AssertionEvaluator.EvaluateAssertions( + scenario.Assertions, allSkillsMetrics.AgentOutput, allSkillsMetrics.WorkDir); + } + var soConstraints = AssertionEvaluator.EvaluateConstraints(scenario, skillOnlyMetrics); + var asConstraints = AssertionEvaluator.EvaluateConstraints(scenario, allSkillsMetrics); + skillOnlyMetrics.AssertionResults = [..skillOnlyMetrics.AssertionResults, ..soConstraints]; + allSkillsMetrics.AssertionResults = [..allSkillsMetrics.AssertionResults, ..asConstraints]; + + skillOnlyMetrics.TaskCompleted = scenario.Assertions is { Count: > 0 } || soConstraints.Count > 0 + ? skillOnlyMetrics.AssertionResults.All(a => a.Passed) + : skillOnlyMetrics.ErrorCount == 0; + allSkillsMetrics.TaskCompleted = scenario.Assertions is { Count: > 0 } || asConstraints.Count > 0 + ? allSkillsMetrics.AssertionResults.All(a => a.Passed) + : allSkillsMetrics.ErrorCount == 0; + + // Judge both runs + var judgeOpts = new JudgeOptions(config.JudgeModel, config.Verbose, config.JudgeTimeout, skillOnlyMetrics.WorkDir, targetSkill.Path); + JudgeResult skillOnlyJudge, allSkillsJudge; + try + { + skillOnlyJudge = await Services.Judge.JudgeRun(scenario, skillOnlyMetrics, judgeOpts); + } + catch + { + skillOnlyJudge = new JudgeResult([], 3, "Judge failed"); + } + try + { + allSkillsJudge = await Services.Judge.JudgeRun(scenario, allSkillsMetrics, + judgeOpts with { WorkDir = allSkillsMetrics.WorkDir }); + } + catch + { + allSkillsJudge = new JudgeResult([], 3, "Judge failed"); + } + + var skillOnly = new RunResult(skillOnlyMetrics, skillOnlyJudge); + var allSkills = new RunResult(allSkillsMetrics, allSkillsJudge); + var activation = MetricsCollector.ExtractSkillActivation( + allSkillsMetrics.Events, skillOnlyMetrics.ToolCallBreakdown); + + return (SkillOnly: skillOnly, AllSkills: allSkills, Activation: activation); + }))); + + scenarioLog($"✓ All {config.Runs} noise run(s) complete"); + + // Average across runs, then compare the averaged results + var avgSkillOnly = AverageResults(runResults.Select(r => r.SkillOnly).ToList()); + var avgAllSkills = AverageResults(runResults.Select(r => r.AllSkills).ToList()); + + // Compare: skill-only is "baseline", all-skills is "with-skill" + // A positive score means all-skills is *better*, negative means degradation + var comparison = Comparator.CompareScenario(scenario.Name, avgSkillOnly, avgAllSkills); + var degradation = -comparison.ImprovementScore; // positive = degradation + + // Aggregate activation info across runs + var activation = new SkillActivationInfo( + Activated: runResults.Any(r => r.Activation.Activated), + DetectedSkills: runResults.SelectMany(r => r.Activation.DetectedSkills).Distinct().ToList(), + ExtraTools: runResults.SelectMany(r => r.Activation.ExtraTools).Distinct().ToList(), + SkillEventCount: runResults.Sum(r => r.Activation.SkillEventCount)); + + scenarioLog($"✓ degradation: {degradation * 100:F1}%, target skill activated: {activation.Activated}"); + + return new NoiseScenarioResult( + scenario.Name, + avgSkillOnly, + avgAllSkills, + degradation, + comparison.Breakdown, + activation, + totalLoaded); + })); + + noiseScenarios = (await Task.WhenAll(tasks)).ToList(); + + // Aggregate only positive (harmful) degradations so that improvements don't mask regressions + double overallDegradation = noiseScenarios.Count > 0 ? noiseScenarios.Average(s => Math.Max(0, s.DegradationScore)) : 0; + + // Also enforce a per-scenario cap so a single bad scenario can't be hidden by others + var worstScenario = noiseScenarios.Count > 0 ? noiseScenarios.MaxBy(s => s.DegradationScore) : null; + double worstDegradation = worstScenario?.DegradationScore ?? 0; + + bool avgPassed = overallDegradation <= config.NoiseDegradationLimit; + bool worstScenarioPassed = worstDegradation <= config.NoiseMaxScenarioDegradation; + bool passed = avgPassed && worstScenarioPassed; + + string reason; + if (!worstScenarioPassed) + { + reason = $"Scenario '{worstScenario!.ScenarioName}' degradation {worstDegradation * 100:F1}% exceeds per-scenario threshold of {config.NoiseMaxScenarioDegradation * 100:F1}% ({totalLoaded} skills loaded)"; + } + else if (!avgPassed) + { + reason = $"Average degradation {overallDegradation * 100:F1}% exceeds threshold of {config.NoiseDegradationLimit * 100:F1}% ({totalLoaded} skills loaded)"; + } + else + { + reason = $"Quality degradation {overallDegradation * 100:F1}% within threshold of {config.NoiseDegradationLimit * 100:F1}%, worst scenario {worstDegradation * 100:F1}% within {config.NoiseMaxScenarioDegradation * 100:F1}% ({totalLoaded} skills loaded)"; + } + + return new NoiseTestResult(noiseScenarios, overallDegradation, passed, reason, totalLoaded); + } + private static RunResult AverageResults(List runs) { if (runs.Count == 1) return runs[0]; diff --git a/eng/skill-validator/src/Models/Models.cs b/eng/skill-validator/src/Models/Models.cs index 6081bcb8d1..2374b7f60d 100644 --- a/eng/skill-validator/src/Models/Models.cs +++ b/eng/skill-validator/src/Models/Models.cs @@ -259,6 +259,7 @@ public sealed class SkillVerdict public IReadOnlyList? ProfileWarnings { get; set; } public bool SkillNotActivated { get; set; } public OverfittingResult? OverfittingResult { get; set; } + public NoiseTestResult? NoiseTestResult { get; set; } } // --- Overfitting assessment --- @@ -306,6 +307,24 @@ public sealed record OverfittingJudgeOptions( int Timeout, string WorkDir); +// --- Multi-skill noise test --- + +public sealed record NoiseScenarioResult( + string ScenarioName, + RunResult WithSkillOnly, + RunResult WithAllSkills, + double DegradationScore, + MetricBreakdown Breakdown, + SkillActivationInfo? SkillActivation, + int TotalSkillsLoaded); + +public sealed record NoiseTestResult( + IReadOnlyList Scenarios, + double OverallDegradation, + bool Passed, + string Reason, + int TotalSkillsLoaded); + // --- Config --- public sealed record ReporterSpec(ReporterType Type); @@ -340,6 +359,9 @@ public sealed record ValidatorConfig public string? TestsDir { get; init; } public bool OverfittingCheck { get; init; } = true; public bool OverfittingFix { get; init; } + public string? NoiseSkillsDir { get; init; } + public double NoiseDegradationLimit { get; init; } = 0.2; + public double NoiseMaxScenarioDegradation { get; init; } = 0.4; } public static class DefaultWeights diff --git a/eng/skill-validator/src/Services/AgentRunner.cs b/eng/skill-validator/src/Services/AgentRunner.cs index 38c246567b..ac857e62b2 100644 --- a/eng/skill-validator/src/Services/AgentRunner.cs +++ b/eng/skill-validator/src/Services/AgentRunner.cs @@ -14,7 +14,8 @@ public sealed record RunOptions( string? EvalPath, string Model, bool Verbose, - Action? Log = null); + Action? Log = null, + IReadOnlyList? AdditionalSkills = null); public static class AgentRunner { @@ -105,8 +106,11 @@ public static bool CheckPermission(PermissionRequest request, string workDir, st internal static SessionConfig BuildSessionConfig( SkillInfo? skill, string model, string workDir, - IReadOnlyDictionary? mcpServers = null) + IReadOnlyDictionary? mcpServers = null, + IReadOnlyList? additionalSkills = null) { + // The SDK expects SkillDirectories entries to be parent directories that + // it scans for child folders containing SKILL.md. var skillPath = skill is not null ? Path.GetDirectoryName(skill.Path) : null; // Create a unique temporary config directory for this session to not share any data @@ -114,6 +118,32 @@ internal static SessionConfig BuildSessionConfig( Directory.CreateDirectory(configDir); _workDirs.Add(configDir); + // Build skill directories list: primary skill + any additional skills. + // For additional skills we stage a temp directory with copies of each + // skill's SKILL.md so the SDK discovers exactly those skills — not + // every sibling that happens to share the same parent directory. + var skillDirs = new List(); + if (skillPath is not null) skillDirs.Add(skillPath); + if (additionalSkills is { Count: > 0 }) + { + var stageDir = Path.Combine(Path.GetTempPath(), $"sv-noise-{Guid.NewGuid():N}"); + Directory.CreateDirectory(stageDir); + _workDirs.Add(stageDir); + + foreach (var s in additionalSkills) + { + var skillMdPath = Path.Combine(s.Path, "SKILL.md"); + if (!File.Exists(skillMdPath)) + continue; + + var stagedSkillDir = Path.Combine(stageDir, Path.GetFileName(s.Path)); + Directory.CreateDirectory(stagedSkillDir); + File.Copy(skillMdPath, Path.Combine(stagedSkillDir, "SKILL.md")); + } + + skillDirs.Add(stageDir); + } + // Convert MCPServerDef records to the SDK's Dictionary shape Dictionary? sdkMcp = null; if (mcpServers is { Count: > 0 }) @@ -139,7 +169,7 @@ internal static SessionConfig BuildSessionConfig( Model = model, Streaming = true, WorkingDirectory = workDir, - SkillDirectories = skill is not null ? [skillPath!] : [], + SkillDirectories = skillDirs, ConfigDir = configDir, McpServers = sdkMcp, InfiniteSessions = new InfiniteSessionConfig { Enabled = false }, @@ -183,7 +213,7 @@ private static async Task RunAgentCore(RunOptions options, Cancellat var client = await GetSharedClient(options.Verbose); await using var session = await client.CreateSessionAsync( - BuildSessionConfig(options.Skill, options.Model, workDir, options.Skill?.McpServers)); + BuildSessionConfig(options.Skill, options.Model, workDir, options.Skill?.McpServers, options.AdditionalSkills)); var done = new TaskCompletionSource(); var effectiveTimeout = options.Scenario.Timeout; diff --git a/eng/skill-validator/src/Services/Reporter.cs b/eng/skill-validator/src/Services/Reporter.cs index 83e542b62d..47e40a5119 100644 --- a/eng/skill-validator/src/Services/Reporter.cs +++ b/eng/skill-validator/src/Services/Reporter.cs @@ -133,6 +133,25 @@ private static void ReportConsole(IReadOnlyList verdicts, bool ver Console.WriteLine($" \x1b[2m•\x1b[0m [{item.Classification}] \x1b[2m{item.AssertionSummary}\x1b[0m\n \x1b[2m— {item.Reasoning}\x1b[0m"); } } + + // Noise test results + if (verdict.NoiseTestResult is { } noiseResult) + { + Console.WriteLine(); + var noiseIcon = noiseResult.Passed ? "✅" : "⚠️"; + var noiseColor = noiseResult.Passed ? "\x1b[32m" : "\x1b[33m"; + Console.WriteLine($" 🔊 Noise test ({noiseResult.TotalSkillsLoaded} skills loaded): {noiseColor}{noiseResult.OverallDegradation * 100:F1}% avg degradation\x1b[0m {noiseIcon}"); + Console.WriteLine($" \x1b[2m{noiseResult.Reason}\x1b[0m"); + + foreach (var ns in noiseResult.Scenarios) + { + var nsIcon = ns.DegradationScore <= 0 ? "\x1b[32m↑\x1b[0m" : "\x1b[33m↓\x1b[0m"; + var activated = ns.SkillActivation?.Activated == true ? "✅" : "⚠️ not activated"; + Console.WriteLine($" {nsIcon} {ns.ScenarioName} degradation: {ns.DegradationScore * 100:F1}% target skill: {activated}"); + Console.WriteLine($" \x1b[2mskill-only: {ns.WithSkillOnly.JudgeResult.OverallScore:F1}/5 → all-skills: {ns.WithAllSkills.JudgeResult.OverallScore:F1}/5\x1b[0m"); + } + } + if (verdict.Scenarios.Count > 0) { Console.WriteLine(); @@ -404,6 +423,41 @@ public static string GenerateMarkdownSummary( if (anyTimeout) sb.AppendLine("\n> ⏰ **timeout** — run hit the scenario timeout limit; scoring may be impacted by aborting model execution before it could produce its full output"); + // Noise test results + var withNoise = verdicts.Where(v => v.NoiseTestResult is not null).ToList(); + if (withNoise.Count > 0) + { + sb.AppendLine(); + sb.AppendLine("### Noise Test (Multi-Skill Loading)"); + sb.AppendLine(); + sb.AppendLine("| Skill | Skills Loaded | Degradation | Verdict |"); + sb.AppendLine("|-------|--------------|-------------|---------|"); + foreach (var v in withNoise) + { + var nr = v.NoiseTestResult!; + var icon = nr.Passed ? "✅" : "⚠️"; + sb.AppendLine($"| {v.SkillName} | {nr.TotalSkillsLoaded} | {nr.OverallDegradation * 100:F1}% | {icon} |"); + } + + foreach (var v in withNoise) + { + var nr = v.NoiseTestResult!; + if (nr.Scenarios.Count > 0) + { + sb.AppendLine(); + sb.AppendLine($"**{v.SkillName}** noise scenarios:"); + sb.AppendLine(); + sb.AppendLine("| Scenario | Skill-Only | All-Skills | Degradation | Target Activated |"); + sb.AppendLine("|----------|-----------|------------|-------------|-----------------|"); + foreach (var ns in nr.Scenarios) + { + var activated = ns.SkillActivation?.Activated == true ? "✅" : "⚠️"; + sb.AppendLine($"| {ns.ScenarioName} | {ns.WithSkillOnly.JudgeResult.OverallScore:F1}/5 | {ns.WithAllSkills.JudgeResult.OverallScore:F1}/5 | {ns.DegradationScore * 100:F1}% | {activated} |"); + } + } + } + } + sb.AppendLine($"\nModel: {model ?? "unknown"} | Judge: {judgeModel ?? "unknown"}"); return sb.ToString(); diff --git a/eng/skill-validator/src/Services/SkillDiscovery.cs b/eng/skill-validator/src/Services/SkillDiscovery.cs index 7fcd2a8b96..8ffab01ac8 100644 --- a/eng/skill-validator/src/Services/SkillDiscovery.cs +++ b/eng/skill-validator/src/Services/SkillDiscovery.cs @@ -33,6 +33,29 @@ public static async Task> DiscoverSkills(string targetP return skills; } + /// + /// Recursively discover all skills under a directory tree by finding SKILL.md files. + /// + public static async Task> DiscoverSkillsRecursive(string targetPath, string? testsDir = null) + { + if (!Directory.Exists(targetPath)) + return []; + + var skills = new List(); + foreach (var skillMdPath in Directory.EnumerateFiles(targetPath, "SKILL.md", SearchOption.AllDirectories)) + { + var dirPath = Path.GetDirectoryName(skillMdPath)!; + if (Path.GetFileName(dirPath).StartsWith('.')) + continue; + + var skill = await DiscoverSkillAt(dirPath, testsDir); + if (skill is not null) + skills.Add(skill); + } + + return skills; + } + private static async Task DiscoverSkillAt(string dirPath, string? testsDir) { var skillMdPath = Path.Combine(dirPath, "SKILL.md"); @@ -49,11 +72,9 @@ public static async Task> DiscoverSkills(string targetP string? evalPath = null; EvalConfig? evalConfig = null; - var evalFilePath = testsDir is not null - ? Path.Combine(testsDir, Path.GetFileName(dirPath), "eval.yaml") - : Path.Combine(dirPath, "tests", "eval.yaml"); + var evalFilePath = ResolveEvalPath(dirPath, testsDir); - if (File.Exists(evalFilePath)) + if (evalFilePath is not null && File.Exists(evalFilePath)) { evalPath = evalFilePath; var evalContent = await File.ReadAllTextAsync(evalFilePath); @@ -119,6 +140,39 @@ await File.ReadAllTextAsync(candidate), return null; } + /// + /// Resolve the eval.yaml path for a skill. Tries flat layout first, + /// then searches one level of subdirectories under testsDir. + /// + private static string? ResolveEvalPath(string skillDirPath, string? testsDir) + { + var skillDirName = Path.GetFileName(skillDirPath); + + if (testsDir is null) + { + var inTree = Path.Combine(skillDirPath, "tests", "eval.yaml"); + return File.Exists(inTree) ? inTree : null; + } + + // Flat: testsDir//eval.yaml + var flat = Path.Combine(testsDir, skillDirName, "eval.yaml"); + if (File.Exists(flat)) + return flat; + + // Nested: testsDir///eval.yaml (e.g., tests/dotnet/csharp-scripts/eval.yaml) + if (Directory.Exists(testsDir)) + { + foreach (var subDir in Directory.GetDirectories(testsDir)) + { + var nested = Path.Combine(subDir, skillDirName, "eval.yaml"); + if (File.Exists(nested)) + return nested; + } + } + + return null; + } + private static readonly IDeserializer FrontmatterDeserializer = new StaticDeserializerBuilder(new SkillValidatorYamlContext()) .WithNamingConvention(UnderscoredNamingConvention.Instance) .IgnoreUnmatchedProperties() diff --git a/eng/skill-validator/src/SkillValidator.csproj b/eng/skill-validator/src/SkillValidator.csproj index 925c30c898..c48b07bdcf 100644 --- a/eng/skill-validator/src/SkillValidator.csproj +++ b/eng/skill-validator/src/SkillValidator.csproj @@ -18,7 +18,7 @@ true - --results-dir "$([MSBuild]::NormalizePath('$(ArtifactsPath)', 'TestResults', '$(AssemblyName)'))" --parallel-skills 3 --parallel-scenarios 3 --parallel-runs 3 + --results-dir "$([MSBuild]::NormalizePath('$(ArtifactsPath)', 'TestResults', '$(AssemblyName)'))" --parallel-skills 3 --parallel-scenarios 3 --parallel-runs 3 diff --git a/eng/skill-validator/src/SkillValidatorJsonContext.cs b/eng/skill-validator/src/SkillValidatorJsonContext.cs index 90e5420e72..8fd69c82a0 100644 --- a/eng/skill-validator/src/SkillValidatorJsonContext.cs +++ b/eng/skill-validator/src/SkillValidatorJsonContext.cs @@ -30,6 +30,8 @@ namespace SkillValidator; [JsonSerializable(typeof(RubricOverfitAssessment))] [JsonSerializable(typeof(AssertionOverfitAssessment))] [JsonSerializable(typeof(OverfittingSeverity))] +[JsonSerializable(typeof(NoiseScenarioResult))] +[JsonSerializable(typeof(NoiseTestResult))] [JsonSerializable(typeof(PairwiseMagnitude))] [JsonSerializable(typeof(AssertionType))] [JsonSerializable(typeof(MCPServerDef))] diff --git a/eng/skill-validator/tests/DiscoveryTests.cs b/eng/skill-validator/tests/DiscoveryTests.cs index 3f2d38e40a..d9613b867e 100644 --- a/eng/skill-validator/tests/DiscoveryTests.cs +++ b/eng/skill-validator/tests/DiscoveryTests.cs @@ -133,4 +133,93 @@ public async Task ReturnsNullWhenNoPluginJson() Directory.Delete(tmpDir, true); } } + + [Fact] + public async Task DiscoverSkillsRecursiveFindsNestedSkills() + { + // Simulates plugins//skills//SKILL.md layout + var tmpDir = Path.Combine(Path.GetTempPath(), $"skill-test-{Guid.NewGuid():N}"); + var skill1Dir = Path.Combine(tmpDir, "plugin-a", "skills", "skill-one"); + var skill2Dir = Path.Combine(tmpDir, "plugin-b", "skills", "skill-two"); + Directory.CreateDirectory(skill1Dir); + Directory.CreateDirectory(skill2Dir); + try + { + await File.WriteAllTextAsync(Path.Combine(skill1Dir, "SKILL.md"), "---\nname: skill-one\ndescription: first\n---\nBody", TestContext.Current.CancellationToken); + await File.WriteAllTextAsync(Path.Combine(skill2Dir, "SKILL.md"), "---\nname: skill-two\ndescription: second\n---\nBody", TestContext.Current.CancellationToken); + + var skills = await SkillDiscovery.DiscoverSkillsRecursive(tmpDir); + Assert.Equal(2, skills.Count); + var names = skills.Select(s => s.Name).OrderBy(n => n).ToList(); + Assert.Equal("skill-one", names[0]); + Assert.Equal("skill-two", names[1]); + } + finally + { + Directory.Delete(tmpDir, true); + } + } + + [Fact] + public async Task DiscoverSkillsRecursiveReturnsEmptyForMissingDir() + { + var skills = await SkillDiscovery.DiscoverSkillsRecursive(Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString("N"))); + Assert.Empty(skills); + } + + [Fact] + public async Task ResolveEvalPathFindsNestedTestDir() + { + // Layout: tests///eval.yaml + var tmpDir = Path.Combine(Path.GetTempPath(), $"skill-test-{Guid.NewGuid():N}"); + var skillDir = Path.Combine(tmpDir, "plugins", "my-plugin", "skills", "my-skill"); + var testsDir = Path.Combine(tmpDir, "tests"); + var evalDir = Path.Combine(testsDir, "my-plugin", "my-skill"); + Directory.CreateDirectory(skillDir); + Directory.CreateDirectory(evalDir); + try + { + await File.WriteAllTextAsync(Path.Combine(skillDir, "SKILL.md"), "---\nname: my-skill\ndescription: test\n---\nBody", TestContext.Current.CancellationToken); + await File.WriteAllTextAsync(Path.Combine(evalDir, "eval.yaml"), "scenarios:\n - name: test\n prompt: hi\n assertions:\n - type: exit_success", TestContext.Current.CancellationToken); + + var skills = await SkillDiscovery.DiscoverSkills(skillDir, testsDir); + Assert.Single(skills); + Assert.NotNull(skills[0].EvalPath); + Assert.Contains("my-plugin", skills[0].EvalPath!); + } + finally + { + Directory.Delete(tmpDir, true); + } + } + + [Fact] + public async Task ResolveEvalPathPrefersFlatLayout() + { + // When both flat and nested exist, flat wins + var tmpDir = Path.Combine(Path.GetTempPath(), $"skill-test-{Guid.NewGuid():N}"); + var skillDir = Path.Combine(tmpDir, "my-skill"); + var testsDir = Path.Combine(tmpDir, "tests"); + var flatEvalDir = Path.Combine(testsDir, "my-skill"); + var nestedEvalDir = Path.Combine(testsDir, "some-plugin", "my-skill"); + Directory.CreateDirectory(skillDir); + Directory.CreateDirectory(flatEvalDir); + Directory.CreateDirectory(nestedEvalDir); + try + { + await File.WriteAllTextAsync(Path.Combine(skillDir, "SKILL.md"), "---\nname: my-skill\ndescription: test\n---\nBody", TestContext.Current.CancellationToken); + await File.WriteAllTextAsync(Path.Combine(flatEvalDir, "eval.yaml"), "scenarios:\n - name: test\n prompt: hi\n assertions:\n - type: exit_success", TestContext.Current.CancellationToken); + await File.WriteAllTextAsync(Path.Combine(nestedEvalDir, "eval.yaml"), "scenarios:\n - name: test\n prompt: hi\n assertions:\n - type: exit_success", TestContext.Current.CancellationToken); + + var skills = await SkillDiscovery.DiscoverSkills(skillDir, testsDir); + Assert.Single(skills); + Assert.NotNull(skills[0].EvalPath); + // Flat path should win + Assert.DoesNotContain("some-plugin", skills[0].EvalPath!); + } + finally + { + Directory.Delete(tmpDir, true); + } + } } diff --git a/eng/skill-validator/tests/RunnerTests.cs b/eng/skill-validator/tests/RunnerTests.cs index 14c87cc3ba..1e05b7d0bf 100644 --- a/eng/skill-validator/tests/RunnerTests.cs +++ b/eng/skill-validator/tests/RunnerTests.cs @@ -25,6 +25,51 @@ public void SetsSkillDirectoriesToParentOfSkillPath() Assert.Equal(Path.GetDirectoryName(MockSkill.Path), config.SkillDirectories![0]); } + [Fact] + public async Task AdditionalSkillsStageOnlyVerifiedSkillDirs() + { + // Create real temp directories with SKILL.md so the staging logic finds them + var tmpBase = Path.Combine(Path.GetTempPath(), $"sv-test-{Guid.NewGuid():N}"); + var skillADir = Path.Combine(tmpBase, "plugin-a", "skills", "skill-a"); + var skillBDir = Path.Combine(tmpBase, "plugin-b", "skills", "skill-b"); + var noSkillDir = Path.Combine(tmpBase, "plugin-c", "skills", "not-a-skill"); + Directory.CreateDirectory(skillADir); + Directory.CreateDirectory(skillBDir); + Directory.CreateDirectory(noSkillDir); + File.WriteAllText(Path.Combine(skillADir, "SKILL.md"), "# A"); + File.WriteAllText(Path.Combine(skillBDir, "SKILL.md"), "# B"); + // noSkillDir intentionally has no SKILL.md + + try + { + var additionalSkills = new[] + { + new SkillInfo("skill-a", "A", skillADir, Path.Combine(skillADir, "SKILL.md"), "# A", null, null), + new SkillInfo("skill-b", "B", skillBDir, Path.Combine(skillBDir, "SKILL.md"), "# B", null, null), + new SkillInfo("no-skill", "None", noSkillDir, Path.Combine(noSkillDir, "SKILL.md"), "", null, null), + }; + + var config = AgentRunner.BuildSessionConfig(MockSkill, "gpt-4.1", "C:\\tmp\\work", + additionalSkills: additionalSkills); + + // Primary skill parent + one staging directory for additional skills + Assert.Equal(2, config.SkillDirectories!.Count); + Assert.Equal(Path.GetDirectoryName(MockSkill.Path), config.SkillDirectories[0]); + + var stageDir = config.SkillDirectories[1]; + Assert.StartsWith(Path.GetTempPath(), stageDir); + + // Staging dir should contain links only for directories that have SKILL.md + var stagedEntries = Directory.GetDirectories(stageDir).Select(Path.GetFileName).OrderBy(n => n).ToArray(); + Assert.Equal(new[] { "skill-a", "skill-b" }, stagedEntries); + } + finally + { + try { Directory.Delete(tmpBase, true); } catch { } + try { await AgentRunner.CleanupWorkDirs(); } catch { } + } + } + [Fact] public void SetsWorkingDirectoryToWorkDir() {