diff --git a/eng/common/pipelines/live-eval.yml b/eng/common/pipelines/live-eval.yml index e18bafa65f..cc1ae1c0d6 100644 --- a/eng/common/pipelines/live-eval.yml +++ b/eng/common/pipelines/live-eval.yml @@ -37,6 +37,7 @@ extends: - 'workflows/live/*.eval.yaml' # Run each shard under AzureCLI@2 so the real MCP's DevOps calls are authenticated. UseAzSdkAuthentication: true + azureSubscription: opensource-api-connection 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..942fca312b 100644 --- a/eng/common/pipelines/templates/jobs/eval-shard.yml +++ b/eng/common/pipelines/templates/jobs/eval-shard.yml @@ -6,10 +6,31 @@ parameters: - name: vallyRoot # Required (no default) so each repo passes its own eval root explicitly. type: string + - name: matrixStageName + # Stage containing the generate_eval_matrix job. + type: string + default: Prepare + - name: artifactPrefix + # Literal prefix after 'eval-result-' / 'eval-debug-'; include any separator (e.g. 'forced-'). + # Match Summary's prefix and keep final artifact names unique across this build's tracks. + type: string + default: '' - name: evalInvokeTemplate # 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: azureSubscription + # Caller-owned service connection; required by the common authenticated invoke template. + type: string + default: '' - name: UseAzSdkAuthentication # Forwarded to the invoke template: true runs the shard under AzureCLI@2. type: boolean @@ -44,7 +65,7 @@ jobs: workspace: clean: all strategy: - matrix: $[ stageDependencies.Prepare.generate_eval_matrix.outputs['detect.matrix'] ] + matrix: $[ stageDependencies.${{ parameters.matrixStageName }}.generate_eval_matrix.outputs['detect.matrix'] ] maxParallel: 10 templateContext: outputs: @@ -53,12 +74,12 @@ jobs: # The Summary job keeps only the highest attempt per shard so retries supersede cleanly. - output: pipelineArtifact displayName: 'Publish $(shardName) raw results, JUnit and completeness' - artifactName: 'eval-result-$(shardName)-$(System.JobAttempt)' + artifactName: 'eval-result-${{ parameters.artifactPrefix }}$(shardName)-$(System.JobAttempt)' targetPath: $(Build.ArtifactStagingDirectory)/eval-result condition: always() - output: pipelineArtifact displayName: 'Publish $(shardName) transcripts (on failure)' - artifactName: 'eval-debug-$(shardName)-$(System.JobAttempt)' + artifactName: 'eval-debug-${{ parameters.artifactPrefix }}$(shardName)-$(System.JobAttempt)' targetPath: $(Build.ArtifactStagingDirectory)/debug condition: failed() steps: @@ -71,6 +92,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 +142,16 @@ 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 }} threshold: ${{ parameters.threshold }} + # Existing custom templates keep their contract unless opting into the argument list. + ${{ if eq(parameters.evalInvokeTemplate, '/eng/common/pipelines/templates/steps/eval-invoke.yml') }}: + azureSubscription: ${{ parameters.azureSubscription }} + ${{ 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..a9552c046a 100644 --- a/eng/common/pipelines/templates/jobs/eval-summarize.yml +++ b/eng/common/pipelines/templates/jobs/eval-summarize.yml @@ -2,7 +2,19 @@ # gates on it when failOnFailedTests is true. parameters: + - name: matrixStageName + type: string + default: Prepare + - name: evalStageName + type: string + default: Eval + - name: artifactPrefix + # Literal prefix, including any separator (e.g. 'forced-'); must match the shard job. + # Callers are responsible for unique final artifact names across tracks in one build. + type: string + default: '' - name: failOnFailedTests + # Only single-run evaluations support this gate; multi-run observations must leave it off. type: boolean default: false - name: createDashboardBundle @@ -22,15 +34,22 @@ jobs: workspace: clean: all variables: - EvalExpectedMatrix: $[ stageDependencies.Prepare.generate_eval_matrix.outputs['detect.matrix'] ] + EvalExpectedMatrix: $[ stageDependencies.${{ parameters.matrixStageName }}.generate_eval_matrix.outputs['detect.matrix'] ] templateContext: outputs: - output: pipelineArtifact displayName: 'Retain selected build summary and saved bundle' - artifactName: 'eval-summary-$(System.JobAttempt)' + artifactName: 'eval-summary-${{ parameters.artifactPrefix }}$(System.JobAttempt)' targetPath: $(Build.ArtifactStagingDirectory)/eval-summary condition: always() steps: + # Dashboard archives identify a single build-wide summary, not separate tracks. + - ${{ if and(ne(parameters.artifactPrefix, ''), or(parameters.createDashboardBundle, parameters.publishDashboardResults)) }}: + - script: | + echo "##vso[task.logissue type=error]Prefixed eval tracks support artifact-only summaries, not dashboard bundles or publication." + exit 1 + displayName: 'Reject ambiguous multi-track dashboard identity' + - checkout: self clean: true fetchDepth: 1 @@ -40,15 +59,15 @@ jobs: inputs: version: 24.x - # Allowlist result data only, excluding debug, MCP, SDL and signing artifacts. + # Download only this track's result data; an empty prefix retains all eval-result-* artifacts. - task: DownloadPipelineArtifact@2 displayName: 'Download all shard results' inputs: buildType: 'current' itemPattern: | - eval-result-*/results.jsonl - eval-result-*/shard.json - eval-result-*/**/*.junit.xml + eval-result-${{ parameters.artifactPrefix }}*/results.jsonl + eval-result-${{ parameters.artifactPrefix }}*/shard.json + eval-result-${{ parameters.artifactPrefix }}*/**/*.junit.xml targetPath: $(Pipeline.Workspace)/eval-results # Only attempt verification needs the current build token; rendering uses saved evidence. @@ -60,6 +79,7 @@ jobs: env: SYSTEM_ACCESSTOKEN: $(System.AccessToken) EVAL_EXPECTED_MATRIX: $(EvalExpectedMatrix) + EVAL_STAGE_NAME: ${{ parameters.evalStageName }} - script: | node --experimental-strip-types "$(Build.SourcesDirectory)/eng/common/scripts/eval/build-eval-summary.ts" \ @@ -71,6 +91,7 @@ jobs: condition: always() env: EVAL_EXPECTED_MATRIX: $(EvalExpectedMatrix) + EVAL_ARTIFACT_PREFIX: ${{ parameters.artifactPrefix }} - ${{ if or(parameters.createDashboardBundle, parameters.publishDashboardResults) }}: - template: ../steps/eval-publish-results.yml diff --git a/eng/common/pipelines/templates/stages/archetype-eval.yml b/eng/common/pipelines/templates/stages/archetype-eval.yml index 8ee61d9878..77aaac1093 100644 --- a/eng/common/pipelines/templates/stages/archetype-eval.yml +++ b/eng/common/pipelines/templates/stages/archetype-eval.yml @@ -43,6 +43,18 @@ parameters: # When true, the invoke step runs under AzureCLI@2 (live MCP inherits the service connection). type: boolean default: false + - name: azureSubscription + # Caller-owned service connection; unused when authentication is disabled. + type: string + default: '' + - name: vallyArguments + # --tag does not filter Prepare's matrix; align evalGlobs so every eval has matching stimuli. + type: object + default: [] + - name: artifactPrefix + # Literal artifact prefix, including any separator (e.g. 'forced-'). + type: string + default: '' - name: threshold # Pass-rate gate forwarded to `vally eval --threshold` and the shard verdict. Repos may override. type: number @@ -100,8 +112,12 @@ extends: - template: /eng/common/pipelines/templates/jobs/eval-shard.yml parameters: vallyRoot: ${{ parameters.vallyRoot }} + artifactPrefix: ${{ parameters.artifactPrefix }} evalInvokeTemplate: ${{ parameters.evalInvokeTemplate }} UseAzSdkAuthentication: ${{ parameters.UseAzSdkAuthentication }} + azureSubscription: ${{ parameters.azureSubscription }} + vallyArguments: ${{ parameters.vallyArguments }} + failOnFailedTests: ${{ parameters.failOnFailedTests }} threshold: ${{ parameters.threshold }} preEvalTemplate: ${{ parameters.preEvalTemplate }} postEvalTemplate: ${{ parameters.postEvalTemplate }} @@ -114,6 +130,7 @@ extends: jobs: - template: /eng/common/pipelines/templates/jobs/eval-summarize.yml parameters: + artifactPrefix: ${{ parameters.artifactPrefix }} failOnFailedTests: ${{ parameters.failOnFailedTests }} createDashboardBundle: ${{ parameters.createDashboardBundle }} publishDashboardResults: ${{ parameters.publishDashboardResults }} diff --git a/eng/common/pipelines/templates/steps/eval-invoke.yml b/eng/common/pipelines/templates/steps/eval-invoke.yml index 191765af3f..2302fb03c3 100644 --- a/eng/common/pipelines/templates/steps/eval-invoke.yml +++ b/eng/common/pipelines/templates/steps/eval-invoke.yml @@ -14,11 +14,24 @@ parameters: type: number default: 0.8 - name: azureSubscription + # Caller-owned service connection; required only for authenticated runs. type: string - default: opensource-api-connection + default: '' + - name: vallyArguments + # Supplementary --tag/--runs/--workers/--timeout argument pairs, 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 0fd3b593a8..75977e3a4a 100644 --- a/eng/common/scripts/eval/README.md +++ b/eng/common/scripts/eval/README.md @@ -13,6 +13,85 @@ 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 | +| --- | --- | +| `matrixStageName` | Stage containing `generate_eval_matrix`; defaults to `Prepare`. The eval stage must depend on it. | +| `artifactPrefix` | Optional literal prefix, e.g. `forced-`. Common adds no separator: `eval-result-forced--` / `eval-debug-forced--` / `eval-summary-forced-`. Empty preserves existing names. Available through the stage and jobs; shard and summary prefixes must match. | +| `preEvalTemplate` / `postEvalTemplate` | Existing parameter-free setup/teardown step templates. | +| `evalInvokeTemplate` | Existing invocation template accepting `vallyRoot`, `UseAzSdkAuthentication` and `threshold`. | +| `azureSubscription` | Caller-owned Azure service connection, forwarded by the stage/job to the common invoke template. Defaults to empty. Required when `UseAzSdkAuthentication: true`; unauthenticated runs need no connection. | +| `vallyArguments` | Optional array of supplementary Vally option/value pairs. Strings are required except that trial/worker counts also accept numbers. Available through the stage, shard job and common invoke template. Defaults to `[]`. | + +Supported supplementary options are `--tag`, `--runs`, `--workers` and `--timeout`. +For example, `['--tag', 'mode=forced', '--runs', '3']` selects a local benchmark +configuration without replacing the common runner. Trial/worker counts must be +positive integer strings or safe integer numbers. Azure template expansion can +serialize numeric parameters as JSON numbers even when using `format`; the runner +validates and converts only `--runs` / `--workers` numeric values to strings. +All other options and values must be strings. Both authentication branches forward the array as JSON in +`VALLY_ARGUMENTS_JSON`; the runner passes literal arguments to the pinned CLI using +Node directly, without a shell or whitespace splitting of supplementary values. +The template JSON-escapes dollar signs before agent macro expansion; the runner's +JSON decoding restores them. Tags such as `build=$(Build.BuildId)` stay literal, +even when the pipeline variable exists. +Unsupported flags (including aliases and `--flag=value`) are rejected. Eval +selection, output paths, JUnit, plugins and verdict settings remain framework-owned. +Legacy custom invoke templates retain their existing parameter contract when the +array is empty. A custom template opting into a non-empty array must declare its +own `vallyArguments` parameter; otherwise template compilation rejects it. + +`--runs` greater than 1 must not be combined with `failOnFailedTests: true`. +Use `failOnFailedTests: false` (the default) for multi-run observations; +explicit `--runs 1` may still be used with the gate enabled. Unsupported +combinations fail the common shard's configuration check before installation or +evaluation. When composing shard and summary jobs directly, pass the same +`failOnFailedTests` value to both. This check covers explicit `--runs` values in +`vallyArguments` only; Summary does not infer run counts from results. Callers must +ensure that repetition configured in `.vally.yaml` or custom invoke templates is +not combined with the enabled gate. + +`--tag` filters stimuli during execution only; it does not filter Prepare's matrix. +The caller must align `evalGlobs` (or its custom matrix) with the tags so every +planned eval retains at least one matching stimulus. For example, `--tag area=github` +alone does not remove other areas from the matrix: also restrict the selected files +to the intended GitHub evals. Recheck this alignment when adding evals or changing tags. +Zero-match evals are not treated as successful skips; a zero-trial shard still fails +the verdict/completeness checks even with `failOnFailedTests: false`. + +Use the existing pre/post-eval and MCP setup hooks for repository-specific work. +Authenticated callers must explicitly configure `azureSubscription`; common has +no deployment-specific connection fallback. An empty or whitespace-only value +fails with a configuration error without creating an `AzureCLI@2` task. +Do not replace authentication, result collection or retry/completeness checks. +Supplementary reports must not rewrite JUnit thresholds or raw results. + +For multiple tracks in one build, use an artifact prefix and configure +the common summary's `matrixStageName`, `evalStageName` and `artifactPrefix` +to match the shard job. Each Summary downloads only result-file patterns matching +its prefix; an empty prefix retains the build-wide `eval-result-*` download. +Each Summary stage must depend on both its Prepare and +Eval stages. Timeline attempts are checked against that exact Eval stage, and +only that track's latest verified attempts are selected. Prefixed tracks +support artifact-only summaries: dashboard bundles/publication are rejected +because the archive identity currently represents one build-wide Summary. +Prefixes and 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; prefixes may also be empty. Include any desired separator +in the prefix: `forced-` and `forced` are used exactly as supplied. Existing shard +names, including names with hyphens, remain supported. + +The caller must ensure final artifact names are unique across all tracks in the +build, including unprefixed tracks. Different prefixes alone do not guarantee this: +`forced-` + `area-foo` and `forced-area-` + `foo` produce the same name. +Common does not perform cross-track collision detection. + ## Complete build results and direct Blob publishing Shards now retain the newest invocation's raw JSONL, JUnit and a completion marker @@ -149,6 +228,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..54b268ea60 100644 --- a/eng/common/scripts/eval/build-eval-summary.ts +++ b/eng/common/scripts/eval/build-eval-summary.ts @@ -379,7 +379,8 @@ async function main(argv) { } } selection = selectSummaryResults({ resultsRoot: options.resultsRoot, - selectedRoot: options.selectedRoot, expectedShards, jobAttempts }); + selectedRoot: options.selectedRoot, expectedShards, jobAttempts, + artifactPrefix: process.env.EVAL_ARTIFACT_PREFIX ?? "" }); if (!matrixValid) { selection.status.push({ shard: "Prepare", attempt: null, complete: false, reason: "The expected shard matrix is unavailable or invalid; check the Prepare stage." }); @@ -413,7 +414,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..bf71969c35 100644 --- a/eng/common/scripts/eval/invoke-eval-shard.ts +++ b/eng/common/scripts/eval/invoke-eval-shard.ts @@ -4,11 +4,49 @@ 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."); + } + // Deliberately exclude eval selection, plugins, output, JUnit and verdict overrides. + const allowed = new Set(["--tag", "--runs", "--workers", "--timeout"]); + const args = []; + for (let index = 0; index < value.length; index += 2) { + const option = value[index]; + if (!allowed.has(option)) { + throw new Error(`Unsupported supplementary Vally option: ${option}`); + } + let argument = value[index + 1]; + const isCount = option === "--runs" || option === "--workers"; + // 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 || argument.startsWith("-")) { + 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); + } + return args; +} /** * Executes the shard and returns the process exit code (0 pass, 1 fail). Side effects @@ -19,11 +57,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] Allowlisted arguments; 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 +74,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 +129,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 +157,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 +169,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..f1251d7805 100644 --- a/eng/common/scripts/eval/lib/shard-results.ts +++ b/eng/common/scripts/eval/lib/shard-results.ts @@ -31,13 +31,14 @@ export function expectedShardsFromMatrix(value) { } // The artifact list cannot reveal a newer retry that lost its agent before publishing. -export function latestShardAttempts(timeline, matrix) { +export function latestShardAttempts(timeline, matrix, evalStageName = "Eval") { expectedShardsFromMatrix(matrix); + if (!/^[A-Za-z_][A-Za-z0-9_]*$/.test(evalStageName)) throw new Error("Invalid eval stage identity."); if (!Array.isArray(timeline?.records)) throw new Error("The current build timeline is required."); const attempts = {}; for (const [key, entry] of Object.entries(matrix)) { if (!validShard(key)) throw new Error("Invalid matrix leg identity."); - const jobs = timeline.records.filter(record => record?.type === "Job" && record.identifier === `Eval.RunShard.${key}`); + const jobs = timeline.records.filter(record => record?.type === "Job" && record.identifier === `${evalStageName}.RunShard.${key}`); for (const record of jobs) { if (!Number.isSafeInteger(record.attempt) || record.attempt < 1) throw new Error("Invalid timeline attempt."); const current = { attempt: record.attempt, complete: record.state === "completed" && @@ -66,7 +67,7 @@ export async function readShardAttempts(env, { fetchImpl = fetch, wait = delay } response = await fetchImpl(url, { headers: { authorization: `Bearer ${env.SYSTEM_ACCESSTOKEN}` }, redirect: "error", signal: AbortSignal.timeout(30_000) }); if (!response.ok) throw new Error("Timeline could not be read."); - return latestShardAttempts(await response.json(), matrix); + return latestShardAttempts(await response.json(), matrix, env.EVAL_STAGE_NAME ?? "Eval"); } catch (error) { if ((response && ![408, 429, 500, 502, 503, 504].includes(response.status)) || attempt === 2) throw error; } finally { await response?.body?.cancel().catch(() => {}); } @@ -177,18 +178,23 @@ export function stageShardResults({ resultsRoot, outputDirectory, shardName, att return metadata; } -export function selectSummaryResults({ resultsRoot, selectedRoot, expectedShards, jobAttempts }) { +export function selectSummaryResults({ resultsRoot, selectedRoot, expectedShards, jobAttempts, artifactPrefix = "" }) { if (!Array.isArray(expectedShards) || expectedShards.some((name) => !validShard(name)) || new Set(expectedShards).size !== expectedShards.length) { throw new Error("Expected shards must be unique, valid shard names from Prepare."); } + if (typeof artifactPrefix !== "string" || (artifactPrefix !== "" && !validShard(artifactPrefix))) { + throw new Error("Invalid artifact prefix."); + } + const resultPrefix = `eval-result-${artifactPrefix}`; 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 +238,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..54deedb0bb 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_ARTIFACT_PREFIX: "", + 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,193 @@ 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("same-build tracks select their own artifacts and authoritative retry evidence", (t) => { + const root = setup(t); + const shard = "area_same"; + const matrix = { leg: { shardName: shard } }; + const timeline = { records: [ + { type: "Job", identifier: "EvalForced.RunShard.leg", attempt: 1, state: "completed", result: "succeeded" }, + { type: "Job", identifier: "EvalForced.RunShard.leg", attempt: 2, state: "completed", result: "failed" }, + { type: "Job", identifier: "EvalTrigger.RunShard.leg", attempt: 3, state: "completed", result: "succeeded" }, + ] }; + for (const [prefix, attempt] of [["forced-", 1], ["forced-", 2], ["trigger-", 3], ["forced-extra-", 4]]) { + const source = invocation(root, `${prefix}${attempt}`); + stageShardResults({ resultsRoot: source, + outputDirectory: path.join(root, "download", `eval-result-${prefix}${shard}-${attempt}`), + shardName: shard, attempt }); + } + const evidence = latestShardAttempts(timeline, matrix, "EvalForced"); + fs.writeFileSync(path.join(root, "job-attempts.json"), JSON.stringify(evidence)); + const result = runSummary(root, [shard], { env: { EVAL_ARTIFACT_PREFIX: "forced-" } }); + 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-forced-${shard}-2`, + }]); + assert.equal(result.result.totals.scenarios, 1); + assert.deepEqual(latestShardAttempts(timeline, matrix, "EvalTrigger").attempts[shard], { attempt: 3, complete: true }); + assert.throws(() => latestShardAttempts(timeline, matrix, "../Eval"), /Invalid eval stage/); + // A newer failed attempt without an artifact must not reuse the older complete run. + const selectedRoot = path.join(root, "selected-missing"); + const missing = selectSummaryResults({ + resultsRoot: path.join(root, "download"), selectedRoot, expectedShards: [shard], + artifactPrefix: "forced-", + jobAttempts: { schemaVersion: 1, valid: true, attempts: { [shard]: { attempt: 4, complete: true } } }, + }); + assert.equal(missing.complete, false); + assert.deepEqual(missing.shardInput.attempts, []); +}); + +test("literal prefixes publish names selected by Summary without adding separators", (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); + const debugName = shardTemplate.match(/^ artifactName: '(eval-debug-[^'\r\n]+)'\r?$/m); + const summaryName = load("eval-summarize").match(/^ artifactName: '(eval-summary-[^'\r\n]+)'\r?$/m); + assert.ok(resultName); + assert.ok(debugName); + assert.ok(summaryName); + for (const prefix of ["", "forced-", "trigger-", "forced", "forced-extra-"]) { + const root = setup(t); + const shard = "extra-area_same"; + const render = expression => expression.replace("${{ parameters.artifactPrefix }}", prefix) + .replace("$(shardName)", shard).replace("$(System.JobAttempt)", "1"); + const name = render(resultName[1]); + assert.equal(name, `eval-result-${prefix}${shard}-1`); + assert.equal(render(debugName[1]), `eval-debug-${prefix}${shard}-1`); + assert.equal(render(summaryName[1]), `eval-summary-${prefix}1`); + const source = invocation(root, "source"); + stageShardResults({ resultsRoot: source, outputDirectory: path.join(root, "download", name), + shardName: shard, attempt: 1 }); + fs.writeFileSync(path.join(root, "job-attempts.json"), JSON.stringify({ + schemaVersion: 1, valid: true, attempts: { [shard]: { attempt: 1, complete: true } }, + })); + const result = runSummary(root, [shard], { env: { EVAL_ARTIFACT_PREFIX: prefix } }); + assert.equal(result.child.status, 0, result.child.stderr); + assert.equal(result.result.complete, true); + assert.deepEqual(result.index.attempts, [{ shard, attempt: 1, directory: name }]); + } +}); + +test("template source defaults to legacy names and forwards a single literal artifact prefix", () => { + const load = name => fs.readFileSync(new URL(`../../../pipelines/templates/${name}.yml`, import.meta.url), "utf8"); + for (const name of ["jobs/eval-shard", "jobs/eval-summarize", "stages/archetype-eval"]) { + const template = load(name); + assert.match(template, /^ - name: artifactPrefix\r?\n(?: #[^\r\n]*\r?\n)* type: string\r?\n default: ''\r?$/m); + assert.doesNotMatch(template, /^ - name: artifactNamespace\r?$/m); + } + const summary = load("jobs/eval-summarize"); + const render = summary.split(/^ - script: \|\r?$/m).find(section => section.includes("build-eval-summary.ts")); + assert.ok(render); + assert.match(render, /^ EVAL_ARTIFACT_PREFIX: \$\{\{ parameters\.artifactPrefix \}\}\r?$/m); + assert.match(summary, /^ - \$\{\{ if and\(ne\(parameters\.artifactPrefix, ''\), or\(parameters\.createDashboardBundle, parameters\.publishDashboardResults\)\) \}\}:\r?$/m); + const stage = load("stages/archetype-eval"); + 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, /^ artifactPrefix: \$\{\{ parameters\.artifactPrefix \}\}\r?$/m); + } +}); + +test("Summary download source applies the literal prefix to all result-file patterns", () => { + 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-\${{ parameters.artifactPrefix }}*/${file}`)); + for (const prefix of ["", "forced-", "trigger-", "forced"]) { + assert.deepEqual(patterns.map(pattern => pattern.replace("${{ parameters.artifactPrefix }}", prefix)), + files.map(file => `eval-result-${prefix}*/${file}`)); + } +}); + +test("empty artifact prefix preserves legacy names and invalid prefixes are rejected", (t) => { + const root = setup(t); + const shard = "area_test"; + artifact(root, shard, 1); + const result = selectSummaryResults({ + resultsRoot: path.join(root, "download"), selectedRoot: path.join(root, "selected"), + expectedShards: [shard], jobAttempts: readJson(path.join(root, "job-attempts.json")), + artifactPrefix: "", + }); + assert.equal(result.complete, true); + assert.deepEqual(result.shardInput.attempts, [{ shard, attempt: 1, directory: `eval-result-${shard}-1` }]); + for (const [index, prefix] of ["../escape", "with spaces", "forced.extra", "/", null, 123, "x".repeat(201)].entries()) { + assert.throws(() => selectSummaryResults({ + resultsRoot: path.join(root, "download"), selectedRoot: path.join(root, `invalid-${index}`), + expectedShards: [shard], jobAttempts: readJson(path.join(root, "job-attempts.json")), + artifactPrefix: prefix, + }), /Invalid artifact prefix/); + } +}); + 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 }); diff --git a/eng/common/scripts/eval/test/verdict.test.ts b/eng/common/scripts/eval/test/verdict.test.ts index fe4978ea5b..3a850dfa2a 100644 --- a/eng/common/scripts/eval/test/verdict.test.ts +++ b/eng/common/scripts/eval/test/verdict.test.ts @@ -5,9 +5,223 @@ 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 { 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("keeps the service connection optional and empty in every common template layer", () => { + for (const template of [invoke, + 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.match(template, /^ - name: UseAzSdkAuthentication\r?\n(?: #[^\r\n]*\r?\n)* type: boolean\r?\n default: false\r?$/m); + } + }); + + 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("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("keeps the existing live entrypoint's connection explicit and forwards caller configuration", () => { + const live = load("../../../pipelines/live-eval.yml"); + assert.match(live, /^ UseAzSdkAuthentication: true\r?$/m); + assert.match(live, /^ azureSubscription: opensource-api-connection\r?$/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 eq\(parameters\.evalInvokeTemplate, '\/eng\/common\/pipelines\/templates\/steps\/eval-invoke\.yml'\) \}\}:\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("accepts only supported option/value pairs without splitting values", () => { + const args = ["--tag", "label=two words; $(not-a-command) & \"quoted\"", "--runs", "3", "--workers", "1", "--timeout", "5m"]; + 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("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 output, verdict, discovery and plugin overrides, including aliases and equals syntax", () => { + for (const option of ["--output-dir", "--output-dir=elsewhere", "--junit", "--no-junit", "--threshold", + "--require-pass", "--skip-grade", "--skip-validate", "-e", "--eval-spec", "--suite", "--work-dir", + "--reporter-plugin", "--executor-plugin", "--grader-plugin", "--runs=3", "--", "-r"]) { + assert.throws(() => validateVallyArguments([option, "value"]), /Unsupported/); + } + for (const args of [null, {}, " --runs 3", [3], ["--runs"], ["--runs", "0"], ["--runs", "-1"], + ["--workers", "1.5"], ["--tag", "--threshold"], ["--tag", "value\0"], + ["--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 = ["--tag", "label=a b; $(echo bad) & \"quotes\" %PATH%", "--tag", "build=$(Build.BuildId)", "--runs", 3]; + 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;