diff --git a/eng/common/pipelines/live-eval.yml b/eng/common/pipelines/live-eval.yml index e18bafa65f..b8c54882dd 100644 --- a/eng/common/pipelines/live-eval.yml +++ b/eng/common/pipelines/live-eval.yml @@ -22,6 +22,7 @@ parameters: variables: # Managed-pool image selection (LINUXPOOL/LINUXVMIMAGE). Repo-local. - template: /eng/pipelines/templates/variables/image.yml + - template: /eng/pipelines/templates/variables/globals.yml extends: template: /eng/common/pipelines/templates/stages/archetype-eval.yml @@ -37,6 +38,7 @@ extends: - 'workflows/live/*.eval.yaml' # Run each shard under AzureCLI@2 so the real MCP's DevOps calls are authenticated. UseAzSdkAuthentication: true + azureSubscription: ${{ variables.EvalAzureSubscription }} failOnFailedTests: true # Live scenarios are end-to-end, so give each shard more headroom than the report-only tiers. shardTimeoutInMinutes: 45 diff --git a/eng/common/pipelines/templates/jobs/eval-shard.yml b/eng/common/pipelines/templates/jobs/eval-shard.yml index 569f7d4fdf..fc202be4b7 100644 --- a/eng/common/pipelines/templates/jobs/eval-shard.yml +++ b/eng/common/pipelines/templates/jobs/eval-shard.yml @@ -10,10 +10,22 @@ parameters: # Swap point: runs `vally eval` for the shard (the common template is auth-conditional). type: string default: /eng/common/pipelines/templates/steps/eval-invoke.yml + - name: vallyArguments + # --tag does not filter the supplied matrix; each planned eval must retain matching stimuli. + type: object + default: [] + - name: failOnFailedTests + # Match Summary's gate; multi-run gating is not currently supported. + type: boolean + default: false - name: UseAzSdkAuthentication # Forwarded to the invoke template: true runs the shard under AzureCLI@2. type: boolean default: false + - name: azureSubscription + # Leave blank to preserve the invoke template's default connection. + type: string + default: '' - name: threshold # Pass-rate gate forwarded to the invoke template (`vally eval --threshold` + shard verdict). type: number @@ -71,6 +83,14 @@ jobs: inputs: version: 24.x + - ${{ if eq(parameters.evalInvokeTemplate, '/eng/common/pipelines/templates/steps/eval-invoke.yml') }}: + - script: | + node --experimental-strip-types "$(Build.SourcesDirectory)/eng/common/scripts/eval/invoke-eval-shard.ts" --validate-only + displayName: 'Validate eval run configuration' + env: + VALLY_ARGUMENTS_JSON: ${{ replace(convertToJson(parameters.vallyArguments), '$', '\u0024') }} + EVAL_FAIL_ON_FAILED_TESTS: ${{ parameters.failOnFailedTests }} + # 1ES Official image blocks registry.npmjs.org; restore via the azure-sdk npm mirror. - template: /eng/common/pipelines/templates/steps/create-authenticated-npmrc.yml parameters: @@ -113,12 +133,17 @@ jobs: - template: /eng/common/pipelines/templates/steps/login-to-copilot.yml - # Runs the eval via the invoke template (auth-conditional in the common default). - template: ${{ parameters.evalInvokeTemplate }} parameters: vallyRoot: ${{ parameters.vallyRoot }} UseAzSdkAuthentication: ${{ parameters.UseAzSdkAuthentication }} + # Custom invoke templates receive this only when explicitly configured. + ${{ if ne(parameters.azureSubscription, '') }}: + azureSubscription: ${{ parameters.azureSubscription }} threshold: ${{ parameters.threshold }} + # Existing custom templates keep their contract unless opting into the argument list. + ${{ if ne(length(parameters.vallyArguments), 0) }}: + vallyArguments: ${{ parameters.vallyArguments }} # Optional repo-specific teardown after the eval. - ${{ if parameters.postEvalTemplate }}: diff --git a/eng/common/pipelines/templates/jobs/eval-summarize.yml b/eng/common/pipelines/templates/jobs/eval-summarize.yml index eddc71c6e9..46241aed8b 100644 --- a/eng/common/pipelines/templates/jobs/eval-summarize.yml +++ b/eng/common/pipelines/templates/jobs/eval-summarize.yml @@ -3,6 +3,7 @@ parameters: - name: failOnFailedTests + # Only single-run evaluations support this gate; multi-run observations must leave it off. type: boolean default: false - name: createDashboardBundle @@ -40,7 +41,6 @@ jobs: inputs: version: 24.x - # Allowlist result data only, excluding debug, MCP, SDL and signing artifacts. - task: DownloadPipelineArtifact@2 displayName: 'Download all shard results' inputs: diff --git a/eng/common/pipelines/templates/stages/archetype-eval.yml b/eng/common/pipelines/templates/stages/archetype-eval.yml index 128cc16c70..7c2d570aa7 100644 --- a/eng/common/pipelines/templates/stages/archetype-eval.yml +++ b/eng/common/pipelines/templates/stages/archetype-eval.yml @@ -43,6 +43,14 @@ parameters: # When true, the invoke step runs under AzureCLI@2 (live MCP inherits the service connection). type: boolean default: false + - name: azureSubscription + # Optional override; blank preserves the invoke template's default connection. + type: string + default: '' + - name: vallyArguments + # --tag does not filter Prepare's matrix; align evalGlobs so every eval has matching stimuli. + type: object + default: [] - name: threshold # Pass-rate gate forwarded to `vally eval --threshold` and the shard verdict. Repos may override. type: number @@ -102,6 +110,9 @@ extends: vallyRoot: ${{ parameters.vallyRoot }} evalInvokeTemplate: ${{ parameters.evalInvokeTemplate }} UseAzSdkAuthentication: ${{ parameters.UseAzSdkAuthentication }} + azureSubscription: ${{ parameters.azureSubscription }} + vallyArguments: ${{ parameters.vallyArguments }} + failOnFailedTests: ${{ parameters.failOnFailedTests }} threshold: ${{ parameters.threshold }} preEvalTemplate: ${{ parameters.preEvalTemplate }} postEvalTemplate: ${{ parameters.postEvalTemplate }} diff --git a/eng/common/pipelines/templates/steps/eval-invoke.yml b/eng/common/pipelines/templates/steps/eval-invoke.yml index 191765af3f..3e0c354e70 100644 --- a/eng/common/pipelines/templates/steps/eval-invoke.yml +++ b/eng/common/pipelines/templates/steps/eval-invoke.yml @@ -2,6 +2,7 @@ # node script directly. UseAzSdkAuthentication=true wraps the same run in AzureCLI@2 so a live # MCP's Azure DevOps calls inherit the service-connection identity. # The shard's login-to-copilot template supplies the executor and prompt-judge token. +# Callers may override the existing service connection through azureSubscription. parameters: - name: vallyRoot @@ -16,9 +17,21 @@ parameters: - name: azureSubscription type: string default: opensource-api-connection + - name: vallyArguments + # Supplementary non-reserved --option value pairs and --switch flags, not shell text. + # --tag does not filter the matrix; callers must keep every planned eval non-empty. + # JSON-escape dollar signs below so agent macros cannot rewrite argument data. + type: object + default: [] steps: - - ${{ if parameters.UseAzSdkAuthentication }}: + - ${{ if and(parameters.UseAzSdkAuthentication, eq(trim(parameters.azureSubscription), '')) }}: + - script: | + echo "##vso[task.logissue type=error]azureSubscription must be provided when UseAzSdkAuthentication is true." + exit 1 + displayName: 'Validate authenticated eval service connection' + + - ${{ elseif parameters.UseAzSdkAuthentication }}: - task: AzureCLI@2 displayName: 'vally eval $(shardName) (authenticated)' inputs: @@ -34,6 +47,7 @@ steps: --threshold ${{ parameters.threshold }} env: COPILOT_GITHUB_TOKEN: $(COPILOT_GITHUB_TOKEN) + VALLY_ARGUMENTS_JSON: ${{ replace(convertToJson(parameters.vallyArguments), '$', '\u0024') }} - ${{ else }}: - script: | @@ -46,3 +60,4 @@ steps: workingDirectory: ${{ parameters.vallyRoot }} env: COPILOT_GITHUB_TOKEN: $(COPILOT_GITHUB_TOKEN) + VALLY_ARGUMENTS_JSON: ${{ replace(convertToJson(parameters.vallyArguments), '$', '\u0024') }} diff --git a/eng/common/scripts/eval/README.md b/eng/common/scripts/eval/README.md index cccbca4809..0e44d2a64e 100644 --- a/eng/common/scripts/eval/README.md +++ b/eng/common/scripts/eval/README.md @@ -13,6 +13,91 @@ templates. its own pnpm package/lock for **fflate only** and CLI/PowerShell tests. Shard evaluator restores do not install it; publication adds no Node Azure SDK dependencies. +## Extending a shard job + +The shared `eng/common/pipelines/templates/jobs/eval-shard.yml` owns checkout, +Node/Vally installation, MCP staging, git fixture initialization and Copilot login. +Its defaults preserve the standard Prepare/Eval/Summary pipeline behavior. +Repo-local adapters can customize it without copying that infrastructure: + +| Parameter | Contract | +| --- | --- | +| `preEvalTemplate` / `postEvalTemplate` | Existing parameter-free setup/teardown step templates. | +| `evalInvokeTemplate` | Existing invocation template accepting `vallyRoot`, `UseAzSdkAuthentication` and `threshold`. | +| `azureSubscription` | Optional service-connection override. Stage/job templates default to `''` and omit an empty override; the common invoke template retains its `opensource-api-connection` default. | +| `vallyArguments` | Optional array of supplementary Vally options, values and switches. Strings are required except that trial/worker counts also accept numbers. Available through the stage, shard job and common invoke template. Defaults to `[]`. | + +**Not allowed in `vallyArguments`:** + +| Affected part | Reserved options | Reason | +| --- | --- | --- | +| Shard structure and execution context | `--eval-spec` / `-e`, `--no-eval-spec`, `--suite`, `--no-suite`, `--eval-plugin`, `--no-eval-plugin`, `--work-dir`, `--no-work-dir` | Prepare already assigns files to shards. These change eval selection/loading or the working directory; `--suite` also conflicts with the assigned `-e` arguments. Use `evalGlobs` and `vallyRoot` instead. | +| Result collection and Summary | `--output-dir`, `--no-output-dir`, `--output`, `--no-output`, `--junit`, `--no-junit` | The pipeline owns result locations and enables JUnit. In Vally 0.19.0, `--output jsonl` sends results to stdout instead of `results.jsonl`. | +| Validation and scoring | `--threshold`, `--no-threshold`, `--skip-grade`, `--no-skip-grade`, `--skip-validate`, `--no-skip-validate` | The runner does not support bypassing validation/grading, and scoring must match the shard gate. Set the template's `threshold` parameter instead. | +| Exit-code gating | `--require-pass`, `--no-require-pass` | The shard decides pass/fail from `results.jsonl`, not Vally's exit code; this option would not control the pipeline gate. | + +**Allowed:** other Vally options using `--option value` or standalone switches such as +`--verbose`. This includes `--tag`, `--runs`, `--workers`, `--timeout`, +`--model` and supplementary plugin options. New options need no common-template changes; +Vally validates their names, values and whether they require a value. +Aliases, `--flag=value` and positional paths/globs are not supported. +Values must be nonempty strings without null characters and must not start with `-`; +`--runs` and `--workers` also accept positive safe integer numbers. +Arguments are passed literally, without a shell or whitespace splitting. + +For example, replace `` and `` with a tag defined in your eval stimuli: + +```yaml +vallyArguments: + - --tag + - '=' + - --runs + - 3 + - --workers + - 2 + - --verbose +failOnFailedTests: false +``` + +Multiple runs require `failOnFailedTests: false`, including repetition configured +outside `vallyArguments`. `--tag` uses keys and values defined in your eval stimuli, +not predefined mode names. Tags filter stimuli, not the shard matrix: keep +`evalGlobs` aligned so every selected eval has matching stimuli. +Custom invoke templates must declare `vallyArguments` to receive a non-empty array. + +Use the existing pre/post-eval and MCP setup hooks for repository-specific work. +Authenticated callers can pass `azureSubscription` explicitly to the stage or shard +template. Nonempty overrides are forwarded to the common invoke step; otherwise +the invoke template retains its existing `opensource-api-connection` default. +An explicitly empty or whitespace-only value passed directly to the invoke template fails with +a configuration error without creating an `AzureCLI@2` task. Unauthenticated runs +do not need the parameter. Custom invoke templates receive it only when nonempty +and must declare their own `azureSubscription` parameter to opt in. +Configure an authorized connection in repo-owned YAML and explicitly pass it from +the entrypoint to the stage template. Do not rely on an +ambient variable being visible across nested templates, or on task/output variables +that are unavailable during template expansion. +The `eng/common/pipelines/live-eval.yml` entrypoint imports the repo-owned +`eng/pipelines/templates/variables/globals.yml` and passes its `EvalAzureSubscription` +value as `azureSubscription`. Nested stage, job and invoke templates use only the +parameter. Scheduled runs use the same YAML configuration without a connection run +parameter. Other repos must configure their own authorized connection in that +repo-owned variable template. +Do not replace authentication, result collection or retry/completeness checks. +Supplementary reports must not rewrite JUnit thresholds or raw results. + +The shared job templates use the `Prepare` matrix and `Eval` timeline stage, with +one build-wide Summary depending on both stages. Result/debug artifacts are named +`eval-result--` / `eval-debug--`; the summary artifact +is `eval-summary-`. Summary downloads `eval-result-*` result data and selects +only the latest verified attempt for each expected shard. +Shard names must start with an ASCII letter, digit or underscore, contain only +ASCII letters, digits, underscores or hyphens, and be at most 200 characters. + +Benchmarks select one mode per build through repo-owned pipeline parameters, +aligned `evalGlobs` and `vallyArguments`. Run separate builds to compare modes; +multiple independently published tracks within one build are not supported. + ## Complete build results and direct Blob publishing Shards now retain the newest invocation's raw JSONL, JUnit and a completion marker @@ -50,11 +135,11 @@ explicit bootstrap, without changing dependency checks or the frozen lock. | Control | Current use | | --- | --- | -| `publishDashboardResults` | The only checkbox; defaults to `true` in the three tools entrypoints. Set `false` to disable both upload and Azure Storage egress | +| `publishDashboardResults` | The only checkbox; defaults to `true` in the three standard tools entrypoints. Set `false` to disable both upload and Azure Storage egress | | `EvalStorageAccountName` | Pipeline-owner configuration. Upload is skipped when absent; set it to `evaltestsummary` for this dashboard | | `EvalDashboardRefreshUrl` | Pipeline-owner configuration: `https://azsdk-eval-bue6a7dwanatgpb3.westus3-01.azurewebsites.net/api/refresh` | | `EvalDashboardResource` | Pipeline-owner configuration: `api://258998df-81ec-460c-bdd7-56a9bdde1e48` (resource URI, not a `/.default` scope) | -| `createDashboardBundle` | Shared-template artifact-only mode; enabled for all three tools entrypoints | +| `createDashboardBundle` | Shared-template artifact-only mode; enabled for the three standard tools entrypoints | The pipeline owner must set **`EvalStorageAccountName=evaltestsummary`** and the two dashboard variables above in each onboarded DevOps pipeline, with **queue-time @@ -155,6 +240,9 @@ then `node --experimental-strip-types --test` from this directory. These shared Node tests do not need Tools-owned CI files, PowerShell tests or the consuming repository's redirect settings. A consumer-copy regression runs them without Tools files or Git history, with absent and different redirect templates. +Template contract checks use bounded text assertions, not a YAML parser, and need +no evaluator dependencies. They do not evaluate Azure template expressions; +server-side expansion and service-connection authorization require separate validation. In Tools, run `Invoke-Pester -Path eng/common-tests/eval/Publish-EvalResults.Tests.ps1 -CI` from the repository root for the **PowerShell 7.4+ / Pester 5 / Azure CLI** tests. That Tools-only diff --git a/eng/common/scripts/eval/build-eval-summary.ts b/eng/common/scripts/eval/build-eval-summary.ts index 43f7ef682b..27758d34bf 100644 --- a/eng/common/scripts/eval/build-eval-summary.ts +++ b/eng/common/scripts/eval/build-eval-summary.ts @@ -413,7 +413,9 @@ async function main(argv) { if (process.env.TF_BUILD) { console.log(`##vso[task.uploadsummary]${path.resolve(options.outputPath)}`); - if (selection) console.log(`##vso[task.setvariable variable=EvalSummaryComplete]${selection.complete}`); + } + if (process.env.TF_BUILD && selection) { + console.log(`##vso[task.setvariable variable=EvalSummaryComplete]${selection.complete}`); } // Completed evaluation failures remain governed by failOnFailedTests. Missing result diff --git a/eng/common/scripts/eval/invoke-eval-shard.ts b/eng/common/scripts/eval/invoke-eval-shard.ts index 135a71d5a9..4d86403016 100644 --- a/eng/common/scripts/eval/invoke-eval-shard.ts +++ b/eng/common/scripts/eval/invoke-eval-shard.ts @@ -4,11 +4,64 @@ import { spawnSync } from "node:child_process"; import path from "node:path"; import { fileURLToPath, pathToFileURL } from "node:url"; -import { getVallyShardVerdict } from "./lib/verdict.ts"; +import { assertSupportedEvalGate, getVallyShardVerdict, parseFailedTestsGate } from "./lib/verdict.ts"; -// The pinned Vally CLI is installed next to this script (package.json + node_modules live here), -// so the npm --prefix is just this file's own directory. +// Invoke the pinned CLI with Node directly so arguments never pass through a shell. const SCRIPT_DIR = path.dirname(fileURLToPath(import.meta.url)); +const VALLY_CLI = path.join(SCRIPT_DIR, "node_modules", "@microsoft", "vally-cli", "dist", "index.js"); + +export function validateVallyArguments(value, { failOnFailedTests = false } = {}) { + if (!Array.isArray(value) || value.some((arg) => + (typeof arg !== "string" && typeof arg !== "number") || + (typeof arg === "string" && arg.includes("\0")))) { + throw new Error("Vally arguments must be an array of strings or numeric count values without null characters."); + } + // These options would change matrix selection, result collection or the verdict contract. + const reserved = new Set([ + "--eval-spec", "--suite", "--work-dir", "--eval-plugin", + "--output-dir", "--output", "--junit", "--threshold", + "--require-pass", "--skip-grade", "--skip-validate", + ]); + const args = []; + for (let index = 0; index < value.length; index++) { + const option = value[index]; + if (typeof option !== "string" || !/^--[a-z][a-z0-9]*(?:-[a-z0-9]+)*$/.test(option)) { + throw new Error(`Unsupported supplementary Vally option format: ${option}. Use --option value or --switch, not aliases or --option=value.`); + } + if (reserved.has(option) || reserved.has(option.replace(/^--no-/, "--"))) { + throw new Error(`Reserved Vally option: ${option}. Eval selection, output and verdict settings are framework-owned.`); + } + let argument = value[index + 1]; + const isCount = option === "--runs" || option === "--workers"; + if (argument === undefined || (typeof argument === "string" && argument.startsWith("-"))) { + if (isCount) { + throw new Error(`Missing value for ${option}`); + } + args.push(option); + continue; + } + // Azure template expansion can serialize even format('{0}', number) as a JSON number. + if (typeof argument === "number") { + if (!isCount || !Number.isSafeInteger(argument) || argument < 1) { + throw new Error("Numeric values are supported only for positive integer --runs and --workers counts."); + } + argument = String(argument); + } + if (!argument) { + throw new Error(`Missing value for ${option}`); + } + if (isCount && + (!/^[1-9]\d*$/.test(argument) || !Number.isSafeInteger(Number(argument)))) { + throw new Error(`${option} must be a positive integer.`); + } + if (option === "--runs") { + assertSupportedEvalGate({ runs: Number(argument), failOnFailedTests }); + } + args.push(option, argument); + index++; + } + return args; +} /** * Executes the shard and returns the process exit code (0 pass, 1 fail). Side effects @@ -19,11 +72,14 @@ const SCRIPT_DIR = path.dirname(fileURLToPath(import.meta.url)); * @param {string} options.shardName Shard name (log messages + result folder). * @param {string} options.outputDir `--output-dir` Vally writes results into. * @param {number} [options.threshold] Pass-rate gate forwarded to `vally eval --threshold`. + * @param {(string | number)[]} [options.vallyArguments] Non-reserved options and switches; numeric trial/worker counts are normalized. * @returns {number} 0 when the shard verdict passes, 1 otherwise. */ -export function runShard({ evalArgs, shardName, outputDir, threshold = 0.8 }) { +export function runShard({ evalArgs, shardName, outputDir, threshold = 0.8, vallyArguments = [] }, + { spawn = spawnSync } = {}) { const evalArgList = evalArgs.split(/\s+/).filter(Boolean); const thresholdArg = String(threshold); + const supplementaryArgs = validateVallyArguments(vallyArguments); console.log( `Running: vally eval ${evalArgs} --junit --threshold ${thresholdArg} --output-dir "${outputDir}"` @@ -33,25 +89,22 @@ export function runShard({ evalArgs, shardName, outputDir, threshold = 0.8 }) { // Uses spawnSync (not the vendored execFile helper) on purpose: the shard needs vally's output // streamed live to the log (stdio: "inherit") and must keep going on a non-zero exit, whereas // the exec helper captures output and rejects on failure. - const proc = spawnSync( - "npm", + const proc = spawn( + process.execPath, [ - "exec", - "--no", - "--prefix", - SCRIPT_DIR, - "--", - "vally", + VALLY_CLI, "eval", ...evalArgList, + ...supplementaryArgs, "--junit", "--threshold", thresholdArg, "--output-dir", outputDir, ], - { stdio: "inherit", shell: process.platform === "win32" } + { stdio: "inherit", shell: false } ); + if (proc.error) throw proc.error; const vallyExit = proc.status ?? 1; const verdict = getVallyShardVerdict({ resultsDir: outputDir, threshold }); @@ -91,11 +144,18 @@ export function runShard({ evalArgs, shardName, outputDir, threshold = 0.8 }) { // ----- CLI ----- -function parseArgs(argv) { - const options = { threshold: 0.8 }; +export function parseArgs(argv, env = process.env) { + const options = { + threshold: 0.8, + vallyArguments: validateVallyArguments(JSON.parse(env.VALLY_ARGUMENTS_JSON ?? "[]"), + { failOnFailedTests: parseFailedTestsGate(env.EVAL_FAIL_ON_FAILED_TESTS) }), + }; for (let i = 0; i < argv.length; i++) { const next = () => argv[++i]; switch (argv[i]) { + case "--validate-only": + options.validateOnly = true; + break; case "--eval-args": options.evalArgs = next(); break; @@ -112,9 +172,11 @@ function parseArgs(argv) { throw new Error(`Unknown argument: ${argv[i]}`); } } - for (const required of ["evalArgs", "shardName", "outputDir"]) { - if (!options[required]) { - throw new Error(`Missing required argument for ${required}.`); + if (!options.validateOnly) { + for (const required of ["evalArgs", "shardName", "outputDir"]) { + if (!options[required]) { + throw new Error(`Missing required argument for ${required}.`); + } } } return options; @@ -122,7 +184,12 @@ function parseArgs(argv) { if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { try { - process.exit(runShard(parseArgs(process.argv.slice(2)))); + const options = parseArgs(process.argv.slice(2)); + if (options.validateOnly) { + console.log("Eval run configuration is supported."); + } else { + process.exit(runShard(options)); + } } catch (error) { console.error(error.message); process.exit(1); diff --git a/eng/common/scripts/eval/lib/shard-results.ts b/eng/common/scripts/eval/lib/shard-results.ts index 8160e51f22..7159d5c35e 100644 --- a/eng/common/scripts/eval/lib/shard-results.ts +++ b/eng/common/scripts/eval/lib/shard-results.ts @@ -182,13 +182,15 @@ export function selectSummaryResults({ resultsRoot, selectedRoot, expectedShards new Set(expectedShards).size !== expectedShards.length) { throw new Error("Expected shards must be unique, valid shard names from Prepare."); } + const resultPrefix = "eval-result-"; emptyDirectory(selectedRoot); const expected = new Set(expectedShards); const timeline = jobAttempts?.schemaVersion === 1 && jobAttempts.valid === true ? jobAttempts.attempts : null; const selected = new Map(); const seen = new Set(); for (const entry of fs.readdirSync(resultsRoot, { withFileTypes: true })) { - const match = /^eval-result-(.+)-(\d+)$/.exec(entry.name); + if (!entry.name.startsWith(resultPrefix)) continue; + const match = /^(.+)-(\d+)$/.exec(entry.name.slice(resultPrefix.length)); if (!match || !expected.has(match[1])) continue; if (!entry.isDirectory()) throw new Error("Shard artifact paths must be directories, not files or symbolic links."); const attempt = Number(match[2]); @@ -232,7 +234,7 @@ export function selectSummaryResults({ resultsRoot, selectedRoot, expectedShards // The Tests tab and Markdown use the SAME selected attempts as the bundle. // Old-layout JUnit remains diagnostic; old artifacts are never upload-ready. const files = junit.length ? junit : globFiles(directory, "**/*.junit.xml"); - const output = path.join(selectedRoot, selectedAttempt.directory, "junit"); + const output = path.join(selectedRoot, `eval-result-${shard}-${selectedAttempt.attempt}`, "junit"); fs.mkdirSync(output, { recursive: true }); for (const [index, file] of files.entries()) { try { diff --git a/eng/common/scripts/eval/lib/verdict.ts b/eng/common/scripts/eval/lib/verdict.ts index 330c59be75..e9e1617c19 100644 --- a/eng/common/scripts/eval/lib/verdict.ts +++ b/eng/common/scripts/eval/lib/verdict.ts @@ -7,6 +7,19 @@ import fs from "node:fs"; import path from "node:path"; import { globFiles } from "./glob.ts"; +export function parseFailedTestsGate(value: unknown = "false") { + if (typeof value !== "string" || !/^(true|false)$/i.test(value)) { + throw new Error("EVAL_FAIL_ON_FAILED_TESTS must be true or false."); + } + return value.toLowerCase() === "true"; +} + +export function assertSupportedEvalGate({ runs, failOnFailedTests }) { + if (failOnFailedTests && runs > 1) { + throw new Error("This eval configuration is not currently supported: multiple runs cannot be combined with failOnFailedTests=true. Disable the gate or set --runs to 1."); + } +} + // Formats a 0..1 ratio as a fixed-1-decimal percentage string (e.g. 97.1). function pct(ratio) { return (ratio * 100).toFixed(1); diff --git a/eng/common/scripts/eval/test/shard-results.test.ts b/eng/common/scripts/eval/test/shard-results.test.ts index 68fb908b08..74eb6541c3 100644 --- a/eng/common/scripts/eval/test/shard-results.test.ts +++ b/eng/common/scripts/eval/test/shard-results.test.ts @@ -70,7 +70,8 @@ function runSummary(root, shards, options = {}) { const child = runCli("build-eval-summary.ts", [ "--results-root", path.join(root, "download"), "--selected-root", path.join(root, "selected"), "--output-path", output, - ], { TF_BUILD: "true", EVAL_EXPECTED_MATRIX: matrix }); + ], { TF_BUILD: "true", EVAL_EXPECTED_MATRIX: matrix, + EVAL_FAIL_ON_FAILED_TESTS: "false", ...options.env }); return { child, markdown: fs.readFileSync(output, "utf8"), result: readJson(path.join(root, "summary", "eval-summary.json")), @@ -88,6 +89,102 @@ test("expected shards come from the full Prepare matrix, not arrived artifacts o } }); +test("Summary renders threshold verdicts, including 4/5 passes, without inferring configuration support", (t) => { + for (const [runs, passed] of [[1, 1], [5, 4], [5, 5]]) { + for (const gate of ["true", "false"]) { + const root = setup(t); + const shard = "area_runs"; + const output = artifact(root, shard, 1); + const records = Array.from({ length: runs }, (_, index) => ({ + type: "trial-result", status: index < passed ? "success" : "error", itemId: "example", + })); + fs.writeFileSync(path.join(output, "results.jsonl"), + [...records, { type: "run-summary", evals: [{ name: "example", passed: true }] }] + .map(record => JSON.stringify(record)).join("\n") + "\n"); + const metadata = readJson(path.join(output, "shard.json")); + metadata.trials = runs; + fs.writeFileSync(path.join(output, "shard.json"), JSON.stringify(metadata)); + const xml = '' + + Array.from({ length: runs }, (_, index) => `${ + index < passed ? "" : ''}`).join("") + ""; + const junit = path.join(output, "junit", "0.junit.xml"); + fs.writeFileSync(junit, xml); + const result = runSummary(root, [shard], { env: { EVAL_FAIL_ON_FAILED_TESTS: gate } }); + assert.equal(result.child.status, 0, result.child.stderr); + assert.equal(result.result.complete, true); + assert.equal(result.result.totals.failed, 0); + assert.equal(fs.readFileSync(junit, "utf8"), xml); + assert.equal(fs.readFileSync(path.join(root, "selected", `eval-result-${shard}-1`, "junit", "0.junit.xml"), "utf8"), xml); + assert.match(result.child.stdout, /variable=EvalSummaryComplete]true/); + } + } +}); + +test("Summary does not reject distinct single-run evals with the same stimulus name", (t) => { + const root = setup(t); + const shard = "area_same_names"; + const output = artifact(root, shard, 1); + const evals = ["first", "second"]; + fs.writeFileSync(path.join(output, "results.jsonl"), + [...evals.map(name => ({ type: "trial-result", status: "success", itemId: name })), + { type: "run-summary", evals: evals.map(name => ({ name, passed: true })) }] + .map(record => JSON.stringify(record)).join("\n") + "\n"); + const metadata = readJson(path.join(output, "shard.json")); + metadata.trials = evals.length; + fs.writeFileSync(path.join(output, "shard.json"), JSON.stringify(metadata)); + fs.writeFileSync(path.join(output, "junit", "0.junit.xml"), + `${evals.map(name => ` + + + `).join("")}`); + const result = runSummary(root, [shard], { env: { EVAL_FAIL_ON_FAILED_TESTS: "true" } }); + assert.equal(result.child.status, 0, result.child.stderr); + assert.equal(result.result.complete, true); + assert.equal(result.result.totals.failed, 0); + assert.match(result.child.stdout, /variable=EvalSummaryComplete]true/); +}); + +test("single-run gating does not bypass missing-result completeness failures", (t) => { + const root = setup(t); + artifact(root, "area_present", 1); + const result = runSummary(root, ["area_present", "area_missing"], { env: { EVAL_FAIL_ON_FAILED_TESTS: "true" } }); + assert.equal(result.child.status, 1); + assert.equal(result.result.complete, false); +}); + +test("shard job artifact names preserve hyphenated shard identities through Summary selection", (t) => { + const load = name => fs.readFileSync(new URL(`../../../pipelines/templates/jobs/${name}.yml`, import.meta.url), "utf8"); + const shardTemplate = load("eval-shard"); + const resultName = shardTemplate.match(/^ artifactName: '(eval-result-[^'\r\n]+)'\r?$/m); + assert.ok(resultName); + const root = setup(t); + const shard = "extra-area_same"; + for (const attempt of [1, 2]) { + const name = resultName[1].replace("$(shardName)", shard).replace("$(System.JobAttempt)", String(attempt)); + stageShardResults({ resultsRoot: invocation(root, `source-${attempt}`), + outputDirectory: path.join(root, "download", name), shardName: shard, attempt }); + } + fs.writeFileSync(path.join(root, "job-attempts.json"), JSON.stringify({ + schemaVersion: 1, valid: true, attempts: { [shard]: { attempt: 2, complete: true } }, + })); + const result = runSummary(root, [shard]); + assert.equal(result.child.status, 0, result.child.stderr); + assert.equal(result.result.complete, true); + assert.deepEqual(result.index.attempts, [{ shard, attempt: 2, directory: `eval-result-${shard}-2` }]); +}); + +test("Summary downloads only build-wide result data", () => { + const summary = fs.readFileSync(new URL("../../../pipelines/templates/jobs/eval-summarize.yml", import.meta.url), "utf8"); + const download = summary.split(/^ - task: /m).find(section => + section.split(/\r?\n/, 1)[0] === "DownloadPipelineArtifact@2"); + assert.ok(download); + const block = download.match(/^ itemPattern: \|\r?\n((?: {12}[^\r\n]+\r?\n)+)/m); + assert.ok(block); + const patterns = block[1].trim().split(/\r?\n/).map(line => line.trim()); + const files = ["results.jsonl", "shard.json", "**/*.junit.xml"]; + assert.deepEqual(patterns, files.map(file => `eval-result-*/${file}`)); +}); + test("completed failing evaluations stage exact raw bytes and all JUnit, but no unrelated files", (t) => { const root = setup(t); const source = invocation(root, "source", { failed: true }); @@ -484,7 +581,8 @@ const timelineEnv = { SYSTEM_COLLECTIONURI: "https://dev.azure.com/azure-sdk/", const timelineRecord = { type: "Job", identifier: "Eval.RunShard.a", attempt: 1, state: "completed", result: "failed" }; test("timeline uses matrix identifiers, highest attempts and completed state, rejecting ambiguous attempts", () => { - const matrix = { a: { shardName: "a" } }, records = [timelineRecord, { ...timelineRecord, attempt: 2 }]; + const matrix = { a: { shardName: "a" } }, records = [timelineRecord, { ...timelineRecord, attempt: 2 }, + { ...timelineRecord, identifier: "OtherEval.RunShard.a", attempt: 3 }]; assert.deepEqual(latestShardAttempts({ records }, matrix).attempts.a, { attempt: 2, complete: true }); records[1].result = "canceled"; assert.equal(latestShardAttempts({ records }, matrix).attempts.a.complete, false); diff --git a/eng/common/scripts/eval/test/verdict.test.ts b/eng/common/scripts/eval/test/verdict.test.ts index fe4978ea5b..8fc89c01e6 100644 --- a/eng/common/scripts/eval/test/verdict.test.ts +++ b/eng/common/scripts/eval/test/verdict.test.ts @@ -5,9 +5,290 @@ import assert from "node:assert/strict"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import { fileURLToPath } from "node:url"; +import { runInNewContext } from "node:vm"; import { after, before, describe, it } from "node:test"; -import { getVallyShardVerdict } from "../lib/verdict.ts"; +import { getVallyShardVerdict, parseFailedTestsGate } from "../lib/verdict.ts"; +import { parseArgs, runShard, validateVallyArguments } from "../invoke-eval-shard.ts"; +import { spawnSync } from "node:child_process"; + +describe("eval template authentication source contract", () => { + const load = (relative: string) => fs.readFileSync(new URL(relative, import.meta.url), "utf8"); + const invoke = load("../../../pipelines/templates/steps/eval-invoke.yml"); + const branches = invoke.split(/(?=^ - \$\{\{ )/m).slice(1); + + it("preserves the invoke connection default and leaves upstream overrides optional", () => { + assert.match(invoke, /^ - name: azureSubscription\r?\n type: string\r?\n default: opensource-api-connection\r?$/m); + for (const template of [invoke, + load("../../../pipelines/templates/jobs/eval-shard.yml"), + load("../../../pipelines/templates/stages/archetype-eval.yml")]) { + assert.match(template, /^ - name: UseAzSdkAuthentication\r?\n(?: #[^\r\n]*\r?\n)* type: boolean\r?\n default: false\r?$/m); + } + for (const template of [ + load("../../../pipelines/templates/jobs/eval-shard.yml"), + load("../../../pipelines/templates/stages/archetype-eval.yml")]) { + assert.match(template, /^ - name: azureSubscription\r?\n(?: #[^\r\n]*\r?\n)* type: string\r?\n default: ''\r?$/m); + assert.doesNotMatch(template, /EvalAzureSubscription|opensource-api-connection|azuresdkqabot-dev/); + } + }); + + it("keeps the missing-connection error in the first branch without an AzureCLI task", () => { + assert.equal(branches.length, 3); + const configuration = branches[0]; + assert.equal(configuration.split(/\r?\n/, 1)[0], + " - ${{ if and(parameters.UseAzSdkAuthentication, eq(trim(parameters.azureSubscription), '')) }}:"); + assert.equal(configuration.match(/^ - /gm)?.length, 1); + assert.match(configuration, /^ - script: \|\r?$/m); + assert.doesNotMatch(configuration, /^\s*- task:/m); + assert.match(configuration, /##vso\[task\.logissue type=error\].*azureSubscription.*UseAzSdkAuthentication/); + assert.match(configuration, /^ exit 1\r?$/m); + }); + + it("uses the default when omitted and the caller's override when provided", () => { + const defaultConnection = invoke.match(/^ default: (opensource-api-connection)\r?$/m)?.[1]; + assert.ok(defaultConnection); + const condition = branches[0].split(/\r?\n/, 1)[0].match(/if (.+) \}\}:$/)?.[1]; + const input = branches[1].match(/azureSubscription: \$\{\{ (.+) \}\}/)?.[1]; + assert.ok(condition); + assert.ok(input); + for (const enabled of [false, true]) { + for (const connection of [undefined, "", " \t ", "repo-owned-connection"]) { + const parameters = { UseAzSdkAuthentication: enabled, azureSubscription: connection ?? defaultConnection }; + const missingConnection = runInNewContext(condition, { + parameters, + variables: { EvalAzureSubscription: "unrelated-outer-variable" }, + and: (...values: boolean[]) => values.every(Boolean), + eq: (left: string, right: string) => left === right, + trim: (value: string) => value.trim(), + }); + assert.equal(missingConnection, enabled && !(connection ?? defaultConnection).trim(), + `authentication=${enabled}, connection=${JSON.stringify(connection)}`); + if (enabled && !missingConnection) { + assert.equal(runInNewContext(input, { parameters }), connection ?? defaultConnection); + } + } + } + }); + + it("references the caller's connection only in the authenticated branch", () => { + const authenticated = branches[1]; + assert.equal(authenticated.split(/\r?\n/, 1)[0], " - ${{ elseif parameters.UseAzSdkAuthentication }}:"); + assert.equal(authenticated.match(/^ - /gm)?.length, 1); + assert.match(authenticated, /^ - task: AzureCLI@2\r?$/m); + assert.match(authenticated, /^ azureSubscription: \$\{\{ parameters\.azureSubscription \}\}\r?$/m); + const unauthenticated = branches[2]; + assert.equal(unauthenticated.split(/\r?\n/, 1)[0], " - ${{ else }}:"); + assert.equal(unauthenticated.match(/^ - /gm)?.length, 1); + assert.match(unauthenticated, /^ - script: \|\r?$/m); + assert.match(unauthenticated, /invoke-eval-shard\.ts/); + assert.doesNotMatch(unauthenticated, /^\s*(?:- task|azureSubscription):/m); + for (const branch of [authenticated, unauthenticated]) { + assert.equal(branch.match(/^ VALLY_ARGUMENTS_JSON: (.+)\r?$/m)?.[1].trim(), + "${{ replace(convertToJson(parameters.vallyArguments), '$', '\\u0024') }}"); + assert.match(branch, /^ COPILOT_GITHUB_TOKEN: \$\(COPILOT_GITHUB_TOKEN\)\r?$/m); + } + }); + + it("resolves the live connection at the entrypoint and forwards it through stage and job parameters", () => { + const live = load("../../../pipelines/live-eval.yml"); + assert.match(live, /^ UseAzSdkAuthentication: true\r?$/m); + assert.match(live, /^ - template: \/eng\/pipelines\/templates\/variables\/globals\.yml\r?$/m); + assert.match(live, /^ azureSubscription: \$\{\{ variables\.EvalAzureSubscription \}\}\r?$/m); + assert.doesNotMatch(live, /^ - name: azureSubscription\r?$|parameters\.azureSubscription|opensource-api-connection|azuresdkqabot-dev/m); + const stage = load("../../../pipelines/templates/stages/archetype-eval.yml"); + const evalStage = stage.split(/^ - stage: /m).find(section => section.split(/\r?\n/, 1)[0] === "Eval"); + assert.ok(evalStage); + assert.match(evalStage, /^ - template: \/eng\/common\/pipelines\/templates\/jobs\/eval-shard\.yml\r?$/m); + assert.match(evalStage, /^ azureSubscription: \$\{\{ parameters\.azureSubscription \}\}\r?$/m); + const shard = load("../../../pipelines/templates/jobs/eval-shard.yml"); + const invokeStep = shard.split(/^ - template: /m).find(section => + section.split(/\r?\n/, 1)[0] === "${{ parameters.evalInvokeTemplate }}"); + assert.ok(invokeStep); + assert.match(invokeStep, /^ \$\{\{ if ne\(parameters\.azureSubscription, ''\) \}\}:\r?\n azureSubscription: \$\{\{ parameters\.azureSubscription \}\}\r?$/m); + }); + + it("preflights common shard arguments before setup and forwards the gate to both jobs", () => { + const shard = load("../../../pipelines/templates/jobs/eval-shard.yml"); + const preflight = shard.indexOf("displayName: 'Validate eval run configuration'"); + assert.ok(preflight >= 0 && preflight < shard.indexOf("displayName: 'Install Vally CLI")); + assert.match(shard, /^ - \$\{\{ if eq\(parameters\.evalInvokeTemplate, '\/eng\/common\/pipelines\/templates\/steps\/eval-invoke\.yml'\) \}\}:\r?\n - script: \|/m); + assert.match(shard, /invoke-eval-shard\.ts" --validate-only/); + assert.match(shard, /^ EVAL_FAIL_ON_FAILED_TESTS: \$\{\{ parameters\.failOnFailedTests \}\}\r?$/m); + assert.equal(shard.match(/^ VALLY_ARGUMENTS_JSON: (.+)\r?$/m)?.[1].trim(), + "${{ replace(convertToJson(parameters.vallyArguments), '$', '\\u0024') }}"); + const stage = load("../../../pipelines/templates/stages/archetype-eval.yml"); + for (const name of ["Eval", "Summary"]) { + const section = stage.split(/^ - stage: /m).find(value => value.split(/\r?\n/, 1)[0] === name); + assert.ok(section); + assert.match(section, /^ failOnFailedTests: \$\{\{ parameters\.failOnFailedTests \}\}\r?$/m); + } + const summary = load("../../../pipelines/templates/jobs/eval-summarize.yml"); + assert.doesNotMatch(summary, /EVAL_FAIL_ON_FAILED_TESTS/); + assert.match(summary, /^ failTaskOnFailedTests: \$\{\{ parameters\.failOnFailedTests \}\}\r?$/m); + }); +}); + +describe("supplementary Vally arguments", () => { + it("rejects only multi-run gating, preserving single-run gates and multi-run observations", () => { + for (const runs of [1, "1"]) { + assert.deepEqual(validateVallyArguments(["--runs", runs], { failOnFailedTests: true }), ["--runs", "1"]); + } + for (const runs of [2, "2", 5, "5"]) { + assert.throws(() => validateVallyArguments(["--runs", runs], { failOnFailedTests: true }), + /not currently supported.*failOnFailedTests=true/); + assert.deepEqual(validateVallyArguments(["--runs", runs], { failOnFailedTests: false }), ["--runs", String(runs)]); + } + assert.deepEqual(validateVallyArguments([], { failOnFailedTests: true }), []); + assert.deepEqual(validateVallyArguments(["--workers", 5], { failOnFailedTests: true }), ["--workers", "5"]); + for (const value of ["true", "True", "TRUE"]) assert.equal(parseFailedTestsGate(value), true); + for (const value of [undefined, "false", "False"]) assert.equal(parseFailedTestsGate(value), false); + for (const value of ["", "yes", "1", null, true]) assert.throws(() => parseFailedTestsGate(value), /must be true or false/); + }); + + it("validates the configuration through the CLI without starting an evaluation", () => { + for (const [runs, gate, expected] of [[5, "true", 1], ["5", "True", 1], [1, "true", 0], ["1", "True", 0], [5, "false", 0]] as const) { + const env = { ...process.env, EVAL_FAIL_ON_FAILED_TESTS: gate, VALLY_ARGUMENTS_JSON: JSON.stringify(["--runs", runs]) }; + delete env.NODE_TEST_CONTEXT; + const child = spawnSync(process.execPath, ["--experimental-strip-types", + fileURLToPath(new URL("../invoke-eval-shard.ts", import.meta.url)), "--validate-only"], { + encoding: "utf8", timeout: 30_000, env, + }); + assert.ifError(child.error); + assert.equal(child.status, expected, child.stderr); + assert.doesNotMatch(child.stdout, /Running: vally eval/); + if (expected) assert.match(child.stderr, /not currently supported.*failOnFailedTests=true/); + else assert.match(child.stdout, /configuration is supported/); + } + }); + + it("preserves defined pipeline macros through simulated agent expansion and JSON decoding", () => { + const variables: Record = { "Build.BuildId": "12345", DefinedValue: 'quoted "value"\nand newline' }; + const expand = (value: string) => value.replace(/\$\(([^)]+)\)/g, (macro, name) => variables[name] ?? macro); + const args = ["--tag", "build=$(Build.BuildId)", "--tag", "label=$(DefinedValue)", + "--tag", String.raw`literal=\u0024; $5; $(undefined-variable)`]; + assert.equal(expand(args[1]), "build=12345"); + const json = JSON.stringify(args); + assert.throws(() => JSON.parse(expand(json)), SyntaxError); + const escaped = json.replaceAll("$", "\\u0024"); + assert.doesNotMatch(escaped, /\$\(/); + assert.equal(expand(escaped), escaped); + const options = parseArgs(["--eval-args", "-e example.eval.yaml", "--shard-name", "example", "--output-dir", "out"], + { VALLY_ARGUMENTS_JSON: expand(escaped) }); + assert.deepEqual(options.vallyArguments, args); + }); + + it("passes non-reserved options through without a catalog or splitting values", () => { + const args = ["--tag", "label=two words; $(not-a-command) & \"quoted\"", "--runs", "3", "--workers", "1", "--timeout", "5m"]; + args.push("--model", "example-model", "--max-retries", "0", "--reporter-plugin", "./reporter.js", + "--executor-plugin", "./executor.js", "--grader-plugin", "./grader.js", "--future-option", "literal value"); + assert.deepEqual(validateVallyArguments(args), args); + const options = parseArgs(["--eval-args", "-e example.eval.yaml", "--shard-name", "example", "--output-dir", "out"], + { VALLY_ARGUMENTS_JSON: JSON.stringify(args) }); + assert.deepEqual(options.vallyArguments, args); + }); + + it("preserves switches before, between and after option values without a switch catalog", () => { + for (const args of [ + ["--verbose"], + ["--verbose", "--in-place", "--future-switch"], + ["--verbose", "--model", "example-model", "--future-switch", "--runs", 2, "--in-place"], + ]) { + const expected = args.map(String); + assert.deepEqual(validateVallyArguments(args), expected); + assert.deepEqual(parseArgs(["--validate-only"], + { VALLY_ARGUMENTS_JSON: JSON.stringify(args) }).vallyArguments, expected); + } + // Vally, not the wrapper, validates whether ordinary options require values. + assert.deepEqual(validateVallyArguments(["--model"]), ["--model"]); + }); + + it("normalizes numeric trial and worker counts produced by Azure template expansion", () => { + for (const mode of ["forced", "trigger", "no-skill"]) { + for (const runs of [1, 3]) { + const args = ["--tag", `mode=${mode}`, "--runs", runs, "--workers", 2]; + const expected = ["--tag", `mode=${mode}`, "--runs", String(runs), "--workers", "2"]; + assert.deepEqual(validateVallyArguments(args), expected); + assert.equal(args[3], runs); + const options = parseArgs(["--eval-args", "-e example.eval.yaml", "--shard-name", "example", "--output-dir", "out"], + { VALLY_ARGUMENTS_JSON: JSON.stringify(args) }); + assert.deepEqual(options.vallyArguments, expected); + } + } + }); + + it("rejects framework-owned output, verdict and eval selection overrides", () => { + for (const option of ["--output-dir", "--output", "--junit", "--threshold", + "--require-pass", "--skip-grade", "--skip-validate", "--eval-spec", "--suite", "--work-dir", "--eval-plugin"]) { + for (const spelling of [option, option.replace(/^--/, "--no-")]) { + assert.throws(() => validateVallyArguments([spelling, "value"]), /Reserved Vally option/); + assert.throws(() => validateVallyArguments(["--verbose", spelling]), /Reserved Vally option/); + } + } + }); + + it("rejects aliases, equals syntax, positional eval paths and malformed arguments", () => { + for (const option of ["-e", "-eelsewhere.eval.yaml", "-r", "--", "--output-dir=elsewhere", + "--no-junit=false", "--runs=3", "--model=example", "elsewhere.eval.yaml", "**/*.eval.yaml", + "--model example", "--model\n", "--model;echo"]) { + assert.throws(() => validateVallyArguments([option, "value"]), /Unsupported.*format/); + } + for (const args of [null, {}, " --runs 3", [3], ["--runs"], ["--workers"], + ["--runs", "--verbose"], ["--workers", "--in-place"], ["--runs", "0"], ["--runs", "-1"], + ["--workers", "1.5"], ["--tag", "--threshold"], ["--tag", "value\0"], + ["--model", ""], ["--model", 1], ["--model", null], ["--model", "value\0"], + ["--model", "example", "elsewhere.eval.yaml"], + ["--runs", null], ["--runs", true], ["--runs", {}], ["--runs", []], + ["--runs", 0], ["--runs", -1], ["--runs", 1.5], ["--runs", NaN], + ["--workers", Infinity], ["--workers", Number.MAX_SAFE_INTEGER + 1], + ["--tag", 1], ["--timeout", 5], [1, "value"]]) { + assert.throws(() => validateVallyArguments(args)); + } + assert.throws(() => parseArgs([], { VALLY_ARGUMENTS_JSON: "not JSON" })); + assert.deepEqual(validateVallyArguments([]), []); + }); + + it("passes literal arguments through a real Node process, retaining framework-owned settings", (t) => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), "vally-arguments-")); + t.after(() => fs.rmSync(root, { recursive: true, force: true })); + const capture = path.join(root, "capture.json"); + const fakeCli = path.join(root, "fake-cli.mjs"); + const outputDir = path.join(root, "results with spaces"); + fs.writeFileSync(fakeCli, ` + import fs from "node:fs"; + import path from "node:path"; + const args = process.argv.slice(2); + fs.writeFileSync(${JSON.stringify(capture)}, JSON.stringify(args)); + const output = args[args.indexOf("--output-dir") + 1]; + fs.mkdirSync(output, { recursive: true }); + fs.writeFileSync(path.join(output, "results.jsonl"), JSON.stringify({ + type: "run-summary", evals: [{ name: "test", passed: true, stimuliRun: 1 }] + })); + `); + const args = ["--verbose", "--tag", "label=a b; $(echo bad) & \"quotes\" %PATH%", "--tag", "build=$(Build.BuildId)", "--runs", 3, + "--in-place", "--model", "example-model", "--reporter-plugin", "./reporter with spaces.js", + "--future-option", "literal value", "--future-switch"]; + const options = parseArgs(["--eval-args", "-e test.eval.yaml", "--shard-name", "test", "--output-dir", outputDir], + { VALLY_ARGUMENTS_JSON: JSON.stringify(args).replaceAll("$", "\\u0024") }); + const exit = runShard(options, { + spawn(command, commandArgs, options) { + assert.equal(command, process.execPath); + assert.equal(options.shell, false); + assert.match(commandArgs[0], /vally-cli[\\/]dist[\\/]index\.js$/); + return spawnSync(command, [fakeCli, ...commandArgs.slice(1)], options); + }, + }); + assert.equal(exit, 0); + const actual = JSON.parse(fs.readFileSync(capture, "utf8")); + assert.deepEqual(actual, ["eval", "-e", "test.eval.yaml", ...validateVallyArguments(args), + "--junit", "--threshold", "0.8", "--output-dir", outputDir]); + }); + + it("surfaces process launch failures instead of reading stale results", () => { + assert.throws(() => runShard({ evalArgs: "-e x", shardName: "x", outputDir: "unused" }, + { spawn: () => ({ error: new Error("failed to launch") }) }), /failed to launch/); + }); +}); describe("getVallyShardVerdict", () => { let root;