Migrate external plugin quality gates from skill-validator to vally lint

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>
This commit is contained in:
Aaron Powell
2026-06-23 16:09:13 +10:00
parent 3101aed8bf
commit ee39347184
3 changed files with 52 additions and 92 deletions
+2 -2
View File
@@ -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. 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. 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: 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 - 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`. 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. 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: 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. 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. 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**: 4. **Sync workflow-state labels on the PR**:
- `ready-for-review` when all checks pass - `ready-for-review` when all checks pass
+8 -8
View File
@@ -423,11 +423,11 @@ export function parseMarkReadyForReviewCommand(body) {
function normalizeQualityGateResult(rawResult) { function normalizeQualityGateResult(rawResult) {
const defaults = { const defaults = {
overall_status: "not_run", overall_status: "not_run",
skill_validator_status: "not_run", vally_lint_status: "not_run",
smoke_status: "not_run", smoke_status: "not_run",
failure_class: "none", failure_class: "none",
summary: "", summary: "",
skill_validator_output: "", vally_lint_output: "",
smoke_output: "", smoke_output: "",
}; };
@@ -442,7 +442,7 @@ function normalizeQualityGateResult(rawResult) {
} }
function buildQualityGatesCommentSection(qualityResult) { 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 smokeState = qualityResult.smoke_status || "not_run";
const summaryText = String(qualityResult.summary || "").trim() || "_No quality gate details were provided._"; const summaryText = String(qualityResult.summary || "").trim() || "_No quality gate details were provided._";
@@ -451,21 +451,21 @@ function buildQualityGatesCommentSection(qualityResult) {
"", "",
"| Gate | Status |", "| Gate | Status |",
"|---|---|", "|---|---|",
`| skill-validator | ${skillState} |`, `| vally lint | ${vallyState} |`,
`| install smoke test | ${smokeState} |`, `| install smoke test | ${smokeState} |`,
"", "",
summaryText, summaryText,
]; ];
const skillOutput = String(qualityResult.skill_validator_output || "").trim(); const vallyOutput = String(qualityResult.vally_lint_output || "").trim();
if (skillOutput) { if (vallyOutput) {
sections.push( sections.push(
"", "",
"<details>", "<details>",
"<summary>skill-validator output</summary>", "<summary>vally lint output</summary>",
"", "",
"```text", "```text",
skillOutput, vallyOutput,
"```", "```",
"", "",
"</details>", "</details>",
+42 -82
View File
@@ -6,7 +6,6 @@ import path from "path";
import { spawnSync } from "child_process"; import { spawnSync } from "child_process";
const MAX_OUTPUT_LENGTH = 12000; 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 = [ const INFRA_ERROR_PATTERNS = [
/\b401\b/, /\b401\b/,
@@ -132,35 +131,10 @@ function cloneSubmissionRepository(workDir, plugin) {
return repoDir; 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. // 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
// 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,
// manifest ourselves so skill/agent paths can be resolved from the plugin root // regardless of where the manifest lives.
// consistently, regardless of where the manifest lives.
// NOTE: Keep in sync with EXTERNAL_PLUGIN_ROOT_MANIFEST_PATHS in external-plugin-validation.mjs // NOTE: Keep in sync with EXTERNAL_PLUGIN_ROOT_MANIFEST_PATHS in external-plugin-validation.mjs
const PLUGIN_JSON_CANDIDATES = [ const PLUGIN_JSON_CANDIDATES = [
[".github", "plugin", "plugin.json"], [".github", "plugin", "plugin.json"],
@@ -178,72 +152,58 @@ function findPluginJson(pluginRoot) {
return null; return null;
} }
function buildSkillValidatorArgs(pluginRoot) { function buildVallyLintArgs(pluginRoot) {
const pluginJsonPath = findPluginJson(pluginRoot); const pluginJsonPath = findPluginJson(pluginRoot);
if (!pluginJsonPath) { if (!pluginJsonPath) {
// No recognised plugin.json location found — let the validator fail with its // No recognised plugin.json location — lint the whole plugin root and let
// own diagnostic (covers exotic layouts and surfaces the real error to submitters). // vally surface the real error to the submitter.
return ["check", "--verbose", "--plugin", pluginRoot]; return [pluginRoot];
} }
let pluginJson; let pluginJson;
try { try {
pluginJson = JSON.parse(fs.readFileSync(pluginJsonPath, "utf8")); pluginJson = JSON.parse(fs.readFileSync(pluginJsonPath, "utf8"));
} catch { } catch {
// Malformed plugin.json — let the validator surface the parse error. // Malformed plugin.json — fall back to linting the full root.
return ["check", "--verbose", "--plugin", pluginRoot]; return [pluginRoot];
} }
const args = ["check", "--verbose"]; // Collect skill directory paths from plugin.json.
// 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.
const skillPaths = [].concat(pluginJson.skills ?? []) const skillPaths = [].concat(pluginJson.skills ?? [])
.map((s) => path.resolve(pluginRoot, s)) .map((s) => path.resolve(pluginRoot, s))
.filter((p) => fs.existsSync(p)); .filter((p) => fs.existsSync(p) && fs.statSync(p).isDirectory());
// 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))
)];
if (skillPaths.length > 0) { if (skillPaths.length > 0) {
args.push("--skills", ...skillPaths); return skillPaths;
}
if (agentPaths.length > 0) {
args.push("--agents", ...agentPaths);
} }
if (skillPaths.length === 0 && agentPaths.length === 0) { // No resolvable skill directories — lint the full plugin root so vally can
// plugin.json found but no resolvable skills/agents — fall back to --plugin so the // surface the specific validation error to the submitter.
// validator can surface the specific validation error to the submitter. return [pluginRoot];
return ["check", "--verbose", "--plugin", pluginRoot];
}
return args;
} }
function runSkillValidatorGate(workDir, pluginRoot) { function runVallyLintGate(pluginRoot) {
try { try {
const validatorBinary = downloadSkillValidator(workDir); const targets = buildVallyLintArgs(pluginRoot);
const args = buildSkillValidatorArgs(pluginRoot);
const check = runCommand(validatorBinary, args);
if (check.exitCode === 0) { let combinedOutput = "";
return { status: "pass", output: check.output }; 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) { } catch (error) {
return { return {
status: "infra_error", status: "infra_error",
@@ -362,11 +322,11 @@ export function runExternalPluginQualityGates(plugin) {
const workDir = fs.mkdtempSync(path.join(os.tmpdir(), "external-plugin-quality-")); const workDir = fs.mkdtempSync(path.join(os.tmpdir(), "external-plugin-quality-"));
const result = { const result = {
overall_status: "not_run", overall_status: "not_run",
skill_validator_status: "not_run", vally_lint_status: "not_run",
smoke_status: "not_run", smoke_status: "not_run",
failure_class: "none", failure_class: "none",
summary: "", summary: "",
skill_validator_output: "", vally_lint_output: "",
smoke_output: "", smoke_output: "",
}; };
@@ -376,7 +336,7 @@ export function runExternalPluginQualityGates(plugin) {
const pluginRoot = normalizedPluginPath ? path.join(repoDir, normalizedPluginPath) : repoDir; const pluginRoot = normalizedPluginPath ? path.join(repoDir, normalizedPluginPath) : repoDir;
if (!fs.existsSync(pluginRoot) || !fs.statSync(pluginRoot).isDirectory()) { if (!fs.existsSync(pluginRoot) || !fs.statSync(pluginRoot).isDirectory()) {
result.skill_validator_status = "fail"; result.vally_lint_status = "fail";
result.smoke_status = "fail"; result.smoke_status = "fail";
result.overall_status = "fail"; result.overall_status = "fail";
result.failure_class = "submitter_fixes"; result.failure_class = "submitter_fixes";
@@ -384,18 +344,18 @@ export function runExternalPluginQualityGates(plugin) {
return result; return result;
} }
const skillResult = runSkillValidatorGate(workDir, pluginRoot); const vallyResult = runVallyLintGate(pluginRoot);
result.skill_validator_status = skillResult.status; result.vally_lint_status = vallyResult.status;
result.skill_validator_output = skillResult.output; result.vally_lint_output = vallyResult.output;
const smokeResult = runInstallSmokeGate(workDir, plugin); const smokeResult = runInstallSmokeGate(workDir, plugin);
result.smoke_status = smokeResult.status; result.smoke_status = smokeResult.status;
result.smoke_output = smokeResult.output; 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.failure_class = toFailureClass(result.overall_status);
result.summary = [ result.summary = [
`- skill-validator: ${result.skill_validator_status}`, `- vally lint: ${result.vally_lint_status}`,
`- install smoke test: ${result.smoke_status}`, `- install smoke test: ${result.smoke_status}`,
`- overall: ${result.overall_status}`, `- overall: ${result.overall_status}`,
].join("\n"); ].join("\n");
@@ -405,7 +365,7 @@ export function runExternalPluginQualityGates(plugin) {
result.overall_status = "infra_error"; result.overall_status = "infra_error";
result.failure_class = "infra"; result.failure_class = "infra";
result.summary = truncateOutput(error.message); 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; return result;
} finally { } finally {
fs.rmSync(workDir, { recursive: true, force: true }); fs.rmSync(workDir, { recursive: true, force: true });