From 3bd5c7e4c9b27c5eb8e8999b1d2589e360a7c65f Mon Sep 17 00:00:00 2001 From: Aaron Powell Date: Tue, 3 Nov 2026 10:04:36 +1100 Subject: [PATCH] fix(plugins): prefer root manifest over legacy locations Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- CONTRIBUTING.md | 2 +- eng/external-plugin-quality-gates.mjs | 4 +- eng/external-plugin-quality-gates.test.mjs | 66 ++++++++++++++++++++++ 3 files changed, 69 insertions(+), 3 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3802ffa656..7df81e5dcd 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -255,7 +255,7 @@ 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: - - Agent Plugins v1.0.0 manifest compliance, including a root `plugin.json` with the required schema and supported fields + - Agent Plugins v1.0.0 manifest compliance, including a root `plugin.json` with the required schema and supported fields. When legacy manifests coexist, the root manifest is authoritative; legacy files do not override it or provide a fallback for an invalid root manifest. - `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` diff --git a/eng/external-plugin-quality-gates.mjs b/eng/external-plugin-quality-gates.mjs index c1f2be03cd..6cf710f131 100644 --- a/eng/external-plugin-quality-gates.mjs +++ b/eng/external-plugin-quality-gates.mjs @@ -165,15 +165,15 @@ function cloneSubmissionRepository(workDir, plugin) { }; } -// Ordered list of candidate locations for plugin.json, from most to least specific. +// The root manifest is authoritative; nested conventions are legacy fallbacks. // 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 = [ + ["plugin.json"], [".github", "plugin", "plugin.json"], [".plugin", "plugin.json"], - ["plugin.json"], ]; function toPosixPath(...segments) { diff --git a/eng/external-plugin-quality-gates.test.mjs b/eng/external-plugin-quality-gates.test.mjs index 03c4b9aa03..328df526e3 100644 --- a/eng/external-plugin-quality-gates.test.mjs +++ b/eng/external-plugin-quality-gates.test.mjs @@ -80,6 +80,36 @@ test("manifest compliance passes for an Agent Plugins v1.0.0 manifest", () => { assert.equal(result.status, "pass", result.output); }); +function writeLegacyManifests(pluginRoot, content) { + for (const segments of [[".github", "plugin"], [".plugin"]]) { + const dir = path.join(pluginRoot, ...segments); + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync(path.join(dir, "plugin.json"), content); + } +} + +for (const [name, content] of [ + ["legacy fields", JSON.stringify({ name: "legacy", version: "0.9.0", skills: "./legacy-skills" })], + ["invalid JSON", "{"], +]) { + test(`manifest compliance prefers the root over nested manifests with ${name}`, () => { + const pluginRoot = createPluginFixture(); + writeLegacyManifests(pluginRoot, content); + const result = inspectAgentPluginSpecCompliance(pluginRoot); + assert.equal(result.status, "pass", result.output); + assert.match(result.output, /passed for plugin\.json/); + }); +} + +test("manifest compliance does not fall back from an invalid root to valid nested manifests", () => { + const pluginRoot = createPluginFixture(); + writeLegacyManifests(pluginRoot, fs.readFileSync(path.join(pluginRoot, "plugin.json"), "utf8")); + fs.writeFileSync(path.join(pluginRoot, "plugin.json"), "{"); + const result = inspectAgentPluginSpecCompliance(pluginRoot); + assert.equal(result.status, "fail", result.output); + assert.match(result.output, /not valid JSON/); +}); + for (const [name, update, expectedOutput] of [ ["missing manifest", (root) => fs.unlinkSync(path.join(root, "plugin.json")), /No plugin.json found/], ["legacy manifest location", (root) => { @@ -147,6 +177,24 @@ test("vally lint blocks Agent Skills spec violations", async () => { assert.match(result.logOutput, /spec-compliance/); }); +test("vally lint uses root skills rather than legacy manifest skill targets", async () => { + const pluginRoot = createPluginFixture(); + writeSkillFixture(pluginRoot, { name: "Invalid-Name" }); + const legacySkillsRoot = path.join(pluginRoot, "legacy-skills"); + fs.mkdirSync(legacySkillsRoot); + fs.writeFileSync(path.join(legacySkillsRoot, "SKILL.md"), [ + "---", + "name: legacy-skills", + "description: A valid legacy skill for quality gate tests.", + "---", + "Follow these instructions.", + ].join("\n")); + writeLegacyManifests(pluginRoot, JSON.stringify({ skills: "./legacy-skills" })); + 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)." }); @@ -393,6 +441,24 @@ function cloneSubmissionRepo(remoteDir, primaryFetchSpec) { return repoDir; } +for (const [version, expectedStatus] of [["1.0.0", "pass"], ["2.0.0", "fail"]]) { + test(`runVersionMatchGate uses the root version when nested manifests coexist (${expectedStatus})`, () => { + const repoDir = createTempRepo(); + const pluginPath = path.join(".github", "plugins", "example-plugin"); + const pluginRoot = path.join(repoDir, pluginPath); + fs.mkdirSync(pluginRoot, { recursive: true }); + fs.writeFileSync(path.join(pluginRoot, "plugin.json"), JSON.stringify({ version })); + writeLegacyManifests(pluginRoot, JSON.stringify({ version: "1.0.0" })); + const sha = commitAll(repoDir, "Add modern and legacy manifests"); + const result = runVersionMatchGate(repoDir, { + version: "1.0.0", + source: { sha, path: pluginPath.split(path.sep).join("/") }, + }, sha); + assert.equal(result.status, expectedStatus, result.output); + assert.match(result.output, /\.github\/plugins\/example-plugin\/plugin\.json/); + }); +} + test("runVersionMatchGate passes for a tag ref alongside a sha", () => { const remoteDir = initRemoteRepo(); writeValidPluginContent(remoteDir);