From ee39347184e948f49ba86045f51d10cb63360186 Mon Sep 17 00:00:00 2001 From: Aaron Powell Date: Tue, 23 Jun 2026 16:09:13 +1000 Subject: [PATCH] Migrate external plugin quality gates from skill-validator to vally lint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the downloaded skill-validator binary with px @microsoft/vally-cli lint in the external plugin quality gates pipeline: - Remove downloadSkillValidator() and SKILL_VALIDATOR_ARCHIVE_URL constant - Replace uildSkillValidatorArgs() + unSkillValidatorGate() with uildVallyLintArgs() + unVallyLintGate() that run px vally-cli lint per resolved skill directory (falling back to the full plugin root when no specific skill paths can be resolved from plugin.json) - Rename result keys skill_validator_status / skill_validator_output to ally_lint_status / ally_lint_output throughout both ng/external-plugin-quality-gates.mjs and ng/external-plugin-intake.mjs - Update PR comment markdown to show 'vally lint' instead of 'skill-validator' - Update CONTRIBUTING.md prose references accordingly Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- CONTRIBUTING.md | 4 +- eng/external-plugin-intake.mjs | 16 ++-- eng/external-plugin-quality-gates.mjs | 124 +++++++++----------------- 3 files changed, 52 insertions(+), 92 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9f2c54fd..70c8d680 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -231,7 +231,7 @@ The public-submission policy builds on those rules and also requires `license` p 1. **Open an issue** using the external plugin issue form. Automation applies the `external-plugin` and `awaiting-review` labels. 2. **Automated intake validation** checks that the required fields are present and correctly formatted for a GitHub-hosted plugin. Invalid submissions are labeled `requires-submitter-fixes` with a comment explaining what must be fixed before maintainer review. 3. **Automated quality gates** run after metadata validation: - - `skill-validator check --plugin` against the submitted plugin path/ref/sha + - `vally lint` against the submitted plugin path/ref/sha - install smoke test via Copilot CLI against an ephemeral marketplace entry generated from the submission 4. **Ready for maintainer review**: if metadata validation and quality gates pass, automation removes `awaiting-review` and adds `ready-for-review`. 5. **Submitter-fix blocker**: if metadata is valid but quality gates fail, automation applies `requires-submitter-fixes` instead of advancing to human review. @@ -246,7 +246,7 @@ The public-submission policy builds on those rules and also requires `license` p When a pull request updates `plugins/external.json` (for example, version updates for a previously approved listing), automation runs PR quality checks and posts the result directly on the PR: 1. **Detect changed entries**: automation identifies added/updated external plugin entries in the PR. -2. **Run quality gates**: automation runs install smoke tests and `skill-validator` checks against each changed plugin source ref/SHA/path. +2. **Run quality gates**: automation runs install smoke tests and `vally lint` checks against each changed plugin source ref/SHA/path. 3. **Post source links**: automation updates a bot comment with per-plugin results and direct GitHub tree links to each plugin source location. 4. **Sync workflow-state labels on the PR**: - `ready-for-review` when all checks pass diff --git a/eng/external-plugin-intake.mjs b/eng/external-plugin-intake.mjs index ade914ec..22d551ce 100644 --- a/eng/external-plugin-intake.mjs +++ b/eng/external-plugin-intake.mjs @@ -423,11 +423,11 @@ export function parseMarkReadyForReviewCommand(body) { function normalizeQualityGateResult(rawResult) { const defaults = { overall_status: "not_run", - skill_validator_status: "not_run", + vally_lint_status: "not_run", smoke_status: "not_run", failure_class: "none", summary: "", - skill_validator_output: "", + vally_lint_output: "", smoke_output: "", }; @@ -442,7 +442,7 @@ function normalizeQualityGateResult(rawResult) { } function buildQualityGatesCommentSection(qualityResult) { - const skillState = qualityResult.skill_validator_status || "not_run"; + const vallyState = qualityResult.vally_lint_status || "not_run"; const smokeState = qualityResult.smoke_status || "not_run"; const summaryText = String(qualityResult.summary || "").trim() || "_No quality gate details were provided._"; @@ -451,21 +451,21 @@ function buildQualityGatesCommentSection(qualityResult) { "", "| Gate | Status |", "|---|---|", - `| skill-validator | ${skillState} |`, + `| vally lint | ${vallyState} |`, `| install smoke test | ${smokeState} |`, "", summaryText, ]; - const skillOutput = String(qualityResult.skill_validator_output || "").trim(); - if (skillOutput) { + const vallyOutput = String(qualityResult.vally_lint_output || "").trim(); + if (vallyOutput) { sections.push( "", "
", - "skill-validator output", + "vally lint output", "", "```text", - skillOutput, + vallyOutput, "```", "", "
", diff --git a/eng/external-plugin-quality-gates.mjs b/eng/external-plugin-quality-gates.mjs index 06edfcd3..5a9283e4 100644 --- a/eng/external-plugin-quality-gates.mjs +++ b/eng/external-plugin-quality-gates.mjs @@ -6,7 +6,6 @@ import path from "path"; import { spawnSync } from "child_process"; const MAX_OUTPUT_LENGTH = 12000; -const SKILL_VALIDATOR_ARCHIVE_URL = "https://github.com/dotnet/skills/releases/download/skill-validator-nightly/skill-validator-linux-x64.tar.gz"; const INFRA_ERROR_PATTERNS = [ /\b401\b/, @@ -132,35 +131,10 @@ function cloneSubmissionRepository(workDir, plugin) { return repoDir; } -function downloadSkillValidator(workDir) { - const validatorDir = path.join(workDir, "skill-validator"); - ensureDirectory(validatorDir); - const archivePath = path.join(validatorDir, "skill-validator-linux-x64.tar.gz"); - - const download = runCommand("curl", ["-fsSL", SKILL_VALIDATOR_ARCHIVE_URL, "-o", archivePath]); - if (download.exitCode !== 0) { - throw new Error(`Failed to download skill-validator: ${download.output}`); - } - - const untar = runCommand("tar", ["-xzf", archivePath, "-C", validatorDir]); - if (untar.exitCode !== 0) { - throw new Error(`Failed to extract skill-validator: ${untar.output}`); - } - - const binaryPath = path.join(validatorDir, "skill-validator"); - if (!fs.existsSync(binaryPath)) { - throw new Error("skill-validator binary was not found after extraction"); - } - - runCommand("chmod", ["+x", binaryPath]); - return binaryPath; -} - // Ordered list of candidate locations for plugin.json, from most to least specific. -// The skill-validator --plugin mode expects plugin.json at the plugin root, but -// both the Copilot CLI and many external repos use nested conventions. We read the -// manifest ourselves so skill/agent paths can be resolved from the plugin root -// consistently, regardless of where the manifest lives. +// Both the Copilot CLI and many external repos use nested conventions. We read the +// manifest ourselves so skill paths can be resolved from the plugin root consistently, +// regardless of where the manifest lives. // NOTE: Keep in sync with EXTERNAL_PLUGIN_ROOT_MANIFEST_PATHS in external-plugin-validation.mjs const PLUGIN_JSON_CANDIDATES = [ [".github", "plugin", "plugin.json"], @@ -178,72 +152,58 @@ function findPluginJson(pluginRoot) { return null; } -function buildSkillValidatorArgs(pluginRoot) { +function buildVallyLintArgs(pluginRoot) { const pluginJsonPath = findPluginJson(pluginRoot); if (!pluginJsonPath) { - // No recognised plugin.json location found — let the validator fail with its - // own diagnostic (covers exotic layouts and surfaces the real error to submitters). - return ["check", "--verbose", "--plugin", pluginRoot]; + // No recognised plugin.json location — lint the whole plugin root and let + // vally surface the real error to the submitter. + return [pluginRoot]; } let pluginJson; try { pluginJson = JSON.parse(fs.readFileSync(pluginJsonPath, "utf8")); } catch { - // Malformed plugin.json — let the validator surface the parse error. - return ["check", "--verbose", "--plugin", pluginRoot]; + // Malformed plugin.json — fall back to linting the full root. + return [pluginRoot]; } - const args = ["check", "--verbose"]; - - // Paths in plugin.json are relative to the plugin root regardless of where - // plugin.json itself lives. Use [].concat() to accept both string and array values. + // Collect skill directory paths from plugin.json. const skillPaths = [].concat(pluginJson.skills ?? []) .map((s) => path.resolve(pluginRoot, s)) - .filter((p) => fs.existsSync(p)); - - // Agent entries may be directory paths or explicit file paths; normalise to directories - // so AgentDiscovery.DiscoverAgentsInDirectory can discover agents within them. - // Deduplicate in case multiple file entries share the same parent directory. - const agentPaths = [...new Set( - [].concat(pluginJson.agents ?? []) - .map((a) => { - const resolved = path.resolve(pluginRoot, a); - if (fs.existsSync(resolved) && fs.statSync(resolved).isFile()) { - return path.dirname(resolved); - } - return resolved; - }) - .filter((p) => fs.existsSync(p)) - )]; + .filter((p) => fs.existsSync(p) && fs.statSync(p).isDirectory()); if (skillPaths.length > 0) { - args.push("--skills", ...skillPaths); - } - if (agentPaths.length > 0) { - args.push("--agents", ...agentPaths); + return skillPaths; } - if (skillPaths.length === 0 && agentPaths.length === 0) { - // plugin.json found but no resolvable skills/agents — fall back to --plugin so the - // validator can surface the specific validation error to the submitter. - return ["check", "--verbose", "--plugin", pluginRoot]; - } - - return args; + // No resolvable skill directories — lint the full plugin root so vally can + // surface the specific validation error to the submitter. + return [pluginRoot]; } -function runSkillValidatorGate(workDir, pluginRoot) { +function runVallyLintGate(pluginRoot) { try { - const validatorBinary = downloadSkillValidator(workDir); - const args = buildSkillValidatorArgs(pluginRoot); - const check = runCommand(validatorBinary, args); + const targets = buildVallyLintArgs(pluginRoot); - if (check.exitCode === 0) { - return { status: "pass", output: check.output }; + let combinedOutput = ""; + let anyFailure = false; + + for (const target of targets) { + const check = runCommand( + "npx", + ["--yes", "@microsoft/vally-cli", "lint", target, "--verbose"], + ); + combinedOutput += check.output + "\n"; + if (check.exitCode !== 0) { + anyFailure = true; + } } - return { status: "fail", output: check.output }; + return { + status: anyFailure ? "fail" : "pass", + output: truncateOutput(combinedOutput), + }; } catch (error) { return { status: "infra_error", @@ -362,11 +322,11 @@ export function runExternalPluginQualityGates(plugin) { const workDir = fs.mkdtempSync(path.join(os.tmpdir(), "external-plugin-quality-")); const result = { overall_status: "not_run", - skill_validator_status: "not_run", + vally_lint_status: "not_run", smoke_status: "not_run", failure_class: "none", summary: "", - skill_validator_output: "", + vally_lint_output: "", smoke_output: "", }; @@ -376,7 +336,7 @@ export function runExternalPluginQualityGates(plugin) { const pluginRoot = normalizedPluginPath ? path.join(repoDir, normalizedPluginPath) : repoDir; if (!fs.existsSync(pluginRoot) || !fs.statSync(pluginRoot).isDirectory()) { - result.skill_validator_status = "fail"; + result.vally_lint_status = "fail"; result.smoke_status = "fail"; result.overall_status = "fail"; result.failure_class = "submitter_fixes"; @@ -384,18 +344,18 @@ export function runExternalPluginQualityGates(plugin) { return result; } - const skillResult = runSkillValidatorGate(workDir, pluginRoot); - result.skill_validator_status = skillResult.status; - result.skill_validator_output = skillResult.output; + const vallyResult = runVallyLintGate(pluginRoot); + result.vally_lint_status = vallyResult.status; + result.vally_lint_output = vallyResult.output; const smokeResult = runInstallSmokeGate(workDir, plugin); result.smoke_status = smokeResult.status; result.smoke_output = smokeResult.output; - result.overall_status = toOverallStatus(result.skill_validator_status, result.smoke_status); + result.overall_status = toOverallStatus(result.vally_lint_status, result.smoke_status); result.failure_class = toFailureClass(result.overall_status); result.summary = [ - `- skill-validator: ${result.skill_validator_status}`, + `- vally lint: ${result.vally_lint_status}`, `- install smoke test: ${result.smoke_status}`, `- overall: ${result.overall_status}`, ].join("\n"); @@ -405,7 +365,7 @@ export function runExternalPluginQualityGates(plugin) { result.overall_status = "infra_error"; result.failure_class = "infra"; result.summary = truncateOutput(error.message); - result.skill_validator_output = truncateOutput(error.stack || error.message); + result.vally_lint_output = truncateOutput(error.stack || error.message); return result; } finally { fs.rmSync(workDir, { recursive: true, force: true });