From 8e4365fa17e692d1b168d2277acb3b1718435f27 Mon Sep 17 00:00:00 2001 From: Youssef1313 Date: Wed, 18 Mar 2026 12:12:31 +0100 Subject: [PATCH 01/10] Add 'run_command_and_assert' assertion --- eng/skill-validator/src/Models/Models.cs | 8 ++- .../src/Services/AssertionEvaluator.cs | 61 +++++++++++++++++++ .../src/Services/EvalSchema.cs | 18 +++++- 3 files changed, 85 insertions(+), 2 deletions(-) diff --git a/eng/skill-validator/src/Models/Models.cs b/eng/skill-validator/src/Models/Models.cs index 2641a10380..35dccfb846 100644 --- a/eng/skill-validator/src/Models/Models.cs +++ b/eng/skill-validator/src/Models/Models.cs @@ -16,6 +16,7 @@ public enum AssertionType OutputMatches, OutputNotMatches, ExitSuccess, + RunCommandAndAssert, ExpectTools, RejectTools, MaxTurns, @@ -26,7 +27,12 @@ public sealed record Assertion( AssertionType Type, string? Path = null, string? Value = null, - string? Pattern = null); + string? Pattern = null, + string? CommandToRun = null, + string? CommandArguments = null, + int? ExpectedExitCode = null, + string? ExpectedStdOut = null, + string? ExpectedStdError = null); public sealed record AssertionResult( Assertion Assertion, diff --git a/eng/skill-validator/src/Services/AssertionEvaluator.cs b/eng/skill-validator/src/Services/AssertionEvaluator.cs index eab00ced4a..ffbc418d58 100644 --- a/eng/skill-validator/src/Services/AssertionEvaluator.cs +++ b/eng/skill-validator/src/Services/AssertionEvaluator.cs @@ -1,3 +1,5 @@ +using System.Diagnostics; +using System.Text; using System.Text.RegularExpressions; using Microsoft.Extensions.FileSystemGlobbing; using Microsoft.Extensions.FileSystemGlobbing.Abstractions; @@ -97,6 +99,7 @@ private static async Task EvaluateAssertion( AssertionType.OutputMatches => EvalOutputMatches(assertion, agentOutput), AssertionType.OutputNotMatches => EvalOutputNotMatches(assertion, agentOutput), AssertionType.ExitSuccess => EvalExitSuccess(assertion, agentOutput), + AssertionType.RunCommandAndAssert => EvalRunCommandAndAssert(assertion, workDir), _ => new AssertionResult(assertion, false, $"Unknown assertion type: {assertion.Type}"), }; } @@ -236,6 +239,64 @@ private static AssertionResult EvalExitSuccess(Assertion a, string agentOutput) : "Agent produced no output"); } + private static AssertionResult EvalRunCommandAndAssert(Assertion a, string workDir) + { + var command = a.CommandToRun ?? throw new UnreachableException(); + + var processStartInfo = new ProcessStartInfo(command, a.CommandArguments ?? string.Empty) + { + WorkingDirectory = workDir, + RedirectStandardOutput = true, + RedirectStandardError = true, + UseShellExecute = false, + }; + + var process = Process.Start(processStartInfo); + + process!.BeginOutputReadLine(); + process.BeginErrorReadLine(); + var outBuilder = new StringBuilder(); + var errBuilder = new StringBuilder(); + process.OutputDataReceived += (_, e) => + { + if (e.Data is not null) + outBuilder.AppendLine(e.Data); + }; + process.ErrorDataReceived += (_, e) => + { + if (e.Data is not null) + errBuilder.AppendLine(e.Data); + }; + + process.WaitForExit(); + + var actualExitCode = process.ExitCode; + if (a.ExpectedExitCode.HasValue && a.ExpectedExitCode.Value != actualExitCode) + { + return new AssertionResult(a, false, $"Command exited with code {actualExitCode} but expected {a.ExpectedExitCode.Value}"); + } + + if (a.ExpectedStdOut is not null) + { + var actualStdOut = outBuilder.ToString(); + if (!actualStdOut.Contains(a.ExpectedStdOut, StringComparison.Ordinal)) + { + return new AssertionResult(a, false, $"Command stdout did not contain expected value. Stdout: {actualStdOut}"); + } + } + + if (a.ExpectedStdError is not null) + { + var actualStdErr = errBuilder.ToString(); + if (!actualStdErr.Contains(a.ExpectedStdError, StringComparison.Ordinal)) + { + return new AssertionResult(a, false, $"Command stderr did not contain expected value. Stderr: {actualStdErr}"); + } + } + + return new AssertionResult(a, true, string.Empty); + } + private static Task FileExistsGlob(string pattern, string workDir) { try diff --git a/eng/skill-validator/src/Services/EvalSchema.cs b/eng/skill-validator/src/Services/EvalSchema.cs index 6130da3a08..349c38ed5b 100644 --- a/eng/skill-validator/src/Services/EvalSchema.cs +++ b/eng/skill-validator/src/Services/EvalSchema.cs @@ -84,6 +84,7 @@ private static Assertion ParseAssertion(RawAssertion raw) "output_matches" => AssertionType.OutputMatches, "output_not_matches" => AssertionType.OutputNotMatches, "exit_success" => AssertionType.ExitSuccess, + "run_command_and_assert" => AssertionType.RunCommandAndAssert, _ => throw new InvalidOperationException($"Unknown assertion type: {raw.Type}"), }; @@ -108,9 +109,18 @@ private static Assertion ParseAssertion(RawAssertion raw) if (string.IsNullOrWhiteSpace(raw.Pattern)) throw new InvalidOperationException($"Assertion '{raw.Type}' requires 'pattern'"); break; + case AssertionType.RunCommandAndAssert: + if (string.IsNullOrWhiteSpace(raw.CommandToRun)) + throw new InvalidOperationException($"Assertion '{raw.Type}' requires 'command_to_run'"); + + if (raw.ExpectedExitCode is null && + string.IsNullOrWhiteSpace(raw.ExpectedStdOutput) && + string.IsNullOrWhiteSpace(raw.ExpectedStdError)) + throw new InvalidOperationException($"Assertion '{raw.Type}' requires one or more of 'expected_exit_code', 'expected_std_output', or 'expected_std_error'"); + break; } - return new Assertion(type, raw.Path, raw.Value, raw.Pattern); + return new Assertion(type, raw.Path, raw.Value, raw.Pattern, raw.CommandToRun, raw.CommandArguments, raw.ExpectedExitCode, raw.ExpectedStdOutput, raw.ExpectedStdError); } // Raw YAML deserialization models @@ -176,5 +186,11 @@ internal sealed class RawAssertion public string? Path { get; set; } public string? Value { get; set; } public string? Pattern { get; set; } + + public string? CommandToRun { get; set; } + public string? CommandArguments { get; set; } + public int? ExpectedExitCode { get; set; } + public string? ExpectedStdOutput { get; set; } + public string? ExpectedStdError { get; set; } } } From 8c84f995749e7b1962c5d3be1e8542b4fd9c7f47 Mon Sep 17 00:00:00 2001 From: Youssef1313 Date: Wed, 18 Mar 2026 18:14:50 +0100 Subject: [PATCH 02/10] Rename for clarity --- eng/skill-validator/src/Models/Models.cs | 4 ++-- .../src/Services/AssertionEvaluator.cs | 12 +++++++----- eng/skill-validator/src/Services/EvalSchema.cs | 10 +++++----- 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/eng/skill-validator/src/Models/Models.cs b/eng/skill-validator/src/Models/Models.cs index 35dccfb846..c731acba6f 100644 --- a/eng/skill-validator/src/Models/Models.cs +++ b/eng/skill-validator/src/Models/Models.cs @@ -31,8 +31,8 @@ public sealed record Assertion( string? CommandToRun = null, string? CommandArguments = null, int? ExpectedExitCode = null, - string? ExpectedStdOut = null, - string? ExpectedStdError = null); + string? ExpectedStdOutContains = null, + string? ExpectedStdErrorContains = null); public sealed record AssertionResult( Assertion Assertion, diff --git a/eng/skill-validator/src/Services/AssertionEvaluator.cs b/eng/skill-validator/src/Services/AssertionEvaluator.cs index ffbc418d58..95ddac12dc 100644 --- a/eng/skill-validator/src/Services/AssertionEvaluator.cs +++ b/eng/skill-validator/src/Services/AssertionEvaluator.cs @@ -268,7 +268,9 @@ private static AssertionResult EvalRunCommandAndAssert(Assertion a, string workD errBuilder.AppendLine(e.Data); }; - process.WaitForExit(); + // 3 minutes timeout is an arbitrary timeout. + // We can adjust it in future if needed, or make it customizable. + process.WaitForExit(TimeSpan.FromMinutes(3)); var actualExitCode = process.ExitCode; if (a.ExpectedExitCode.HasValue && a.ExpectedExitCode.Value != actualExitCode) @@ -276,19 +278,19 @@ private static AssertionResult EvalRunCommandAndAssert(Assertion a, string workD return new AssertionResult(a, false, $"Command exited with code {actualExitCode} but expected {a.ExpectedExitCode.Value}"); } - if (a.ExpectedStdOut is not null) + if (a.ExpectedStdOutContains is not null) { var actualStdOut = outBuilder.ToString(); - if (!actualStdOut.Contains(a.ExpectedStdOut, StringComparison.Ordinal)) + if (!actualStdOut.Contains(a.ExpectedStdOutContains, StringComparison.Ordinal)) { return new AssertionResult(a, false, $"Command stdout did not contain expected value. Stdout: {actualStdOut}"); } } - if (a.ExpectedStdError is not null) + if (a.ExpectedStdErrorContains is not null) { var actualStdErr = errBuilder.ToString(); - if (!actualStdErr.Contains(a.ExpectedStdError, StringComparison.Ordinal)) + if (!actualStdErr.Contains(a.ExpectedStdErrorContains, StringComparison.Ordinal)) { return new AssertionResult(a, false, $"Command stderr did not contain expected value. Stderr: {actualStdErr}"); } diff --git a/eng/skill-validator/src/Services/EvalSchema.cs b/eng/skill-validator/src/Services/EvalSchema.cs index 349c38ed5b..2e2784f644 100644 --- a/eng/skill-validator/src/Services/EvalSchema.cs +++ b/eng/skill-validator/src/Services/EvalSchema.cs @@ -114,13 +114,13 @@ private static Assertion ParseAssertion(RawAssertion raw) throw new InvalidOperationException($"Assertion '{raw.Type}' requires 'command_to_run'"); if (raw.ExpectedExitCode is null && - string.IsNullOrWhiteSpace(raw.ExpectedStdOutput) && - string.IsNullOrWhiteSpace(raw.ExpectedStdError)) + string.IsNullOrWhiteSpace(raw.ExpectedStdOutputContains) && + string.IsNullOrWhiteSpace(raw.ExpectedStdErrorContains)) throw new InvalidOperationException($"Assertion '{raw.Type}' requires one or more of 'expected_exit_code', 'expected_std_output', or 'expected_std_error'"); break; } - return new Assertion(type, raw.Path, raw.Value, raw.Pattern, raw.CommandToRun, raw.CommandArguments, raw.ExpectedExitCode, raw.ExpectedStdOutput, raw.ExpectedStdError); + return new Assertion(type, raw.Path, raw.Value, raw.Pattern, raw.CommandToRun, raw.CommandArguments, raw.ExpectedExitCode, raw.ExpectedStdOutputContains, raw.ExpectedStdErrorContains); } // Raw YAML deserialization models @@ -190,7 +190,7 @@ internal sealed class RawAssertion public string? CommandToRun { get; set; } public string? CommandArguments { get; set; } public int? ExpectedExitCode { get; set; } - public string? ExpectedStdOutput { get; set; } - public string? ExpectedStdError { get; set; } + public string? ExpectedStdOutputContains { get; set; } + public string? ExpectedStdErrorContains { get; set; } } } From 7798cceae645c19ec902c247bd8b2b619e6248bb Mon Sep 17 00:00:00 2001 From: Youssef1313 Date: Wed, 18 Mar 2026 18:27:34 +0100 Subject: [PATCH 03/10] Add tests --- eng/skill-validator/tests/AssertionsTests.cs | 178 +++++++++++++++++++ 1 file changed, 178 insertions(+) diff --git a/eng/skill-validator/tests/AssertionsTests.cs b/eng/skill-validator/tests/AssertionsTests.cs index 3bf3f1003a..0e0fa828b3 100644 --- a/eng/skill-validator/tests/AssertionsTests.cs +++ b/eng/skill-validator/tests/AssertionsTests.cs @@ -327,3 +327,181 @@ public void ExpectToolsChecksEachToolIndependently() Assert.True(results[2].Passed); // create_file: used } } + +public class RunCommandAndAssertTests : IDisposable +{ + private readonly string _tmpDir; + + public RunCommandAndAssertTests() + { + _tmpDir = Path.Combine(Path.GetTempPath(), $"run-cmd-test-{Guid.NewGuid():N}"); + Directory.CreateDirectory(_tmpDir); + } + + public void Dispose() + { + try { Directory.Delete(_tmpDir, true); } catch { } + } + + private static string Shell => OperatingSystem.IsWindows() ? "cmd" : "/bin/sh"; + + private static string ShellArgs(string command) => + OperatingSystem.IsWindows() + ? $"/c {command}" + : $"-c \"{command}\""; + + [Fact] + public async Task PassesWhenExitCodeMatches() + { + var results = await AssertionEvaluator.EvaluateAssertions( + [new Assertion(AssertionType.RunCommandAndAssert, + CommandToRun: Shell, + CommandArguments: ShellArgs("exit 0"), + ExpectedExitCode: 0)], + "", _tmpDir); + Assert.True(results[0].Passed); + } + + [Fact] + public async Task FailsWhenExitCodeDoesNotMatch() + { + var results = await AssertionEvaluator.EvaluateAssertions( + [new Assertion(AssertionType.RunCommandAndAssert, + CommandToRun: Shell, + CommandArguments: ShellArgs("exit 1"), + ExpectedExitCode: 0)], + "", _tmpDir); + Assert.False(results[0].Passed); + Assert.Contains("exited with code 1 but expected 0", results[0].Message); + } + + [Fact] + public async Task PassesWithNonZeroExpectedExitCode() + { + var results = await AssertionEvaluator.EvaluateAssertions( + [new Assertion(AssertionType.RunCommandAndAssert, + CommandToRun: Shell, + CommandArguments: ShellArgs("exit 42"), + ExpectedExitCode: 42)], + "", _tmpDir); + Assert.True(results[0].Passed); + } + + [Fact] + public async Task PassesWhenStdOutContainsExpectedValue() + { + var results = await AssertionEvaluator.EvaluateAssertions( + [new Assertion(AssertionType.RunCommandAndAssert, + CommandToRun: Shell, + CommandArguments: ShellArgs("echo hello_world"), + ExpectedStdOutContains: "hello_world")], + "", _tmpDir); + Assert.True(results[0].Passed); + } + + [Fact] + public async Task FailsWhenStdOutDoesNotContainExpectedValue() + { + var results = await AssertionEvaluator.EvaluateAssertions( + [new Assertion(AssertionType.RunCommandAndAssert, + CommandToRun: Shell, + CommandArguments: ShellArgs("echo hello"), + ExpectedStdOutContains: "goodbye")], + "", _tmpDir); + Assert.False(results[0].Passed); + Assert.Contains("stdout did not contain expected value", results[0].Message); + } + + [Fact] + public async Task PassesWhenStdErrContainsExpectedValue() + { + var results = await AssertionEvaluator.EvaluateAssertions( + [new Assertion(AssertionType.RunCommandAndAssert, + CommandToRun: Shell, + CommandArguments: ShellArgs("echo error_marker 1>&2"), + ExpectedStdErrorContains: "error_marker")], + "", _tmpDir); + Assert.True(results[0].Passed); + } + + [Fact] + public async Task FailsWhenStdErrDoesNotContainExpectedValue() + { + var results = await AssertionEvaluator.EvaluateAssertions( + [new Assertion(AssertionType.RunCommandAndAssert, + CommandToRun: Shell, + CommandArguments: ShellArgs("echo some_error 1>&2"), + ExpectedStdErrorContains: "different_error")], + "", _tmpDir); + Assert.False(results[0].Passed); + Assert.Contains("stderr did not contain expected value", results[0].Message); + } + + [Fact] + public async Task PassesWhenAllChecksPass() + { + var results = await AssertionEvaluator.EvaluateAssertions( + [new Assertion(AssertionType.RunCommandAndAssert, + CommandToRun: Shell, + CommandArguments: ShellArgs("echo stdout_text && echo stderr_text 1>&2"), + ExpectedExitCode: 0, + ExpectedStdOutContains: "stdout_text", + ExpectedStdErrorContains: "stderr_text")], + "", _tmpDir); + Assert.True(results[0].Passed); + } + + [Fact] + public async Task ExitCodeFailureShortCircuitsOtherChecks() + { + var results = await AssertionEvaluator.EvaluateAssertions( + [new Assertion(AssertionType.RunCommandAndAssert, + CommandToRun: Shell, + CommandArguments: ShellArgs("echo hello && exit 1"), + ExpectedExitCode: 0, + ExpectedStdOutContains: "hello")], + "", _tmpDir); + Assert.False(results[0].Passed); + Assert.Contains("exited with code 1 but expected 0", results[0].Message); + } + + [Fact] + public async Task StdOutFailureShortCircuitsStdErrCheck() + { + var results = await AssertionEvaluator.EvaluateAssertions( + [new Assertion(AssertionType.RunCommandAndAssert, + CommandToRun: Shell, + CommandArguments: ShellArgs("echo wrong && echo expected_error 1>&2"), + ExpectedStdOutContains: "expected_output", + ExpectedStdErrorContains: "expected_error")], + "", _tmpDir); + Assert.False(results[0].Passed); + Assert.Contains("stdout did not contain expected value", results[0].Message); + } + + [Fact] + public async Task IgnoresExitCodeWhenNotSpecified() + { + var results = await AssertionEvaluator.EvaluateAssertions( + [new Assertion(AssertionType.RunCommandAndAssert, + CommandToRun: Shell, + CommandArguments: ShellArgs("echo output_text && exit 1"), + ExpectedStdOutContains: "output_text")], + "", _tmpDir); + Assert.True(results[0].Passed); + } + + [Fact] + public async Task UsesWorkDirAsProcessWorkingDirectory() + { + File.WriteAllText(Path.Combine(_tmpDir, "marker.txt"), "test_content"); + var catCmd = OperatingSystem.IsWindows() ? "type marker.txt" : "cat marker.txt"; + var results = await AssertionEvaluator.EvaluateAssertions( + [new Assertion(AssertionType.RunCommandAndAssert, + CommandToRun: Shell, + CommandArguments: ShellArgs(catCmd), + ExpectedStdOutContains: "test_content")], + "", _tmpDir); + Assert.True(results[0].Passed); + } +} From 9465057516a7c928316ed734f5be79a3cae42534 Mon Sep 17 00:00:00 2001 From: Jan Krivanek Date: Wed, 18 Mar 2026 20:47:26 +0100 Subject: [PATCH 04/10] Fix race condition in command output handling --- eng/skill-validator/src/Services/AssertionEvaluator.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/eng/skill-validator/src/Services/AssertionEvaluator.cs b/eng/skill-validator/src/Services/AssertionEvaluator.cs index 95ddac12dc..544d87b531 100644 --- a/eng/skill-validator/src/Services/AssertionEvaluator.cs +++ b/eng/skill-validator/src/Services/AssertionEvaluator.cs @@ -253,11 +253,9 @@ private static AssertionResult EvalRunCommandAndAssert(Assertion a, string workD var process = Process.Start(processStartInfo); - process!.BeginOutputReadLine(); - process.BeginErrorReadLine(); var outBuilder = new StringBuilder(); var errBuilder = new StringBuilder(); - process.OutputDataReceived += (_, e) => + process!.OutputDataReceived += (_, e) => { if (e.Data is not null) outBuilder.AppendLine(e.Data); @@ -267,6 +265,8 @@ private static AssertionResult EvalRunCommandAndAssert(Assertion a, string workD if (e.Data is not null) errBuilder.AppendLine(e.Data); }; + process.BeginOutputReadLine(); + process.BeginErrorReadLine(); // 3 minutes timeout is an arbitrary timeout. // We can adjust it in future if needed, or make it customizable. From 567696e6ee7cb4d8d4bc54a5b79752e8679366ab Mon Sep 17 00:00:00 2001 From: Jan Krivanek Date: Fri, 20 Mar 2026 16:04:27 +0100 Subject: [PATCH 05/10] Fix async output race condition in RunCommandAndAssert WaitForExit(TimeSpan) does not guarantee that async output event handlers have completed. Add parameterless WaitForExit() call after the timed wait to ensure stdout/stderr are fully captured before checking their contents. Also handle the timeout case explicitly by killing the process. --- eng/skill-validator/src/Services/AssertionEvaluator.cs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/eng/skill-validator/src/Services/AssertionEvaluator.cs b/eng/skill-validator/src/Services/AssertionEvaluator.cs index 544d87b531..116c823387 100644 --- a/eng/skill-validator/src/Services/AssertionEvaluator.cs +++ b/eng/skill-validator/src/Services/AssertionEvaluator.cs @@ -270,7 +270,15 @@ private static AssertionResult EvalRunCommandAndAssert(Assertion a, string workD // 3 minutes timeout is an arbitrary timeout. // We can adjust it in future if needed, or make it customizable. - process.WaitForExit(TimeSpan.FromMinutes(3)); + if (!process.WaitForExit(TimeSpan.FromMinutes(3))) + { + process.Kill(); + return new AssertionResult(a, false, "Command timed out after 3 minutes"); + } + + // The parameterless WaitForExit() ensures that all async output event handlers + // have completed processing. Without this, stdout/stderr may not be fully captured. + process.WaitForExit(); var actualExitCode = process.ExitCode; if (a.ExpectedExitCode.HasValue && a.ExpectedExitCode.Value != actualExitCode) From 6bc66cc5911a95a7fcb4960b9db4124d621d322c Mon Sep 17 00:00:00 2001 From: Jan Krivanek Date: Fri, 20 Mar 2026 18:10:48 +0100 Subject: [PATCH 06/10] Address PR review: extract CommandAssertionArgs, add regex matching, configurable timeout - Extract command-specific fields from Assertion into a dedicated CommandAssertionArgs record type for cleaner separation of concerns - Add ExpectedStdOutMatches/ExpectedStdErrorMatches for regex-based stdout/stderr assertions (matches type asserts) - Make command timeout configurable via assertion-level 'command_timeout' field, falling back to the scenario timeout when not specified - Show actual timeout value in timeout error messages - Update EvalSchema parsing, EvaluateCommand call sites, and all tests - Add new tests for regex matching and timeout configuration --- .../src/Commands/EvaluateCommand.cs | 10 +- eng/skill-validator/src/Models/Models.cs | 16 +- .../src/Services/AssertionEvaluator.cs | 70 +++++--- .../src/Services/EvalSchema.cs | 23 ++- eng/skill-validator/tests/AssertionsTests.cs | 157 ++++++++++++------ 5 files changed, 192 insertions(+), 84 deletions(-) diff --git a/eng/skill-validator/src/Commands/EvaluateCommand.cs b/eng/skill-validator/src/Commands/EvaluateCommand.cs index b419b285fb..93c17ebce7 100644 --- a/eng/skill-validator/src/Commands/EvaluateCommand.cs +++ b/eng/skill-validator/src/Commands/EvaluateCommand.cs @@ -641,9 +641,9 @@ private static async Task ExecuteRun( // Evaluate assertions on all three runs if (scenario.Assertions is { Count: > 0 }) { - baselineMetrics.AssertionResults = await AssertionEvaluator.EvaluateAssertions(scenario.Assertions, baselineMetrics.AgentOutput, baselineMetrics.WorkDir); - isolatedMetrics.AssertionResults = await AssertionEvaluator.EvaluateAssertions(scenario.Assertions, isolatedMetrics.AgentOutput, isolatedMetrics.WorkDir); - pluginMetrics.AssertionResults = await AssertionEvaluator.EvaluateAssertions(scenario.Assertions, pluginMetrics.AgentOutput, pluginMetrics.WorkDir); + baselineMetrics.AssertionResults = await AssertionEvaluator.EvaluateAssertions(scenario.Assertions, baselineMetrics.AgentOutput, baselineMetrics.WorkDir, scenario.Timeout); + isolatedMetrics.AssertionResults = await AssertionEvaluator.EvaluateAssertions(scenario.Assertions, isolatedMetrics.AgentOutput, isolatedMetrics.WorkDir, scenario.Timeout); + pluginMetrics.AssertionResults = await AssertionEvaluator.EvaluateAssertions(scenario.Assertions, pluginMetrics.AgentOutput, pluginMetrics.WorkDir, scenario.Timeout); } // Evaluate constraints on all three runs @@ -889,9 +889,9 @@ private static async Task ExecuteNoiseTest( if (scenario.Assertions is { Count: > 0 }) { skillOnlyMetrics.AssertionResults = await AssertionEvaluator.EvaluateAssertions( - scenario.Assertions, skillOnlyMetrics.AgentOutput, skillOnlyMetrics.WorkDir); + scenario.Assertions, skillOnlyMetrics.AgentOutput, skillOnlyMetrics.WorkDir, scenario.Timeout); allSkillsMetrics.AssertionResults = await AssertionEvaluator.EvaluateAssertions( - scenario.Assertions, allSkillsMetrics.AgentOutput, allSkillsMetrics.WorkDir); + scenario.Assertions, allSkillsMetrics.AgentOutput, allSkillsMetrics.WorkDir, scenario.Timeout); } var soConstraints = AssertionEvaluator.EvaluateConstraints(scenario, skillOnlyMetrics); var asConstraints = AssertionEvaluator.EvaluateConstraints(scenario, allSkillsMetrics); diff --git a/eng/skill-validator/src/Models/Models.cs b/eng/skill-validator/src/Models/Models.cs index c731acba6f..ce0ba64672 100644 --- a/eng/skill-validator/src/Models/Models.cs +++ b/eng/skill-validator/src/Models/Models.cs @@ -23,16 +23,22 @@ public enum AssertionType MaxTokens, } +public sealed record CommandAssertionArgs( + string CommandToRun, + string? CommandArguments = null, + int? ExpectedExitCode = null, + string? ExpectedStdOutContains = null, + string? ExpectedStdErrorContains = null, + string? ExpectedStdOutMatches = null, + string? ExpectedStdErrorMatches = null, + int? Timeout = null); + public sealed record Assertion( AssertionType Type, string? Path = null, string? Value = null, string? Pattern = null, - string? CommandToRun = null, - string? CommandArguments = null, - int? ExpectedExitCode = null, - string? ExpectedStdOutContains = null, - string? ExpectedStdErrorContains = null); + CommandAssertionArgs? CommandArgs = null); public sealed record AssertionResult( Assertion Assertion, diff --git a/eng/skill-validator/src/Services/AssertionEvaluator.cs b/eng/skill-validator/src/Services/AssertionEvaluator.cs index 116c823387..3692afb6f4 100644 --- a/eng/skill-validator/src/Services/AssertionEvaluator.cs +++ b/eng/skill-validator/src/Services/AssertionEvaluator.cs @@ -12,12 +12,13 @@ public static class AssertionEvaluator public static async Task> EvaluateAssertions( IReadOnlyList assertions, string agentOutput, - string workDir) + string workDir, + int scenarioTimeoutSeconds = 120) { var results = new List(); foreach (var assertion in assertions) { - var result = await EvaluateAssertion(assertion, agentOutput, workDir); + var result = await EvaluateAssertion(assertion, agentOutput, workDir, scenarioTimeoutSeconds); results.Add(result); } return results; @@ -86,7 +87,8 @@ public static List EvaluateConstraints( private static async Task EvaluateAssertion( Assertion assertion, string agentOutput, - string workDir) + string workDir, + int scenarioTimeoutSeconds) { return assertion.Type switch { @@ -99,7 +101,7 @@ private static async Task EvaluateAssertion( AssertionType.OutputMatches => EvalOutputMatches(assertion, agentOutput), AssertionType.OutputNotMatches => EvalOutputNotMatches(assertion, agentOutput), AssertionType.ExitSuccess => EvalExitSuccess(assertion, agentOutput), - AssertionType.RunCommandAndAssert => EvalRunCommandAndAssert(assertion, workDir), + AssertionType.RunCommandAndAssert => EvalRunCommandAndAssert(assertion, workDir, scenarioTimeoutSeconds), _ => new AssertionResult(assertion, false, $"Unknown assertion type: {assertion.Type}"), }; } @@ -239,11 +241,13 @@ private static AssertionResult EvalExitSuccess(Assertion a, string agentOutput) : "Agent produced no output"); } - private static AssertionResult EvalRunCommandAndAssert(Assertion a, string workDir) + private static AssertionResult EvalRunCommandAndAssert(Assertion a, string workDir, int scenarioTimeoutSeconds) { - var command = a.CommandToRun ?? throw new UnreachableException(); + var cmd = a.CommandArgs ?? throw new UnreachableException(); + var command = cmd.CommandToRun; + var timeoutSeconds = cmd.Timeout ?? scenarioTimeoutSeconds; - var processStartInfo = new ProcessStartInfo(command, a.CommandArguments ?? string.Empty) + var processStartInfo = new ProcessStartInfo(command, cmd.CommandArguments ?? string.Empty) { WorkingDirectory = workDir, RedirectStandardOutput = true, @@ -268,12 +272,10 @@ private static AssertionResult EvalRunCommandAndAssert(Assertion a, string workD process.BeginOutputReadLine(); process.BeginErrorReadLine(); - // 3 minutes timeout is an arbitrary timeout. - // We can adjust it in future if needed, or make it customizable. - if (!process.WaitForExit(TimeSpan.FromMinutes(3))) + if (!process.WaitForExit(TimeSpan.FromSeconds(timeoutSeconds))) { process.Kill(); - return new AssertionResult(a, false, "Command timed out after 3 minutes"); + return new AssertionResult(a, false, $"Command timed out after {timeoutSeconds}s"); } // The parameterless WaitForExit() ensures that all async output event handlers @@ -281,29 +283,59 @@ private static AssertionResult EvalRunCommandAndAssert(Assertion a, string workD process.WaitForExit(); var actualExitCode = process.ExitCode; - if (a.ExpectedExitCode.HasValue && a.ExpectedExitCode.Value != actualExitCode) + if (cmd.ExpectedExitCode.HasValue && cmd.ExpectedExitCode.Value != actualExitCode) { - return new AssertionResult(a, false, $"Command exited with code {actualExitCode} but expected {a.ExpectedExitCode.Value}"); + return new AssertionResult(a, false, $"Command exited with code {actualExitCode} but expected {cmd.ExpectedExitCode.Value}"); } - if (a.ExpectedStdOutContains is not null) + var actualStdOut = outBuilder.ToString(); + if (cmd.ExpectedStdOutContains is not null) { - var actualStdOut = outBuilder.ToString(); - if (!actualStdOut.Contains(a.ExpectedStdOutContains, StringComparison.Ordinal)) + if (!actualStdOut.Contains(cmd.ExpectedStdOutContains, StringComparison.Ordinal)) { return new AssertionResult(a, false, $"Command stdout did not contain expected value. Stdout: {actualStdOut}"); } } - if (a.ExpectedStdErrorContains is not null) + if (cmd.ExpectedStdOutMatches is not null) { - var actualStdErr = errBuilder.ToString(); - if (!actualStdErr.Contains(a.ExpectedStdErrorContains, StringComparison.Ordinal)) + try + { + if (!Regex.IsMatch(actualStdOut, cmd.ExpectedStdOutMatches, RegexOptions.IgnoreCase, RegexTimeout)) + { + return new AssertionResult(a, false, $"Command stdout did not match pattern '{cmd.ExpectedStdOutMatches}'. Stdout: {actualStdOut}"); + } + } + catch (RegexMatchTimeoutException) + { + return new AssertionResult(a, false, $"Regex pattern '{cmd.ExpectedStdOutMatches}' timed out after {RegexTimeout.TotalSeconds}s"); + } + } + + var actualStdErr = errBuilder.ToString(); + if (cmd.ExpectedStdErrorContains is not null) + { + if (!actualStdErr.Contains(cmd.ExpectedStdErrorContains, StringComparison.Ordinal)) { return new AssertionResult(a, false, $"Command stderr did not contain expected value. Stderr: {actualStdErr}"); } } + if (cmd.ExpectedStdErrorMatches is not null) + { + try + { + if (!Regex.IsMatch(actualStdErr, cmd.ExpectedStdErrorMatches, RegexOptions.IgnoreCase, RegexTimeout)) + { + return new AssertionResult(a, false, $"Command stderr did not match pattern '{cmd.ExpectedStdErrorMatches}'. Stderr: {actualStdErr}"); + } + } + catch (RegexMatchTimeoutException) + { + return new AssertionResult(a, false, $"Regex pattern '{cmd.ExpectedStdErrorMatches}' timed out after {RegexTimeout.TotalSeconds}s"); + } + } + return new AssertionResult(a, true, string.Empty); } diff --git a/eng/skill-validator/src/Services/EvalSchema.cs b/eng/skill-validator/src/Services/EvalSchema.cs index 2e2784f644..3db13279a0 100644 --- a/eng/skill-validator/src/Services/EvalSchema.cs +++ b/eng/skill-validator/src/Services/EvalSchema.cs @@ -115,12 +115,26 @@ private static Assertion ParseAssertion(RawAssertion raw) if (raw.ExpectedExitCode is null && string.IsNullOrWhiteSpace(raw.ExpectedStdOutputContains) && - string.IsNullOrWhiteSpace(raw.ExpectedStdErrorContains)) - throw new InvalidOperationException($"Assertion '{raw.Type}' requires one or more of 'expected_exit_code', 'expected_std_output', or 'expected_std_error'"); + string.IsNullOrWhiteSpace(raw.ExpectedStdErrorContains) && + string.IsNullOrWhiteSpace(raw.ExpectedStdOutputMatches) && + string.IsNullOrWhiteSpace(raw.ExpectedStdErrorMatches)) + throw new InvalidOperationException($"Assertion '{raw.Type}' requires one or more of 'expected_exit_code', 'expected_std_output', 'expected_std_error', 'expected_std_output_matches', or 'expected_std_error_matches'"); break; } - return new Assertion(type, raw.Path, raw.Value, raw.Pattern, raw.CommandToRun, raw.CommandArguments, raw.ExpectedExitCode, raw.ExpectedStdOutputContains, raw.ExpectedStdErrorContains); + CommandAssertionArgs? commandArgs = type == AssertionType.RunCommandAndAssert + ? new CommandAssertionArgs( + raw.CommandToRun!, + raw.CommandArguments, + raw.ExpectedExitCode, + raw.ExpectedStdOutputContains, + raw.ExpectedStdErrorContains, + raw.ExpectedStdOutputMatches, + raw.ExpectedStdErrorMatches, + raw.CommandTimeout) + : null; + + return new Assertion(type, raw.Path, raw.Value, raw.Pattern, commandArgs); } // Raw YAML deserialization models @@ -192,5 +206,8 @@ internal sealed class RawAssertion public int? ExpectedExitCode { get; set; } public string? ExpectedStdOutputContains { get; set; } public string? ExpectedStdErrorContains { get; set; } + public string? ExpectedStdOutputMatches { get; set; } + public string? ExpectedStdErrorMatches { get; set; } + public int? CommandTimeout { get; set; } } } diff --git a/eng/skill-validator/tests/AssertionsTests.cs b/eng/skill-validator/tests/AssertionsTests.cs index 0e0fa828b3..4e4e5eefdb 100644 --- a/eng/skill-validator/tests/AssertionsTests.cs +++ b/eng/skill-validator/tests/AssertionsTests.cs @@ -350,14 +350,30 @@ private static string ShellArgs(string command) => ? $"/c {command}" : $"-c \"{command}\""; + private static Assertion CmdAssertion( + string? commandArguments = null, + int? expectedExitCode = null, + string? expectedStdOutContains = null, + string? expectedStdErrorContains = null, + string? expectedStdOutMatches = null, + string? expectedStdErrorMatches = null, + int? timeout = null) => + new(AssertionType.RunCommandAndAssert, + CommandArgs: new CommandAssertionArgs( + Shell, + commandArguments, + expectedExitCode, + expectedStdOutContains, + expectedStdErrorContains, + expectedStdOutMatches, + expectedStdErrorMatches, + timeout)); + [Fact] public async Task PassesWhenExitCodeMatches() { var results = await AssertionEvaluator.EvaluateAssertions( - [new Assertion(AssertionType.RunCommandAndAssert, - CommandToRun: Shell, - CommandArguments: ShellArgs("exit 0"), - ExpectedExitCode: 0)], + [CmdAssertion(commandArguments: ShellArgs("exit 0"), expectedExitCode: 0)], "", _tmpDir); Assert.True(results[0].Passed); } @@ -366,10 +382,7 @@ [new Assertion(AssertionType.RunCommandAndAssert, public async Task FailsWhenExitCodeDoesNotMatch() { var results = await AssertionEvaluator.EvaluateAssertions( - [new Assertion(AssertionType.RunCommandAndAssert, - CommandToRun: Shell, - CommandArguments: ShellArgs("exit 1"), - ExpectedExitCode: 0)], + [CmdAssertion(commandArguments: ShellArgs("exit 1"), expectedExitCode: 0)], "", _tmpDir); Assert.False(results[0].Passed); Assert.Contains("exited with code 1 but expected 0", results[0].Message); @@ -379,10 +392,7 @@ [new Assertion(AssertionType.RunCommandAndAssert, public async Task PassesWithNonZeroExpectedExitCode() { var results = await AssertionEvaluator.EvaluateAssertions( - [new Assertion(AssertionType.RunCommandAndAssert, - CommandToRun: Shell, - CommandArguments: ShellArgs("exit 42"), - ExpectedExitCode: 42)], + [CmdAssertion(commandArguments: ShellArgs("exit 42"), expectedExitCode: 42)], "", _tmpDir); Assert.True(results[0].Passed); } @@ -391,10 +401,7 @@ [new Assertion(AssertionType.RunCommandAndAssert, public async Task PassesWhenStdOutContainsExpectedValue() { var results = await AssertionEvaluator.EvaluateAssertions( - [new Assertion(AssertionType.RunCommandAndAssert, - CommandToRun: Shell, - CommandArguments: ShellArgs("echo hello_world"), - ExpectedStdOutContains: "hello_world")], + [CmdAssertion(commandArguments: ShellArgs("echo hello_world"), expectedStdOutContains: "hello_world")], "", _tmpDir); Assert.True(results[0].Passed); } @@ -403,10 +410,7 @@ [new Assertion(AssertionType.RunCommandAndAssert, public async Task FailsWhenStdOutDoesNotContainExpectedValue() { var results = await AssertionEvaluator.EvaluateAssertions( - [new Assertion(AssertionType.RunCommandAndAssert, - CommandToRun: Shell, - CommandArguments: ShellArgs("echo hello"), - ExpectedStdOutContains: "goodbye")], + [CmdAssertion(commandArguments: ShellArgs("echo hello"), expectedStdOutContains: "goodbye")], "", _tmpDir); Assert.False(results[0].Passed); Assert.Contains("stdout did not contain expected value", results[0].Message); @@ -416,10 +420,7 @@ [new Assertion(AssertionType.RunCommandAndAssert, public async Task PassesWhenStdErrContainsExpectedValue() { var results = await AssertionEvaluator.EvaluateAssertions( - [new Assertion(AssertionType.RunCommandAndAssert, - CommandToRun: Shell, - CommandArguments: ShellArgs("echo error_marker 1>&2"), - ExpectedStdErrorContains: "error_marker")], + [CmdAssertion(commandArguments: ShellArgs("echo error_marker 1>&2"), expectedStdErrorContains: "error_marker")], "", _tmpDir); Assert.True(results[0].Passed); } @@ -428,25 +429,59 @@ [new Assertion(AssertionType.RunCommandAndAssert, public async Task FailsWhenStdErrDoesNotContainExpectedValue() { var results = await AssertionEvaluator.EvaluateAssertions( - [new Assertion(AssertionType.RunCommandAndAssert, - CommandToRun: Shell, - CommandArguments: ShellArgs("echo some_error 1>&2"), - ExpectedStdErrorContains: "different_error")], + [CmdAssertion(commandArguments: ShellArgs("echo some_error 1>&2"), expectedStdErrorContains: "different_error")], "", _tmpDir); Assert.False(results[0].Passed); Assert.Contains("stderr did not contain expected value", results[0].Message); } + [Fact] + public async Task PassesWhenStdOutMatchesRegex() + { + var results = await AssertionEvaluator.EvaluateAssertions( + [CmdAssertion(commandArguments: ShellArgs("echo build_v2.3.1_ok"), expectedStdOutMatches: @"build_v\d+\.\d+\.\d+_ok")], + "", _tmpDir); + Assert.True(results[0].Passed); + } + + [Fact] + public async Task FailsWhenStdOutDoesNotMatchRegex() + { + var results = await AssertionEvaluator.EvaluateAssertions( + [CmdAssertion(commandArguments: ShellArgs("echo no_version_here"), expectedStdOutMatches: @"v\d+\.\d+\.\d+")], + "", _tmpDir); + Assert.False(results[0].Passed); + Assert.Contains("stdout did not match pattern", results[0].Message); + } + + [Fact] + public async Task PassesWhenStdErrMatchesRegex() + { + var results = await AssertionEvaluator.EvaluateAssertions( + [CmdAssertion(commandArguments: ShellArgs("echo warn_code_42 1>&2"), expectedStdErrorMatches: @"warn_code_\d+")], + "", _tmpDir); + Assert.True(results[0].Passed); + } + + [Fact] + public async Task FailsWhenStdErrDoesNotMatchRegex() + { + var results = await AssertionEvaluator.EvaluateAssertions( + [CmdAssertion(commandArguments: ShellArgs("echo some_text 1>&2"), expectedStdErrorMatches: @"error_\d+")], + "", _tmpDir); + Assert.False(results[0].Passed); + Assert.Contains("stderr did not match pattern", results[0].Message); + } + [Fact] public async Task PassesWhenAllChecksPass() { var results = await AssertionEvaluator.EvaluateAssertions( - [new Assertion(AssertionType.RunCommandAndAssert, - CommandToRun: Shell, - CommandArguments: ShellArgs("echo stdout_text && echo stderr_text 1>&2"), - ExpectedExitCode: 0, - ExpectedStdOutContains: "stdout_text", - ExpectedStdErrorContains: "stderr_text")], + [CmdAssertion( + commandArguments: ShellArgs("echo stdout_text && echo stderr_text 1>&2"), + expectedExitCode: 0, + expectedStdOutContains: "stdout_text", + expectedStdErrorContains: "stderr_text")], "", _tmpDir); Assert.True(results[0].Passed); } @@ -455,11 +490,10 @@ [new Assertion(AssertionType.RunCommandAndAssert, public async Task ExitCodeFailureShortCircuitsOtherChecks() { var results = await AssertionEvaluator.EvaluateAssertions( - [new Assertion(AssertionType.RunCommandAndAssert, - CommandToRun: Shell, - CommandArguments: ShellArgs("echo hello && exit 1"), - ExpectedExitCode: 0, - ExpectedStdOutContains: "hello")], + [CmdAssertion( + commandArguments: ShellArgs("echo hello && exit 1"), + expectedExitCode: 0, + expectedStdOutContains: "hello")], "", _tmpDir); Assert.False(results[0].Passed); Assert.Contains("exited with code 1 but expected 0", results[0].Message); @@ -469,11 +503,10 @@ [new Assertion(AssertionType.RunCommandAndAssert, public async Task StdOutFailureShortCircuitsStdErrCheck() { var results = await AssertionEvaluator.EvaluateAssertions( - [new Assertion(AssertionType.RunCommandAndAssert, - CommandToRun: Shell, - CommandArguments: ShellArgs("echo wrong && echo expected_error 1>&2"), - ExpectedStdOutContains: "expected_output", - ExpectedStdErrorContains: "expected_error")], + [CmdAssertion( + commandArguments: ShellArgs("echo wrong && echo expected_error 1>&2"), + expectedStdOutContains: "expected_output", + expectedStdErrorContains: "expected_error")], "", _tmpDir); Assert.False(results[0].Passed); Assert.Contains("stdout did not contain expected value", results[0].Message); @@ -483,10 +516,9 @@ [new Assertion(AssertionType.RunCommandAndAssert, public async Task IgnoresExitCodeWhenNotSpecified() { var results = await AssertionEvaluator.EvaluateAssertions( - [new Assertion(AssertionType.RunCommandAndAssert, - CommandToRun: Shell, - CommandArguments: ShellArgs("echo output_text && exit 1"), - ExpectedStdOutContains: "output_text")], + [CmdAssertion( + commandArguments: ShellArgs("echo output_text && exit 1"), + expectedStdOutContains: "output_text")], "", _tmpDir); Assert.True(results[0].Passed); } @@ -497,11 +529,32 @@ public async Task UsesWorkDirAsProcessWorkingDirectory() File.WriteAllText(Path.Combine(_tmpDir, "marker.txt"), "test_content"); var catCmd = OperatingSystem.IsWindows() ? "type marker.txt" : "cat marker.txt"; var results = await AssertionEvaluator.EvaluateAssertions( - [new Assertion(AssertionType.RunCommandAndAssert, - CommandToRun: Shell, - CommandArguments: ShellArgs(catCmd), - ExpectedStdOutContains: "test_content")], + [CmdAssertion(commandArguments: ShellArgs(catCmd), expectedStdOutContains: "test_content")], "", _tmpDir); Assert.True(results[0].Passed); } + + [Fact] + public async Task UsesCustomTimeoutFromAssertion() + { + // Use a very short timeout (1 second) so a long-running command times out + var sleepCmd = OperatingSystem.IsWindows() ? "ping -n 10 127.0.0.1 >nul" : "sleep 10"; + var results = await AssertionEvaluator.EvaluateAssertions( + [CmdAssertion(commandArguments: ShellArgs(sleepCmd), expectedExitCode: 0, timeout: 1)], + "", _tmpDir); + Assert.False(results[0].Passed); + Assert.Contains("Command timed out after 1s", results[0].Message); + } + + [Fact] + public async Task FallsBackToScenarioTimeout() + { + // Pass a short scenario timeout (1 second) - the command should time out + var sleepCmd = OperatingSystem.IsWindows() ? "ping -n 10 127.0.0.1 >nul" : "sleep 10"; + var results = await AssertionEvaluator.EvaluateAssertions( + [CmdAssertion(commandArguments: ShellArgs(sleepCmd), expectedExitCode: 0)], + "", _tmpDir, scenarioTimeoutSeconds: 1); + Assert.False(results[0].Passed); + Assert.Contains("Command timed out after 1s", results[0].Message); + } } From 9f34b437816d86aa1851d3fc1ba2f90642f5415c Mon Sep 17 00:00:00 2001 From: Jan Krivanek Date: Fri, 20 Mar 2026 18:32:07 +0100 Subject: [PATCH 07/10] Address code review feedback on RunCommandAndAssert - Convert EvalRunCommandAndAssert to async using WaitForExitAsync with CancellationTokenSource for timeout instead of blocking thread - Wrap Process.Start in try/catch, handle null return with clear failure message including command and args - Dispose Process with 'using', use Kill(entireProcessTree: true) on timeout to avoid leaking child processes - Truncate stdout/stderr in failure messages to 4KB to avoid log bloat - Validate timeout > 0 before use, return clear error for bad config - Fix validation error message to show correct YAML field names (expected_std_output_contains/expected_std_error_contains) - Add EvalSchema parsing tests for run_command_and_assert (valid parse, missing command_to_run, missing expected checks) - Use 'timeout /t' instead of 'ping' for Windows sleep in tests --- .../src/Services/AssertionEvaluator.cs | 151 +++++++++++------- .../src/Services/EvalSchema.cs | 2 +- eng/skill-validator/tests/AssertionsTests.cs | 4 +- eng/skill-validator/tests/EvalSchemaTests.cs | 64 ++++++++ 4 files changed, 158 insertions(+), 63 deletions(-) diff --git a/eng/skill-validator/src/Services/AssertionEvaluator.cs b/eng/skill-validator/src/Services/AssertionEvaluator.cs index 3692afb6f4..e750c92a77 100644 --- a/eng/skill-validator/src/Services/AssertionEvaluator.cs +++ b/eng/skill-validator/src/Services/AssertionEvaluator.cs @@ -101,7 +101,7 @@ private static async Task EvaluateAssertion( AssertionType.OutputMatches => EvalOutputMatches(assertion, agentOutput), AssertionType.OutputNotMatches => EvalOutputNotMatches(assertion, agentOutput), AssertionType.ExitSuccess => EvalExitSuccess(assertion, agentOutput), - AssertionType.RunCommandAndAssert => EvalRunCommandAndAssert(assertion, workDir, scenarioTimeoutSeconds), + AssertionType.RunCommandAndAssert => await EvalRunCommandAndAssert(assertion, workDir, scenarioTimeoutSeconds), _ => new AssertionResult(assertion, false, $"Unknown assertion type: {assertion.Type}"), }; } @@ -241,12 +241,26 @@ private static AssertionResult EvalExitSuccess(Assertion a, string agentOutput) : "Agent produced no output"); } - private static AssertionResult EvalRunCommandAndAssert(Assertion a, string workDir, int scenarioTimeoutSeconds) + private const int MaxOutputLength = 4096; + + private static string TruncateOutput(string output) + { + if (output.Length <= MaxOutputLength) + return output; + return $"{output[..MaxOutputLength]}... [truncated, {output.Length} chars total]"; + } + + private static async Task EvalRunCommandAndAssert(Assertion a, string workDir, int scenarioTimeoutSeconds) { var cmd = a.CommandArgs ?? throw new UnreachableException(); var command = cmd.CommandToRun; var timeoutSeconds = cmd.Timeout ?? scenarioTimeoutSeconds; + if (timeoutSeconds <= 0) + { + return new AssertionResult(a, false, $"Invalid timeout value {timeoutSeconds}s. Timeout must be greater than 0."); + } + var processStartInfo = new ProcessStartInfo(command, cmd.CommandArguments ?? string.Empty) { WorkingDirectory = workDir, @@ -255,88 +269,105 @@ private static AssertionResult EvalRunCommandAndAssert(Assertion a, string workD UseShellExecute = false, }; - var process = Process.Start(processStartInfo); - - var outBuilder = new StringBuilder(); - var errBuilder = new StringBuilder(); - process!.OutputDataReceived += (_, e) => - { - if (e.Data is not null) - outBuilder.AppendLine(e.Data); - }; - process.ErrorDataReceived += (_, e) => - { - if (e.Data is not null) - errBuilder.AppendLine(e.Data); - }; - process.BeginOutputReadLine(); - process.BeginErrorReadLine(); - - if (!process.WaitForExit(TimeSpan.FromSeconds(timeoutSeconds))) + Process process; + try { - process.Kill(); - return new AssertionResult(a, false, $"Command timed out after {timeoutSeconds}s"); + var started = Process.Start(processStartInfo); + if (started is null) + { + return new AssertionResult(a, false, $"Failed to start process '{command}' {cmd.CommandArguments}"); + } + process = started; } - - // The parameterless WaitForExit() ensures that all async output event handlers - // have completed processing. Without this, stdout/stderr may not be fully captured. - process.WaitForExit(); - - var actualExitCode = process.ExitCode; - if (cmd.ExpectedExitCode.HasValue && cmd.ExpectedExitCode.Value != actualExitCode) + catch (Exception ex) { - return new AssertionResult(a, false, $"Command exited with code {actualExitCode} but expected {cmd.ExpectedExitCode.Value}"); + return new AssertionResult(a, false, $"Failed to start process '{command}' {cmd.CommandArguments}: {ex.Message}"); } - var actualStdOut = outBuilder.ToString(); - if (cmd.ExpectedStdOutContains is not null) + using (process) { - if (!actualStdOut.Contains(cmd.ExpectedStdOutContains, StringComparison.Ordinal)) + var outBuilder = new StringBuilder(); + var errBuilder = new StringBuilder(); + process.OutputDataReceived += (_, e) => { - return new AssertionResult(a, false, $"Command stdout did not contain expected value. Stdout: {actualStdOut}"); - } - } + if (e.Data is not null) + outBuilder.AppendLine(e.Data); + }; + process.ErrorDataReceived += (_, e) => + { + if (e.Data is not null) + errBuilder.AppendLine(e.Data); + }; + process.BeginOutputReadLine(); + process.BeginErrorReadLine(); - if (cmd.ExpectedStdOutMatches is not null) - { + using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(timeoutSeconds)); try { - if (!Regex.IsMatch(actualStdOut, cmd.ExpectedStdOutMatches, RegexOptions.IgnoreCase, RegexTimeout)) - { - return new AssertionResult(a, false, $"Command stdout did not match pattern '{cmd.ExpectedStdOutMatches}'. Stdout: {actualStdOut}"); - } + await process.WaitForExitAsync(cts.Token); } - catch (RegexMatchTimeoutException) + catch (OperationCanceledException) { - return new AssertionResult(a, false, $"Regex pattern '{cmd.ExpectedStdOutMatches}' timed out after {RegexTimeout.TotalSeconds}s"); + try { process.Kill(entireProcessTree: true); } catch { } + return new AssertionResult(a, false, $"Command timed out after {timeoutSeconds}s"); } - } - var actualStdErr = errBuilder.ToString(); - if (cmd.ExpectedStdErrorContains is not null) - { - if (!actualStdErr.Contains(cmd.ExpectedStdErrorContains, StringComparison.Ordinal)) + var actualExitCode = process.ExitCode; + if (cmd.ExpectedExitCode.HasValue && cmd.ExpectedExitCode.Value != actualExitCode) { - return new AssertionResult(a, false, $"Command stderr did not contain expected value. Stderr: {actualStdErr}"); + return new AssertionResult(a, false, $"Command exited with code {actualExitCode} but expected {cmd.ExpectedExitCode.Value}"); } - } - if (cmd.ExpectedStdErrorMatches is not null) - { - try + var actualStdOut = outBuilder.ToString(); + if (cmd.ExpectedStdOutContains is not null) { - if (!Regex.IsMatch(actualStdErr, cmd.ExpectedStdErrorMatches, RegexOptions.IgnoreCase, RegexTimeout)) + if (!actualStdOut.Contains(cmd.ExpectedStdOutContains, StringComparison.Ordinal)) { - return new AssertionResult(a, false, $"Command stderr did not match pattern '{cmd.ExpectedStdErrorMatches}'. Stderr: {actualStdErr}"); + return new AssertionResult(a, false, $"Command stdout did not contain expected value. Stdout: {TruncateOutput(actualStdOut)}"); } } - catch (RegexMatchTimeoutException) + + if (cmd.ExpectedStdOutMatches is not null) { - return new AssertionResult(a, false, $"Regex pattern '{cmd.ExpectedStdErrorMatches}' timed out after {RegexTimeout.TotalSeconds}s"); + try + { + if (!Regex.IsMatch(actualStdOut, cmd.ExpectedStdOutMatches, RegexOptions.IgnoreCase, RegexTimeout)) + { + return new AssertionResult(a, false, $"Command stdout did not match pattern '{cmd.ExpectedStdOutMatches}'. Stdout: {TruncateOutput(actualStdOut)}"); + } + } + catch (RegexMatchTimeoutException) + { + return new AssertionResult(a, false, $"Regex pattern '{cmd.ExpectedStdOutMatches}' timed out after {RegexTimeout.TotalSeconds}s"); + } + } + + var actualStdErr = errBuilder.ToString(); + if (cmd.ExpectedStdErrorContains is not null) + { + if (!actualStdErr.Contains(cmd.ExpectedStdErrorContains, StringComparison.Ordinal)) + { + return new AssertionResult(a, false, $"Command stderr did not contain expected value. Stderr: {TruncateOutput(actualStdErr)}"); + } + } + + if (cmd.ExpectedStdErrorMatches is not null) + { + try + { + if (!Regex.IsMatch(actualStdErr, cmd.ExpectedStdErrorMatches, RegexOptions.IgnoreCase, RegexTimeout)) + { + return new AssertionResult(a, false, $"Command stderr did not match pattern '{cmd.ExpectedStdErrorMatches}'. Stderr: {TruncateOutput(actualStdErr)}"); + } + } + catch (RegexMatchTimeoutException) + { + return new AssertionResult(a, false, $"Regex pattern '{cmd.ExpectedStdErrorMatches}' timed out after {RegexTimeout.TotalSeconds}s"); + } } - } - return new AssertionResult(a, true, string.Empty); + return new AssertionResult(a, true, string.Empty); + } } private static Task FileExistsGlob(string pattern, string workDir) diff --git a/eng/skill-validator/src/Services/EvalSchema.cs b/eng/skill-validator/src/Services/EvalSchema.cs index 3db13279a0..23d4d77d7a 100644 --- a/eng/skill-validator/src/Services/EvalSchema.cs +++ b/eng/skill-validator/src/Services/EvalSchema.cs @@ -118,7 +118,7 @@ private static Assertion ParseAssertion(RawAssertion raw) string.IsNullOrWhiteSpace(raw.ExpectedStdErrorContains) && string.IsNullOrWhiteSpace(raw.ExpectedStdOutputMatches) && string.IsNullOrWhiteSpace(raw.ExpectedStdErrorMatches)) - throw new InvalidOperationException($"Assertion '{raw.Type}' requires one or more of 'expected_exit_code', 'expected_std_output', 'expected_std_error', 'expected_std_output_matches', or 'expected_std_error_matches'"); + throw new InvalidOperationException($"Assertion '{raw.Type}' requires one or more of 'expected_exit_code', 'expected_std_output_contains', 'expected_std_error_contains', 'expected_std_output_matches', or 'expected_std_error_matches'"); break; } diff --git a/eng/skill-validator/tests/AssertionsTests.cs b/eng/skill-validator/tests/AssertionsTests.cs index 4e4e5eefdb..758e0666ee 100644 --- a/eng/skill-validator/tests/AssertionsTests.cs +++ b/eng/skill-validator/tests/AssertionsTests.cs @@ -538,7 +538,7 @@ public async Task UsesWorkDirAsProcessWorkingDirectory() public async Task UsesCustomTimeoutFromAssertion() { // Use a very short timeout (1 second) so a long-running command times out - var sleepCmd = OperatingSystem.IsWindows() ? "ping -n 10 127.0.0.1 >nul" : "sleep 10"; + var sleepCmd = OperatingSystem.IsWindows() ? "timeout /t 10 /nobreak >nul" : "sleep 10"; var results = await AssertionEvaluator.EvaluateAssertions( [CmdAssertion(commandArguments: ShellArgs(sleepCmd), expectedExitCode: 0, timeout: 1)], "", _tmpDir); @@ -550,7 +550,7 @@ public async Task UsesCustomTimeoutFromAssertion() public async Task FallsBackToScenarioTimeout() { // Pass a short scenario timeout (1 second) - the command should time out - var sleepCmd = OperatingSystem.IsWindows() ? "ping -n 10 127.0.0.1 >nul" : "sleep 10"; + var sleepCmd = OperatingSystem.IsWindows() ? "timeout /t 10 /nobreak >nul" : "sleep 10"; var results = await AssertionEvaluator.EvaluateAssertions( [CmdAssertion(commandArguments: ShellArgs(sleepCmd), expectedExitCode: 0)], "", _tmpDir, scenarioTimeoutSeconds: 1); diff --git a/eng/skill-validator/tests/EvalSchemaTests.cs b/eng/skill-validator/tests/EvalSchemaTests.cs index 2b4eaeade4..d3d22d2847 100644 --- a/eng/skill-validator/tests/EvalSchemaTests.cs +++ b/eng/skill-validator/tests/EvalSchemaTests.cs @@ -177,6 +177,70 @@ public void ParsesPartialConfigSection() Assert.Equal(2, config.MaxParallelScenarios); Assert.Null(config.MaxParallelRuns); } + + [Fact] + public void ParsesRunCommandAndAssertAssertion() + { + var yaml = """ + scenarios: + - name: "Build check" + prompt: "Build the project" + assertions: + - type: "run_command_and_assert" + command_to_run: "dotnet" + command_arguments: "build" + expected_exit_code: 0 + expected_std_output_contains: "Build succeeded" + expected_std_error_contains: "warning" + expected_std_output_matches: "Build \\w+" + expected_std_error_matches: "warn.*" + command_timeout: 60 + """; + var config = EvalSchema.ParseEvalConfig(yaml); + var assertion = config.Scenarios[0].Assertions![0]; + Assert.Equal(AssertionType.RunCommandAndAssert, assertion.Type); + Assert.NotNull(assertion.CommandArgs); + var cmd = assertion.CommandArgs!; + Assert.Equal("dotnet", cmd.CommandToRun); + Assert.Equal("build", cmd.CommandArguments); + Assert.Equal(0, cmd.ExpectedExitCode); + Assert.Equal("Build succeeded", cmd.ExpectedStdOutContains); + Assert.Equal("warning", cmd.ExpectedStdErrorContains); + Assert.Equal("Build \\w+", cmd.ExpectedStdOutMatches); + Assert.Equal("warn.*", cmd.ExpectedStdErrorMatches); + Assert.Equal(60, cmd.Timeout); + } + + [Fact] + public void RejectsRunCommandAndAssertWithoutCommandToRun() + { + var yaml = """ + scenarios: + - name: "Test" + prompt: "Do it" + assertions: + - type: "run_command_and_assert" + expected_exit_code: 0 + """; + var ex = Assert.Throws(() => EvalSchema.ParseEvalConfig(yaml)); + Assert.Contains("command_to_run", ex.Message); + } + + [Fact] + public void RejectsRunCommandAndAssertWithoutAnyExpectedChecks() + { + var yaml = """ + scenarios: + - name: "Test" + prompt: "Do it" + assertions: + - type: "run_command_and_assert" + command_to_run: "dotnet" + """; + var ex = Assert.Throws(() => EvalSchema.ParseEvalConfig(yaml)); + Assert.Contains("expected_exit_code", ex.Message); + Assert.Contains("expected_std_output_contains", ex.Message); + } } public class ValidateEvalConfigTests From dd473d7f3bb4f51cb58dc9cac9d4b2729291625f Mon Sep 17 00:00:00 2001 From: Jan Krivanek Date: Fri, 20 Mar 2026 19:16:16 +0100 Subject: [PATCH 08/10] Normalize empty strings to null in CommandAssertionArgs Treat null/whitespace expected_std_* values as 'not specified' when building CommandAssertionArgs, preventing empty strings from becoming accidental always-pass assertions (Contains('') and IsMatch('') always succeed). --- eng/skill-validator/src/Services/EvalSchema.cs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/eng/skill-validator/src/Services/EvalSchema.cs b/eng/skill-validator/src/Services/EvalSchema.cs index 23d4d77d7a..81e3f5d4a9 100644 --- a/eng/skill-validator/src/Services/EvalSchema.cs +++ b/eng/skill-validator/src/Services/EvalSchema.cs @@ -125,18 +125,21 @@ private static Assertion ParseAssertion(RawAssertion raw) CommandAssertionArgs? commandArgs = type == AssertionType.RunCommandAndAssert ? new CommandAssertionArgs( raw.CommandToRun!, - raw.CommandArguments, + NullIfWhiteSpace(raw.CommandArguments), raw.ExpectedExitCode, - raw.ExpectedStdOutputContains, - raw.ExpectedStdErrorContains, - raw.ExpectedStdOutputMatches, - raw.ExpectedStdErrorMatches, + NullIfWhiteSpace(raw.ExpectedStdOutputContains), + NullIfWhiteSpace(raw.ExpectedStdErrorContains), + NullIfWhiteSpace(raw.ExpectedStdOutputMatches), + NullIfWhiteSpace(raw.ExpectedStdErrorMatches), raw.CommandTimeout) : null; return new Assertion(type, raw.Path, raw.Value, raw.Pattern, commandArgs); } + private static string? NullIfWhiteSpace(string? value) => + string.IsNullOrWhiteSpace(value) ? null : value; + // Raw YAML deserialization models internal sealed class RawFrontmatter From 27a531769e0d0357b97353d58ae1e992a9a76167 Mon Sep 17 00:00:00 2001 From: Jan Krivanek Date: Fri, 20 Mar 2026 19:19:30 +0100 Subject: [PATCH 09/10] Fix test --- eng/skill-validator/tests/AssertionsTests.cs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/eng/skill-validator/tests/AssertionsTests.cs b/eng/skill-validator/tests/AssertionsTests.cs index 758e0666ee..ab7686a725 100644 --- a/eng/skill-validator/tests/AssertionsTests.cs +++ b/eng/skill-validator/tests/AssertionsTests.cs @@ -537,8 +537,10 @@ public async Task UsesWorkDirAsProcessWorkingDirectory() [Fact] public async Task UsesCustomTimeoutFromAssertion() { - // Use a very short timeout (1 second) so a long-running command times out - var sleepCmd = OperatingSystem.IsWindows() ? "timeout /t 10 /nobreak >nul" : "sleep 10"; + // Use a very short timeout (1 second) so a long-running command times out. + // Note: 'timeout' is not reliable in non-interactive contexts (exits immediately on CI), + // so we use 'ping -n 11 127.0.0.1' which sleeps ~10s reliably everywhere. + var sleepCmd = OperatingSystem.IsWindows() ? "ping -n 11 127.0.0.1 >nul" : "sleep 10"; var results = await AssertionEvaluator.EvaluateAssertions( [CmdAssertion(commandArguments: ShellArgs(sleepCmd), expectedExitCode: 0, timeout: 1)], "", _tmpDir); @@ -549,8 +551,10 @@ public async Task UsesCustomTimeoutFromAssertion() [Fact] public async Task FallsBackToScenarioTimeout() { - // Pass a short scenario timeout (1 second) - the command should time out - var sleepCmd = OperatingSystem.IsWindows() ? "timeout /t 10 /nobreak >nul" : "sleep 10"; + // Pass a short scenario timeout (1 second) - the command should time out. + // Note: 'timeout' is not reliable in non-interactive contexts (exits immediately on CI), + // so we use 'ping -n 11 127.0.0.1' which sleeps ~10s reliably everywhere. + var sleepCmd = OperatingSystem.IsWindows() ? "ping -n 11 127.0.0.1 >nul" : "sleep 10"; var results = await AssertionEvaluator.EvaluateAssertions( [CmdAssertion(commandArguments: ShellArgs(sleepCmd), expectedExitCode: 0)], "", _tmpDir, scenarioTimeoutSeconds: 1); From 74333b30614f98e3fb6fecd711af46dc5fa7f3fd Mon Sep 17 00:00:00 2001 From: Jan Krivanek Date: Fri, 20 Mar 2026 20:35:06 +0100 Subject: [PATCH 10/10] Address review: env scrubbing, stream draining, case-insensitive contains - Call AgentRunner.ScrubSensitiveEnvironment before Process.Start to prevent leaking CI/credential env vars to child processes - Replace BeginOutputReadLine/BeginErrorReadLine with ReadToEndAsync to reliably drain stdout/stderr before evaluating assertions - Change StringComparison.Ordinal to OrdinalIgnoreCase for contains checks, consistent with output_contains and regex assertions --- .../src/Services/AssertionEvaluator.cs | 28 ++++++------------- 1 file changed, 9 insertions(+), 19 deletions(-) diff --git a/eng/skill-validator/src/Services/AssertionEvaluator.cs b/eng/skill-validator/src/Services/AssertionEvaluator.cs index e750c92a77..b6f569ed05 100644 --- a/eng/skill-validator/src/Services/AssertionEvaluator.cs +++ b/eng/skill-validator/src/Services/AssertionEvaluator.cs @@ -1,5 +1,4 @@ using System.Diagnostics; -using System.Text; using System.Text.RegularExpressions; using Microsoft.Extensions.FileSystemGlobbing; using Microsoft.Extensions.FileSystemGlobbing.Abstractions; @@ -269,6 +268,8 @@ private static async Task EvalRunCommandAndAssert(Assertion a, UseShellExecute = false, }; + AgentRunner.ScrubSensitiveEnvironment(processStartInfo); + Process process; try { @@ -286,20 +287,8 @@ private static async Task EvalRunCommandAndAssert(Assertion a, using (process) { - var outBuilder = new StringBuilder(); - var errBuilder = new StringBuilder(); - process.OutputDataReceived += (_, e) => - { - if (e.Data is not null) - outBuilder.AppendLine(e.Data); - }; - process.ErrorDataReceived += (_, e) => - { - if (e.Data is not null) - errBuilder.AppendLine(e.Data); - }; - process.BeginOutputReadLine(); - process.BeginErrorReadLine(); + var stdOutTask = process.StandardOutput.ReadToEndAsync(); + var stdErrTask = process.StandardError.ReadToEndAsync(); using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(timeoutSeconds)); try @@ -312,16 +301,18 @@ private static async Task EvalRunCommandAndAssert(Assertion a, return new AssertionResult(a, false, $"Command timed out after {timeoutSeconds}s"); } + var actualStdOut = await stdOutTask; + var actualStdErr = await stdErrTask; + var actualExitCode = process.ExitCode; if (cmd.ExpectedExitCode.HasValue && cmd.ExpectedExitCode.Value != actualExitCode) { return new AssertionResult(a, false, $"Command exited with code {actualExitCode} but expected {cmd.ExpectedExitCode.Value}"); } - var actualStdOut = outBuilder.ToString(); if (cmd.ExpectedStdOutContains is not null) { - if (!actualStdOut.Contains(cmd.ExpectedStdOutContains, StringComparison.Ordinal)) + if (!actualStdOut.Contains(cmd.ExpectedStdOutContains, StringComparison.OrdinalIgnoreCase)) { return new AssertionResult(a, false, $"Command stdout did not contain expected value. Stdout: {TruncateOutput(actualStdOut)}"); } @@ -342,10 +333,9 @@ private static async Task EvalRunCommandAndAssert(Assertion a, } } - var actualStdErr = errBuilder.ToString(); if (cmd.ExpectedStdErrorContains is not null) { - if (!actualStdErr.Contains(cmd.ExpectedStdErrorContains, StringComparison.Ordinal)) + if (!actualStdErr.Contains(cmd.ExpectedStdErrorContains, StringComparison.OrdinalIgnoreCase)) { return new AssertionResult(a, false, $"Command stderr did not contain expected value. Stderr: {TruncateOutput(actualStdErr)}"); }