From d6faa1a35b298a22d6648aefccb6001b4471c9b8 Mon Sep 17 00:00:00 2001 From: Aaron Powell Date: Thu, 22 Oct 2026 11:13:43 +1100 Subject: [PATCH] fix(external-plugins): enforce blocking quality gates Require manifest compliance and all applicable checks to pass for submissions and PRs. Fail the PR enforcement job when checks fail or cannot complete. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../external-plugin-command-router.yml | 4 +- .github/workflows/external-plugin-intake.yml | 4 +- ...xternal-plugin-pr-quality-gates-writer.yml | 17 +- .../external-plugin-pr-quality-gates.yml | 33 ++++ CONTRIBUTING.md | 10 +- eng/external-plugin-intake.mjs | 23 ++- eng/external-plugin-intake.test.mjs | 52 +++++++ eng/external-plugin-pr-quality-gates.mjs | 22 ++- eng/external-plugin-quality-gates.mjs | 34 ++-- eng/external-plugin-quality-gates.test.mjs | 146 +++++++++++++++++- 10 files changed, 292 insertions(+), 53 deletions(-) diff --git a/.github/workflows/external-plugin-command-router.yml b/.github/workflows/external-plugin-command-router.yml index b40ded157..ea5f0f14a 100644 --- a/.github/workflows/external-plugin-command-router.yml +++ b/.github/workflows/external-plugin-command-router.yml @@ -807,7 +807,7 @@ jobs: if (process.env.QUALITY_JOB_RESULT === 'failure' || process.env.QUALITY_JOB_RESULT === 'cancelled') { qualityResult = { overall_status: 'infra_error', - spec_compliance_status: 'warning', + spec_compliance_status: 'infra_error', vally_lint_status: 'infra_error', smoke_status: 'infra_error', version_match_status: 'infra_error', @@ -820,7 +820,7 @@ jobs: } else { qualityResult = { overall_status: 'infra_error', - spec_compliance_status: 'warning', + spec_compliance_status: 'infra_error', vally_lint_status: 'infra_error', smoke_status: 'infra_error', version_match_status: 'infra_error', diff --git a/.github/workflows/external-plugin-intake.yml b/.github/workflows/external-plugin-intake.yml index b5b4da006..8b3029bcf 100644 --- a/.github/workflows/external-plugin-intake.yml +++ b/.github/workflows/external-plugin-intake.yml @@ -130,7 +130,7 @@ jobs: if (process.env.QUALITY_JOB_RESULT === 'failure' || process.env.QUALITY_JOB_RESULT === 'cancelled') { qualityResult = { overall_status: 'infra_error', - spec_compliance_status: 'warning', + spec_compliance_status: 'infra_error', vally_lint_status: 'infra_error', smoke_status: 'infra_error', version_match_status: 'infra_error', @@ -143,7 +143,7 @@ jobs: } else { qualityResult = { overall_status: 'infra_error', - spec_compliance_status: 'warning', + spec_compliance_status: 'infra_error', vally_lint_status: 'infra_error', smoke_status: 'infra_error', version_match_status: 'infra_error', diff --git a/.github/workflows/external-plugin-pr-quality-gates-writer.yml b/.github/workflows/external-plugin-pr-quality-gates-writer.yml index a46d541d3..5e1d5d75e 100644 --- a/.github/workflows/external-plugin-pr-quality-gates-writer.yml +++ b/.github/workflows/external-plugin-pr-quality-gates-writer.yml @@ -223,22 +223,19 @@ jobs: }); const checkedPlugins = Array.isArray(qualityResult.checked_plugins) ? qualityResult.checked_plugins.slice(0, 50) : []; - const hasSpecWarnings = checkedPlugins.some((entry) => String(entry?.quality?.spec_compliance_status || '') === 'warning'); const header = qualityResult.failure_class === 'submitter_fixes' ? '## 🛑 External plugin PR checks failed (submitter fixes required)' : qualityResult.overall_status === 'infra_error' ? '## 🛑 External plugin PR checks failed (maintainer follow-up)' - : hasSpecWarnings - ? '## ⚠️ External plugin PR checks passed with spec warnings' - : qualityResult.overall_status === 'pass' || !shouldRun - ? '## ✅ External plugin PR checks passed' - : '## ⚠️ External plugin PR checks need maintainer follow-up'; - const formatStatus = (rawStatus, gateName) => { + : qualityResult.overall_status === 'pass' || !shouldRun + ? '## ✅ External plugin PR checks passed' + : '## ⚠️ External plugin PR checks need maintainer follow-up'; + const formatStatus = (rawStatus) => { const status = String(rawStatus || 'not_run'); if (status === 'pass') { return '✅ pass'; } - if (status === 'warning' || (gateName === 'spec compliance' && status === 'fail')) { + if (status === 'warning') { return '⚠️ warning'; } if (status === 'fail' || status === 'infra_error') { @@ -322,7 +319,7 @@ jobs: const failureDetails = checkedPlugins.flatMap((entry) => { const name = String(entry?.name || 'unknown'); const quality = entry?.quality || {}; - const shouldShowSpec = quality.spec_compliance_status === 'warning' || String(quality.spec_compliance_output || '').trim().length > 0; + const shouldShowSpec = quality.spec_compliance_status === 'fail' || quality.spec_compliance_status === 'infra_error' || String(quality.spec_compliance_output || '').trim().length > 0; const shouldShowVally = quality.vally_lint_status === 'fail' || quality.vally_lint_status === 'infra_error' || String(quality.vally_lint_output || '').trim().length > 0; const shouldShowSmoke = quality.smoke_status === 'fail' || quality.smoke_status === 'infra_error' || String(quality.smoke_output || '').trim().length > 0; const shouldShowVersionMatch = quality.version_match_status === 'fail' || quality.version_match_status === 'infra_error' || String(quality.version_match_output || '').trim().length > 0; @@ -362,7 +359,7 @@ jobs: '', '### Per-plugin quality summary', '', - '| Plugin | spec compliance (non-blocking) | vally lint | install smoke test | version match | ref/sha consistency | canvas structure | overall | source tree |', + '| Plugin | spec compliance | vally lint | install smoke test | version match | ref/sha consistency | canvas structure | overall | source tree |', '|---|---|---|---|---|---|---|---|---|', ...rows, '', diff --git a/.github/workflows/external-plugin-pr-quality-gates.yml b/.github/workflows/external-plugin-pr-quality-gates.yml index b99c8c0bc..9d6d8d275 100644 --- a/.github/workflows/external-plugin-pr-quality-gates.yml +++ b/.github/workflows/external-plugin-pr-quality-gates.yml @@ -183,3 +183,36 @@ jobs: path: ${{ runner.temp }}/external-plugin-pr-quality-result/result.json if-no-files-found: error retention-days: 3 + + enforce-quality-gates: + runs-on: ubuntu-latest + needs: [detect-changed-plugins, run-quality-gates] + if: always() + steps: + - name: Require all external plugin quality gates to pass + env: + DETECT_JOB_RESULT: ${{ needs.detect-changed-plugins.result }} + SHOULD_RUN: ${{ needs.detect-changed-plugins.outputs.should-run }} + QUALITY_JOB_RESULT: ${{ needs.run-quality-gates.result }} + QUALITY_RESULT_JSON: ${{ needs.run-quality-gates.outputs.quality-result }} + run: | + if [ "$DETECT_JOB_RESULT" != "success" ]; then + echo "::error::External plugin change detection did not succeed ($DETECT_JOB_RESULT)." + exit 1 + fi + if [ "$SHOULD_RUN" != "true" ]; then + echo "No changed external plugin entries; nothing to enforce." + exit 0 + fi + if [ "$QUALITY_JOB_RESULT" != "success" ]; then + echo "::error::External plugin quality gates did not complete ($QUALITY_JOB_RESULT)." + exit 1 + fi + node -e ' + const result = JSON.parse(process.env.QUALITY_RESULT_JSON || "{}"); + console.log(result.summary || "No summary provided."); + if (result.overall_status !== "pass") { + console.log(`::error::All external plugin quality gates must pass (overall: ${result.overall_status || "unknown"}). See the PR comment for details.`); + process.exit(1); + } + ' diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index daaa83181..c6a44a5ab 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -255,8 +255,13 @@ The public-submission policy builds on those rules and also requires `license` p 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. Intake also checks the repository owner's type using the GitHub API. Organization-owned repositories receive `first party` for the `Microsoft`, `GitHub`, `Azure`, `dotnet`, `MicrosoftDocs`, or `microsoftgraph` organizations (case-insensitive), or `third party` for any other organization. User-owned repositories receive neither label. On the triage board, `first party` skips AI review and creates an `Accept` recommendation with `/approve`; this is only a triage recommendation. Submissions must still pass automated validation and quality gates, and a human maintainer must review and approve them before listing. Ownership labels are updated on each intake pass. 3. **Automated quality gates** run after metadata validation: - - `vally lint` against the submitted plugin path/ref/sha + - Agent Plugins v1.0.0 manifest compliance, including a root `plugin.json` with the required schema and supported fields + - `vally lint` against the submitted plugin path/ref/sha, including Agent Skills spec compliance and valid file references for bundled skills - install smoke test via Copilot CLI against an ephemeral marketplace entry generated from the submission + - version matching against the source manifest, ref/SHA consistency when both locators are provided, and the required extension structure for plugins tagged `canvas` + + All applicable quality gates must pass. Spec compliance violations and lint failures are blocking errors, not warnings. Infrastructure errors keep the submission in `awaiting-review` until checks can complete or a maintainer explicitly overrides the blocker. + 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. 6. **Requesting another intake pass**: after updating the issue body or source plugin, the issue author or a maintainer can comment `/rerun-intake` to re-run automated intake and quality gates on demand while the issue is still open. Open issues re-trigger intake automatically on edit. When the rerun is accepted, automation reacts to the command comment with 👀 so it is visible that processing started. @@ -270,12 +275,13 @@ 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 `vally lint` checks against each changed plugin source ref/SHA/path. +2. **Run quality gates**: automation runs the same blocking quality gates as submission intake against each changed plugin source ref/SHA/path, including manifest compliance, Agent Skills spec compliance via `vally lint`, and install smoke tests. 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 - `requires-submitter-fixes` when quality checks fail due to plugin issues - `awaiting-review` when checks cannot complete because of infrastructure/transient errors +5. **Fail the PR status check**: the `enforce-quality-gates` job fails unless every changed plugin passes every applicable gate, so the PR shows a failing check (blocking merge when configured as a required status check) while any gate is failing or incomplete. ##### Website README diff --git a/eng/external-plugin-intake.mjs b/eng/external-plugin-intake.mjs index b1418c4eb..5d55628b8 100644 --- a/eng/external-plugin-intake.mjs +++ b/eng/external-plugin-intake.mjs @@ -1072,13 +1072,13 @@ function normalizeQualityGateResult(rawResult) { } function buildQualityGatesCommentSection(qualityResult) { - const formatStatus = (rawStatus, gate) => { + const formatStatus = (rawStatus) => { const status = String(rawStatus || "not_run"); if (status === "pass") { return "✅ pass"; } - if (status === "warning" || (gate === "spec" && status === "fail")) { + if (status === "warning") { return "⚠️ warning"; } if (status === "fail" || status === "infra_error") { @@ -1102,7 +1102,7 @@ function buildQualityGatesCommentSection(qualityResult) { "", "| Gate | Status |", "|---|---|", - `| spec compliance (non-blocking) | ${formatStatus(specState, "spec")} |`, + `| spec compliance | ${formatStatus(specState, "spec")} |`, `| vally lint | ${formatStatus(vallyState, "vally")} |`, `| install smoke test | ${formatStatus(smokeState, "smoke")} |`, `| version match | ${formatStatus(versionMatchState, "version match")} |`, @@ -1259,7 +1259,7 @@ function getIntakeStateFromQualityResult(baseResult, qualityResult) { return "awaiting-review"; } - return "ready-for-review"; + return qualityResult.overall_status === "pass" ? "ready-for-review" : "awaiting-review"; } function buildMergedIntakeComment(baseResult, qualityResult, runId, owner, repo, qualityLogUrl) { @@ -1274,24 +1274,19 @@ function buildMergedIntakeComment(baseResult, qualityResult, runId, owner, repo, ? `_[Download full quality gate logs](${qualityLogUrl})_` : ""; - const hasSpecWarnings = String(qualityResult.spec_compliance_status || "") === "warning"; const intro = qualityResult.failure_class === "submitter_fixes" ? "## 🛑 External plugin intake failed (submitter fixes required)" - : qualityResult.failure_class === "infra" + : qualityResult.overall_status !== "pass" ? "## 🛑 External plugin intake failed (quality checks could not complete)" - : hasSpecWarnings - ? "## ⚠️ External plugin intake passed with spec warnings" - : "## ✅ External plugin intake passed"; + : "## ✅ External plugin intake passed"; const statusLine = qualityResult.failure_class === "submitter_fixes" ? "This submission passed metadata validation, but quality gates found issues that must be fixed before it can move to maintainer review. Update the issue details or source plugin and then comment `/rerun-intake`." - : qualityResult.failure_class === "infra" - ? "This submission passed metadata validation, but the automated quality checks hit an infrastructure issue. A maintainer should rerun intake or use the explicit override command after review." - : hasSpecWarnings - ? "This submission passed blocking quality checks and is ready for maintainer review, but it has non-blocking Agent Plugins spec compliance warnings." - : "This submission passed automated intake validation and quality checks and is ready for maintainer review."; + : qualityResult.overall_status !== "pass" + ? "This submission passed metadata validation, but the automated quality checks did not complete successfully. A maintainer should rerun intake or use the explicit override command after review." + : "This submission passed automated intake validation and quality checks and is ready for maintainer review."; return [ marker, diff --git a/eng/external-plugin-intake.test.mjs b/eng/external-plugin-intake.test.mjs index 0f8e06809..9733cf129 100644 --- a/eng/external-plugin-intake.test.mjs +++ b/eng/external-plugin-intake.test.mjs @@ -16,6 +16,58 @@ const TREE_PLUGINS = "tree-plugins"; const TREE_UPGRADE_AGENT = "tree-upgrade-agent"; const TREE_EXTENSIONS = "tree-extensions"; +const validBaseEvaluation = { + valid: true, + plugin: { + name: "example-plugin", + repository: "https://github.com/example/plugin", + source: { repo: "example/plugin", ref: "v1.0.0" }, + }, + warnings: [], +}; + +test("applyQualityGateResult blocks spec failures and displays them as failures", () => { + const result = applyQualityGateResult(validBaseEvaluation, { + overall_status: "fail", + failure_class: "submitter_fixes", + spec_compliance_status: "fail", + spec_compliance_output: "Agent Plugins v1.0.0 manifest errors", + vally_lint_status: "pass", + smoke_status: "pass", + }); + + assert.equal(result.intakeState, "requires-submitter-fixes"); + assert.match(result.commentBody, /intake failed \(submitter fixes required\)/); + assert.match(result.commentBody, /\| spec compliance \| 🛑 fail \|/); + assert.doesNotMatch(result.commentBody, /non-blocking|passed with spec warnings/); +}); + +test("applyQualityGateResult advances successful quality checks", () => { + const result = applyQualityGateResult(validBaseEvaluation, { + overall_status: "pass", + failure_class: "none", + spec_compliance_status: "pass", + vally_lint_status: "pass", + smoke_status: "pass", + }); + + assert.equal(result.intakeState, "ready-for-review"); + assert.match(result.commentBody, /intake passed/); +}); + +for (const quality of [undefined, {}, { overall_status: "not_run" }, { + overall_status: "infra_error", + failure_class: "infra", + spec_compliance_status: "infra_error", +}]) { + test(`applyQualityGateResult keeps incomplete checks awaiting review (${JSON.stringify(quality)})`, () => { + const result = applyQualityGateResult(validBaseEvaluation, quality); + assert.equal(result.intakeState, "awaiting-review"); + assert.match(result.commentBody, /quality checks could not complete/); + assert.doesNotMatch(result.commentBody, /intake passed/); + }); +} + test("applyQualityGateResult links the full quality log artifact", () => { const artifactUrl = "https://github.com/github/awesome-copilot/actions/runs/123/artifacts/456"; const result = applyQualityGateResult( diff --git a/eng/external-plugin-pr-quality-gates.mjs b/eng/external-plugin-pr-quality-gates.mjs index 50d193478..25d59ba47 100644 --- a/eng/external-plugin-pr-quality-gates.mjs +++ b/eng/external-plugin-pr-quality-gates.mjs @@ -41,25 +41,34 @@ export function buildSourceTreeUrl(plugin) { return `https://github.com/${sourceRepo}/tree/${encodedLocator}/${encodedPath}`; } -function aggregateResultStatus(pluginResults) { - if (pluginResults.some((entry) => entry.quality?.overall_status === "fail")) { +export function aggregateResultStatus(pluginResults) { + if (pluginResults.length === 0) { + return { + overallStatus: "not_run", + failureClass: "none", + }; + } + + const statuses = pluginResults.map((entry) => entry.quality?.overall_status); + if (statuses.includes("fail")) { return { overallStatus: "fail", failureClass: "submitter_fixes", }; } - if (pluginResults.some((entry) => entry.quality?.overall_status === "infra_error")) { + if (statuses.includes("infra_error")) { return { overallStatus: "infra_error", failureClass: "infra", }; } - if (pluginResults.length === 0) { + // Every changed plugin must fully pass; anything else (warnings, not_run, unknown) blocks. + if (statuses.some((status) => status !== "pass")) { return { - overallStatus: "not_run", - failureClass: "none", + overallStatus: "fail", + failureClass: "submitter_fixes", }; } @@ -73,6 +82,7 @@ function createValidationFailureQuality(errors) { const output = errors.map((error) => `- ${error}`).join("\n"); return { overall_status: "fail", + spec_compliance_status: "not_run", vally_lint_status: "fail", smoke_status: "not_run", version_match_status: "not_run", diff --git a/eng/external-plugin-quality-gates.mjs b/eng/external-plugin-quality-gates.mjs index 3d56b8922..c1f2be03c 100644 --- a/eng/external-plugin-quality-gates.mjs +++ b/eng/external-plugin-quality-gates.mjs @@ -194,11 +194,11 @@ function findPluginJson(pluginRoot) { return null; } -function inspectAgentPluginSpecCompliance(pluginRoot) { +export function inspectAgentPluginSpecCompliance(pluginRoot) { const pluginJsonPath = findPluginJson(pluginRoot); if (!pluginJsonPath) { return { - status: "warning", + status: "fail", output: "No plugin.json found in a recognized location. Agent Plugins v1.0.0 expects plugin.json at the plugin root.", }; } @@ -214,7 +214,7 @@ function inspectAgentPluginSpecCompliance(pluginRoot) { manifest = JSON.parse(fs.readFileSync(pluginJsonPath, "utf8")); } catch (error) { return { - status: "warning", + status: "fail", output: `plugin.json is not valid JSON: ${error.message}`, }; } @@ -306,9 +306,9 @@ function inspectAgentPluginSpecCompliance(pluginRoot) { } return { - status: "warning", + status: "fail", output: [ - "Agent Plugins v1.0.0 manifest warnings:", + "Agent Plugins v1.0.0 manifest errors:", ...issues.map((issue) => `- ${issue}`), ].join("\n"), }; @@ -344,7 +344,7 @@ function buildVallyLintArgs(pluginRoot) { return [pluginRoot]; } -async function runVallyLintGate(pluginRoot) { +export async function runVallyLintGate(pluginRoot) { try { const targets = buildVallyLintArgs(pluginRoot); @@ -961,11 +961,19 @@ export function runCanvasStructureGate(repoDir, plugin, primaryFetchSpec) { }; } -function toOverallStatus(states) { +export function toOverallStatus(result) { + const states = [ + result.spec_compliance_status, + result.vally_lint_status, + result.smoke_status, + result.version_match_status, + result.ref_sha_consistency_status, + result.canvas_structure_status, + ]; if (states.includes("infra_error")) { return "infra_error"; } - if (states.includes("fail")) { + if (states.some((state) => state !== "pass" && state !== "not_run")) { return "fail"; } if (states.every((state) => state === "not_run")) { @@ -1013,7 +1021,7 @@ export async function runExternalPluginQualityGates(plugin, { logFile } = {}) { if (!fs.existsSync(pluginRoot) || !fs.statSync(pluginRoot).isDirectory()) { result.vally_lint_status = "fail"; result.smoke_status = "fail"; - result.spec_compliance_status = "warning"; + result.spec_compliance_status = "fail"; result.version_match_status = "fail"; result.ref_sha_consistency_status = "not_run"; result.canvas_structure_status = hasCanvasKeyword(plugin) ? "fail" : "not_run"; @@ -1053,13 +1061,7 @@ export async function runExternalPluginQualityGates(plugin, { logFile } = {}) { result.smoke_status = smokeResult.status; result.smoke_output = smokeResult.output; - result.overall_status = toOverallStatus([ - result.vally_lint_status, - result.smoke_status, - result.version_match_status, - result.ref_sha_consistency_status, - result.canvas_structure_status, - ]); + result.overall_status = toOverallStatus(result); result.failure_class = toFailureClass(result.overall_status); result.summary = [ `- spec compliance: ${result.spec_compliance_status}`, diff --git a/eng/external-plugin-quality-gates.test.mjs b/eng/external-plugin-quality-gates.test.mjs index cc6e706bf..03c4b9aa0 100644 --- a/eng/external-plugin-quality-gates.test.mjs +++ b/eng/external-plugin-quality-gates.test.mjs @@ -6,11 +6,17 @@ import { spawnSync } from "child_process"; import { after, test } from "node:test"; import { formatQualityGateLog, + inspectAgentPluginSpecCompliance, runCanvasStructureGate, runRefShaConsistencyGate, + runVallyLintGate, runVersionMatchGate, + toOverallStatus, } from "./external-plugin-quality-gates.mjs"; -import { runExternalPluginPrQualityGates } from "./external-plugin-pr-quality-gates.mjs"; +import { + aggregateResultStatus, + runExternalPluginPrQualityGates, +} from "./external-plugin-pr-quality-gates.mjs"; const tempDirs = []; @@ -44,6 +50,116 @@ function commitAll(repoDir, message) { return runGit(repoDir, "rev-parse", "HEAD"); } +function createPluginFixture() { + const pluginRoot = fs.mkdtempSync(path.join(os.tmpdir(), "external-plugin-lint-")); + tempDirs.push(pluginRoot); + fs.writeFileSync(path.join(pluginRoot, "plugin.json"), JSON.stringify({ + $schema: "https://agent-plugins.org/schemas/1.0.0/plugin.schema.json", + name: "example-plugin", + version: "1.0.0", + description: "An example plugin for quality gate tests.", + })); + return pluginRoot; +} + +function writeSkillFixture(pluginRoot, { name = "example-skill", body = "Follow these instructions." } = {}) { + const skillRoot = path.join(pluginRoot, "skills", "example-skill"); + fs.mkdirSync(skillRoot, { recursive: true }); + fs.writeFileSync(path.join(skillRoot, "SKILL.md"), [ + "---", + `name: ${name}`, + "description: 'An example skill for quality gate tests.'", + "---", + body, + "", + ].join("\n")); +} + +test("manifest compliance passes for an Agent Plugins v1.0.0 manifest", () => { + const result = inspectAgentPluginSpecCompliance(createPluginFixture()); + assert.equal(result.status, "pass", result.output); +}); + +for (const [name, update, expectedOutput] of [ + ["missing manifest", (root) => fs.unlinkSync(path.join(root, "plugin.json")), /No plugin.json found/], + ["legacy manifest location", (root) => { + fs.mkdirSync(path.join(root, ".github", "plugin"), { recursive: true }); + fs.renameSync(path.join(root, "plugin.json"), path.join(root, ".github", "plugin", "plugin.json")); + }, /expected "plugin.json" at plugin root/], + ["invalid JSON", (root) => fs.writeFileSync(path.join(root, "plugin.json"), "{"), /not valid JSON/], + ["invalid manifest type", (root) => fs.writeFileSync(path.join(root, "plugin.json"), "null"), /must be a JSON object/], + ["noncompliant fields", (root) => fs.writeFileSync(path.join(root, "plugin.json"), JSON.stringify({ + name: "Invalid Name", + skills: ["./skills"], + })), /manifest errors/], +]) { + test(`manifest compliance blocks ${name} instead of warning`, () => { + const pluginRoot = createPluginFixture(); + update(pluginRoot); + const result = inspectAgentPluginSpecCompliance(pluginRoot); + assert.equal(result.status, "fail", result.output); + assert.match(result.output, expectedOutput); + }); +} + +const passingQualityResult = { + spec_compliance_status: "pass", + vally_lint_status: "pass", + smoke_status: "pass", + version_match_status: "pass", + ref_sha_consistency_status: "not_run", + canvas_structure_status: "not_run", +}; + +test("overall quality passes when all applicable gates pass", () => { + assert.equal(toOverallStatus(passingQualityResult), "pass"); +}); + +for (const gate of Object.keys(passingQualityResult)) { + for (const status of ["fail", "warning", "infra_error"]) { + test(`overall quality blocks ${gate}=${status}`, () => { + assert.equal( + toOverallStatus({ ...passingQualityResult, [gate]: status }), + status === "infra_error" ? "infra_error" : "fail", + ); + }); + } +} + +test("overall quality does not pass when every gate was skipped", () => { + assert.equal(toOverallStatus(Object.fromEntries( + Object.keys(passingQualityResult).map((gate) => [gate, "not_run"]), + )), "not_run"); +}); + +test("vally lint passes for compliant bundled skills", async () => { + const pluginRoot = createPluginFixture(); + writeSkillFixture(pluginRoot); + const result = await runVallyLintGate(pluginRoot); + assert.equal(result.status, "pass", result.output); +}); + +test("vally lint blocks Agent Skills spec violations", async () => { + const pluginRoot = createPluginFixture(); + writeSkillFixture(pluginRoot, { name: "Invalid-Name" }); + const result = await runVallyLintGate(pluginRoot); + assert.equal(result.status, "fail", result.output); + assert.match(result.logOutput, /spec-compliance/); +}); + +test("vally lint blocks broken skill file references", async () => { + const pluginRoot = createPluginFixture(); + writeSkillFixture(pluginRoot, { body: "Read [the reference](references/missing.md)." }); + const result = await runVallyLintGate(pluginRoot); + assert.equal(result.status, "fail", result.output); + assert.match(result.logOutput, /missing/); +}); + +test("vally lint allows plugins without skills", async () => { + const result = await runVallyLintGate(createPluginFixture()); + assert.equal(result.status, "pass", result.output); +}); + test("formatQualityGateLog preserves full vally output for artifacts", () => { const fullVallyOutput = `lint failure\n${"x".repeat(13000)}`; const log = formatQualityGateLog( @@ -386,3 +502,31 @@ test("runRefShaConsistencyGate passes when ref and sha point to the same commit" const result = runRefShaConsistencyGate(repoDir, plugin, sha); assert.equal(result.status, "pass", result.output); }); + +test("aggregateResultStatus requires every changed plugin to pass", () => { + const entry = (status) => ({ quality: { overall_status: status } }); + + assert.deepEqual(aggregateResultStatus([]), { overallStatus: "not_run", failureClass: "none" }); + assert.deepEqual(aggregateResultStatus([entry("pass"), entry("pass")]), { + overallStatus: "pass", + failureClass: "none", + }); + assert.deepEqual(aggregateResultStatus([entry("pass"), entry("fail")]), { + overallStatus: "fail", + failureClass: "submitter_fixes", + }); + assert.deepEqual(aggregateResultStatus([entry("pass"), entry("infra_error")]), { + overallStatus: "infra_error", + failureClass: "infra", + }); + assert.deepEqual(aggregateResultStatus([entry("fail"), entry("infra_error")]), { + overallStatus: "fail", + failureClass: "submitter_fixes", + }); + for (const status of ["warning", "not_run", undefined]) { + assert.deepEqual(aggregateResultStatus([entry("pass"), entry(status)]), { + overallStatus: "fail", + failureClass: "submitter_fixes", + }); + } +}); \ No newline at end of file